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

Proposal: Allow class attribute for referenceBlock type in layout xml #85

Merged
merged 1 commit into from
Mar 25, 2019
Merged

Proposal: Allow class attribute for referenceBlock type in layout xml #85

merged 1 commit into from
Mar 25, 2019

Conversation

sivaschenko
Copy link
Member

Allow class attribute for referenceBlock type in layout xml

Problem

It is impossible to substitute a block class and/or customize block method(s) implementation on a specific page keeping layout configuration of the block (position, sorting in the layout, child blocks, arguments).

Currently available block class substitution approaches do not resolve the problem:

Currently available approach Why the approach does not resolve the problem
Introduce a preference for block class in di.xml Block is substituted globally, not only on the desired page
Remove the existing block from the layout and declare a new one Layout configuration related to this block is lost
Use plugins Block functionality is affected for all instances, not only on the desired page
Use plugins that change method functionality only for specific page/layout handle This approach is imperative, implicit, prone to mistakes, bugs and implementation inconsistency, it is not compatible with layout updates

Solution

Allow the class attribute of blockReferenceType in lib/internal/Magento/Framework/View/Layout/etc/elements.xsd

See: magento/magento2#18933

Pros

  • Possible to substitute a block class for a specific page easily and in declarative way preserving layout configuration of the block

Cons

  • Allows developer to substitute a block class with a class that is not compatible with a template

POC

POC is implemented in public pull request: magento/magento2#18933

Requested Reviewers

@leandrommedeiros - contributor
@antonkril
@maghamed
@akaplya
@kandy

@sivaschenko sivaschenko mentioned this pull request Feb 6, 2019
@maghamed
Copy link
Contributor

maghamed commented Feb 6, 2019

Allows developer to substitute a block class with a class that is not compatible with a template

This is called Duck Typing, as the contract of a block is not defined in advance it's defining by the template/child blocks which use it

Copy link
Contributor

@maghamed maghamed left a comment

Choose a reason for hiding this comment

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

Structural typing is preferable in OOP in comparison with Duck Typing which this proposal incorporates

@sivaschenko
Copy link
Member Author

@maghamed It is possible to implement structural typing for this proposal by restricting the class defined in class attribute of referenceBlock to be instance or child of the original block class defined in block node.

@sivaschenko sivaschenko mentioned this pull request Feb 6, 2019
@orlangur
Copy link
Contributor

orlangur commented Feb 13, 2019

@maghamed,

Structural typing is preferable in OOP in comparison with Duck Typing which this proposal incorporates

Absolutely not applicable here as we allow complete removal of a particular node and re-adding it back.

This proposal is just a syntactic sugar to avoid something as ugly as https://github.com/magento/magento2/pull/11709/files

@Vinai
Copy link

Vinai commented Feb 18, 2019

I think this would be a nice addition. This gives layout XML more leverage, that is, once learned, more can be achieved with the tool.

@johnhughes1984
Copy link

Agreed this would be a nice addition.

Also I don’t believe the ‘con’:

Allows developer to substitute a block class with a class that is not compatible with a template

Is any different from a developer currently being able to replace the existing template with one that is not compatible with the block class or that you can add / replace / remove arguments in layout XML that could equally break something.

@buskamuza buskamuza merged commit f615b62 into magento:master Mar 25, 2019
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.

7 participants