Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow passing options from template to the MenuBuilder #393

Closed
wants to merge 1 commit into from

Conversation

gnat42
Copy link

@gnat42 gnat42 commented Jun 7, 2024

We were building a menu where we needed to pass some variables from our view to the knp_menu_render function. However in the builder for that menu, the $options array was always empty. With these changes the options successfully get passed through

Copy link
Collaborator

@garak garak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test (or change an existing test) to cover this situation

@gnat42
Copy link
Author

gnat42 commented Jun 8, 2024

Hello, I took a second look and noticed additional locations where the options wouldn't make their way to the MenuBuilder so made more modifications and added tests to cover the modified methods.

We were building a menu were we needed to pass some variables from our view to the knp_menu_render function. However in the builder for that menu, the $options array was always empty. With these changes the options successfully get passed through
@garak garak requested a review from stof June 10, 2024 17:06
@garak
Copy link
Collaborator

garak commented Jun 14, 2024

On a closer look, and after briefly discussing with stof, the idea seems not quite good.
The same change was already proposed in the past, and it was rejected, see for example PR #90 and #337

@gnat42
Copy link
Author

gnat42 commented Jun 18, 2024

OK, so can you explain how the docs show passing options to your menu builder but that's not actually possible? Did we mis-understand the docs or how to do this? It seems like a bug but I don't know enough to know what should be happening.

@stof
Copy link
Collaborator

stof commented Jun 18, 2024

@gnat42 passing options from the template to the menu builder is possible when using the knp_menu_get Twig function to get the menu instance.

@gnat42
Copy link
Author

gnat42 commented Jun 19, 2024

Thanks @stof much appreciated!

I'll look to see if a PR for the docs are appropriate to clarify how to do this and submit if its warranted.

@gnat42 gnat42 closed this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants