woocommerce-blocks
woocommerce-blocks copied to clipboard
Update the context values passed to JS checkout filters to match the parent block's name
Describe the bug
We pass to filters a context value, with the value equal to cart or checkout
This is provided to inform extensions about the exact location that the filter is being applied. The same filter can be applied in multiple places.
With #5885 we decided that the context value for Slots should be equal to the parent block's name: woocommerce/cart or woocommerce/checkout.
We need to update the values for filters as well:
- support the new values and update the docs
- still support the old
cartandcheckoutvalues if they are used somewhere, but - show a deprecation message that they will be removed soon using
@wordpress/deprecated
We need to ensure that any changes we make to this don't break third party integrations that could be using the old names.
Given that these are strings, I can't think of a backwards compatible way to change things here @ralucaStan 😞
PS: Context values passed into the cartItemClass filter are cart and summary (not checkout).
While not a very constructive thought, filter names should embed "positional" context by being unique. The cartItemClass filter inside the summary/checkout component could be called something other than cartItemClass.
If the same component was re-used in different contexts, then it would make sense to pass context down into the filters in that component.
Thank you @manospsyx, you make some really good points.
I also realised after creating the issue that it won't be possible to have multiple values context, cart and at the same time woocommerce/cart.
As the new context passed to Slots is not yet released I am thinking there is still time to revisit our approach.
The only way we can still keep the old context value for compatibility and also have the block's name is, imo, to introduce a new prop. Something like parentBlock or blockContext.
The approaches could be to:
- rename the context prop on Slots to one of the above:
parentBlockorblockContext. - pass for filters both the actual
context="cart"and alsoparentBlock=woocommerce/cartand add a deprecation warning forcontext
If it makes sense we could also paste this information on the DOM by providing the wrappers with something like: data-block-context="woocommerce/cart".
Thoughts?
While not a very constructive thought, filter names should embed "positional" context by being unique. The cartItemClass filter inside the summary/checkout component could be called something other than cartItemClass.
I will create a different issue for this
Thoughts?
Let me summon @xristos3490 😁
I think this is a complex subject overall.
Dealing with "context" in a headless space seems radical. Although, I believe there is a need to draw the line on what we mean when referring to "context." The "context" should not be coupled with a template-related placement. That wouldn't be sustainable.
For example, what's the difference between the noticeContext wc/checkout Vs. the Slot/Filter context woocommerce/checkout? Evidently, we are talking about two different things but reaching from a 3PD perspective; this isn't very clear at the moment.
From a 3PD perspective, I would need to check something like:
if ( context == 'woocommerce/cart' ) {
createNotice ( /... args with context = 'wc/cart' .../ );
} else {
createNotice ( /... args with context = 'wc/checkout' .../ );
}
This further confuses things, suggesting that we are dealing with a generic semantic issue.
Possible handling scopes:
- JS handling for filters, Slots, and InnerBlocks
InnerBlocks should probably already have some mechanism to know the parent block. Is that right?
Nevertheless, all of them should have information about the parent Block that is rendering them. Not sure if we need to examine particular naming based on context for now.
@Raluca, I liked your suggestion to deal with parent blocks instead of generic context. Also, naming things with the block name instead of wc/checkout seems alright!
- PHP REST API handling when registering extended data callbacks
This one's tricky.
Should the REST API know the block namespace? Currently, working with the has_block() function on the global $post seems limited to the Web interface.
I'd love to hear @senadir thoughts on this as we discussed it in a slack thread and raised some concerns that should fit nicely in here.
At this point, I feel like I don't have the overall picture needed to tackle this subject in detail, but we could potentially be preparing a robust system that will scale well.
This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.