shopware icon indicating copy to clipboard operation
shopware copied to clipboard

NEXT-14691 - Add pseudo modal twig blocks

Open lacknere opened this issue 1 year ago • 1 comments

1. Why is this change necessary?

Pseudo modal template is missing many twig blocks which may be used to override/extend the template easily from themes/plugins.

2. What does this change do, exactly?

Add the missing twig blocks and deprecate an inappropriately named block.

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

NEXT-14691

5. Checklist

  • [x] I have rebased my changes to remove merge conflicts
  • [ ] I have written tests and verified that they fail without my change
  • [x] I have created a changelog file with all necessary information about my changes
  • [ ] I have written or adjusted the documentation according to my changes
  • [ ] This change has comments for package types, values, functions, and non-obvious lines of code
  • [x] I have read the contribution requirements and fulfil them.

lacknere avatar Feb 12 '24 10:02 lacknere

Warnings
:warning: You probably moved or deleted a twig block. This is likely a hard break. Please check your template changes and make sure that deleted blocks are already deprecated.
If you are sure everything is fine with your changes, you can resolve this warning.
Moved or deleted block:
Array ( [0] => Array ( [0] => product_detail_zoom_modal_close_button_content [1] => component_pseudo_modal_back_btn )
)

github-actions[bot] avatar Feb 12 '24 10:02 github-actions[bot]

Hi @lacknere,

thank you for your contribution 🎉 and sorry for my late review.

I know these are "just" blocks, but do you really need blocks on every single element?

The good thing about the pseudo-modal right now is, that the general modal markup could change in the background (for example due to a Bootstrap update) and it would not be an issue because new PseudoModalUtil() would still work the same. If every chunk of HTML has a block, every change/moving of the HTML can be considered as a breaking change. Maybe blocks for "overall wrapper", "header", "title" and "content" would be enough.

tobiasberge avatar Mar 15 '24 09:03 tobiasberge

@tobiasberge I've removed the modal dialog and content blocks. The new blocks are now "overall", "header", "title", "close button", "body" and "back button". Is that okay?

lacknere avatar Mar 18 '24 20:03 lacknere

Hi @lacknere thank you. 👍 LGTM.

And good catch with the wrong block naming product_detail_. 🚀

tobiasberge avatar Mar 20 '24 12:03 tobiasberge

Hello,

thank you for creating this pull request. I have opened an issue on our Issue Tracker for you. See the issue link: https://issues.shopware.com/issues/NEXT-34491

Please use this issue to track the state of your pull request.

Hi @lacknere, you changes have been merged to trunk: https://github.com/shopware/shopware/commit/f5bc948b784cc53da487f9f62352d989ec0558cf and will also be backported to 6.5.x

Thank you for your contribution. 🎉

tobiasberge avatar Mar 22 '24 14:03 tobiasberge