slope-it / breadcrumb-bundle

A bundle for generating dynamic breadcrumbs in Symfony applications
Other
12 stars 4 forks source link

Exclude unneeded parameters when generating route #8

Closed Jupi007 closed 2 years ago

Jupi007 commented 2 years ago

The idea of the auto-filling route parameters is nice, but it can cause the addition of useless parameters like this:

/route/name?slug=nice-slug&id=3

I think add something like a newdisableAutofill option could be useful to disable this behaviour.

asprega commented 2 years ago

Thanks for your report. In fact, this is an unintended behavior. I think we should consider a different fix though, and make sure to set only parameters that are actually used by the passed route. In other words, I don't think we should add a configuration parameter as the behavior you're observing is never desired. I will look into a better way to build the url by only extracting parameters used by both routes (current one and the one being built).

Jupi007 commented 2 years ago

Okay. I tried to patch this in a cleaner way (looking at matching parameters as you said), but I didn't found a solution. That's why I proposed an option to disable this behaviour.

I hope you will found a better solution. Should I close the PR?

asprega commented 2 years ago

I think I found a way to make it work using RequestContext. Give me a few days and I'll open a PR. I will let you know when it's ready :)

Jupi007 commented 2 years ago

Very nice, thanks!

asprega commented 2 years ago

Hey, it was easier than expected :) I tried this in my own project and it works, can you give it a shot on your project and let me know? You will have to require a development version of this library pointing to this branch.

In the meanwhile I would like to add some integration tests with the framework components themselves and, if everything works, I will tag a new version.

Thanks!

Jupi007 commented 2 years ago

Awesome, I test this at once!

Jupi007 commented 2 years ago

Thanks a lot to have fixed this :)

This bundle is really awesome, and I'm happy to see that all issues I noticed are fixed now 🥳

asprega commented 2 years ago

Just released 1.3.0 including this fix! I'm very glad you're finding this useful. Thanks!

Jupi007 commented 2 years ago

Thank you very much, I'll update my project at once :)