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

Fixed missing layout parameter when choosing widget page group #3563

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

ma4nn
Copy link
Contributor

@ma4nn ma4nn commented Sep 30, 2023

Description (*)

This PR fixes the issue that the layout parameter is missing in some cases when choosing the page group on the widget instance page in backend.
As a result not all available block references will be shown in the select field.

This has already been tried to fix in #802, but the implementation does only work when choosing "Specified Page" in the page group select. For all other options like category or product pages (where no additional "Page" select is available) only the default layout handle will be used (instead of e.g. catalog_product_view) and therewith not all available blocks will be shown in "Block Reference".

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

  1. In Backend go to CMS > Widgets and create a new Widget instance
  2. In the form section "Layout Updates" verify that all combinations of "Display On", "Page" and "Block Reference" produce the correct behaviour, i.e. the contents of the "Block Reference" select matches the chosen page
  3. Verify step 2. especially for product pages vs. home page (as mentioned in Fix widgets layout handle on edit #802)

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Template : admin Relates to admin template Component: Adminhtml Relates to Mage_Adminhtml Component: Widget Relates to Mage_Widget labels Sep 30, 2023
@fballiano
Copy link
Contributor

@ma4nn I can't seem to replicate the issue or I didn't understand it correctly

Screenshot 2023-10-21 alle 11 55 26

it seems to me that the layout parameter is always passed

@ma4nn
Copy link
Contributor Author

ma4nn commented Oct 22, 2023

@fballiano thanks for your feedback. As mentioned in the ticket description for "Display On = Specified Page" the layout parameter is correctly passed. But if you choose something else like "All Product Types" the layout parameter is empty and as a result the "Block Reference" select field is missing values like "Bottom Block Options Wrapper" which are only available for catalog_product_view layout handle.

@fballiano
Copy link
Contributor

select#layout_handle doesn't even exist in the DOM, interesting, I'm trying to think of why nobody noticed before but this PR seems to work fine.

@fballiano
Copy link
Contributor

before PR
Screenshot 2023-10-22 alle 16 13 56

after PR
Screenshot 2023-10-22 alle 16 13 22

I can't find any problem with this PR so I'm approving it

@ma4nn
Copy link
Contributor Author

ma4nn commented Dec 20, 2023

What about this mini-bugfix? Is any other reviewer able to verify and approve?

@fballiano
Copy link
Contributor

@addison74 @kiatng @Flyingmana I think this one should be approved, I've tested it and it works

Copy link
Contributor

@kiatng kiatng left a comment

Choose a reason for hiding this comment

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

Following the testing steps outlined: after creating a widget, and select it for editing, and set Display On to All Product Types, the dropdown options for Block References increases as shown by the screenshots captured by @fballiano:
image

@fballiano fballiano changed the title Fix missing layout parameter when choosing widget page group Fixed missing layout parameter when choosing widget page group Feb 9, 2024
@fballiano fballiano merged commit 606c272 into OpenMage:main Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Widget Relates to Mage_Widget Template : admin Relates to admin template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants