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

Switch key and value in propertyMap parameter of StructureResolver::resolveProperties #73

Merged
merged 3 commits into from Feb 12, 2021

Conversation

niklasnatter
Copy link
Contributor

This makes the method consistent to various methods in the Sulu codebase. For example Badge::addRouterAttributesToRequest or FormViewBuilderInterface::addRouterAttributesToFormRequest.

Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

I think we can not change how we do properties selection now. This need to be compatbile to the current implemention in the sulu/sulu core as project could use both sulu for rendering and sulu/headless-bundle. Also I think the properties mapping this make this way how it is currently implemented more sense.

I would also not compare it with addRouterAttributesToRequest because most devs will not get in contact with addRouterAttributesToRequest which are developing templates.

@niklasnatter
Copy link
Contributor Author

It does not change the XML properties param 🙂

It just switches the key and value in the propertyMap parameter of the StructureResolver::resolveProperties method. The XML properties param is handled in the PageSelectionResolver and PageDataProviderResolver.

Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

I understand then its no problem. Interpreted the change in the PageSelectionResolver false!

@niklasnatter
Copy link
Contributor Author

Just wanted to make it consistent with the properties param implemented in sulu/sulu#5820 🙂

composer.json Outdated Show resolved Hide resolved
Co-authored-by: Alexander Schranz <alexander@sulu.io>
@alexander-schranz alexander-schranz merged commit 9b1a299 into sulu:0.x Feb 12, 2021
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.

None yet

2 participants