Breadcrumb with PHP8 attributes #87
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updated the breadcrumbs to avoid the following situation in our latest framework projects:
Should be reviewed thoroughly.
I've refactored quite a big chunk of the breadcrumb functionality to solve previously reported bugs. Recent project had some issues with the parent chaining part of the current breadcrumb implementation.
I've removed the ability to pass parameters to
gettermethods as they only allowed static values, e.g.{item.name:nl}and not dynamic properties like{item.name:locale} <- would't work, which makes them not really useful and we had to include a lot more code to support them. By removing getter parameters we can rely on the Symfony ProperyAccessor component, which is more robust and mature. If the situation calls for them we should look into how to make these things work in a clean way, but for now you can simply rewrite the method for the breadcrumb. For example:Having a fallback for these kidsn of methods, like a regular
getNamewhich returns a default is a good idea anyway.I've also removed the following example from the current docs:
The nesting of parameters (
parent={"name"="author", "parameters"={"author"="{author.id}"}})when we already have the{author}parameter in the current URL isn't necessary in my opinion. For now we fallback togetIdto perform the matching of the objects, eliminating the need to define any parameters for a parent route as long as the object names match and the ID is used for paramconversion. If custom paramconverters are used (for example hashes in the front-end), we could support those with a simpler attribute syntax, for example'mapping' => 'hash'.In short, using a logical URL structure and only referring to a parent route if your URL extends from it will work:
This will work because {order} is present in the URL at all times during the breadcrumb chain.
The following will not:
This might seem like a pretty big limitation, but the workaround that is needed to accommodate these kinds of breadcrumbs is huge, and the URL wouldn't even make sense.
To do:
routeoption. When do we use this? Isn't parent always a better option? When would we link to a previous breadcrumb that isn't a parent?