-
Notifications
You must be signed in to change notification settings - Fork 219
Conversation
8639605
to
169296f
Compare
The release ZIP for this PR is accessible via:
|
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Size Change: -102 B (0%) Total Size: 867 kB
ℹ️ View Unchanged
|
169296f
to
b9f23a0
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
b9f23a0
to
014a16c
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
2 similar comments
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Import from @wordpress/elementImport from @wordpress/element
🚀 This comment was generated by the automations bot based on a
|
Script Dependencies ReportThe
This comment was automatically generated by the |
543bf24
to
91739d9
Compare
`EditorBlock` was scoped under the `featured-items` directory at the time of its creation. It is, however, a useful type that should be shared repo-wide. For this reason, I am moving it into the `blocks` type-defs and updating all the references.
Also defines a more generic `WooCommerceBlockVariation` type which should be also useful in the future to implement a similar pattern.
Add two utility functions: 1. `isWooQueryBlockVariation`: is used to check whether a given block is a variation of the core Query Loop block, and also one of the allowed variations within our repo. See: `QueryVariation` enum type. 2. `setCustomQueryAttribute`: is a shorthand to set an attribute within the variation query attribute.
Specifically: 1. Creates a `constant.ts` file to store all shared constants. Currently, the default variation attributes. 2. Move the variations to their own directory. One file per variation. 3. Move the inspector controls into own file and create a conditional logic to allow showing only certain settings.
We had changed the Products on Sale variation slug to something else, but we had forgotten to update the proper enum.
91739d9
to
4ffd40a
Compare
The filter we added to Gutenberg will pass the block and the page, as we might need them in the future and we want to minimize the amount of changes we'll have to do upstream. However, we currently do not use those, so I removed them from our own inner function.
I've tested and the only thing I've found is that when you use Screen.Capture.on.2022-08-17.at.12-07-41.mp4 |
Thanks for testing this PR, @albarin! Glad that you didn't find any issues :D |
* Move `EditorBlock` to general `type-defs` `EditorBlock` was scoped under the `featured-items` directory at the time of its creation. It is, however, a useful type that should be shared repo-wide. For this reason, I am moving it into the `blocks` type-defs and updating all the references. * Define types for the Product Query block Also defines a more generic `WooCommerceBlockVariation` type which should be also useful in the future to implement a similar pattern. * Add Product Query utils Add two utility functions: 1. `isWooQueryBlockVariation`: is used to check whether a given block is a variation of the core Query Loop block, and also one of the allowed variations within our repo. See: `QueryVariation` enum type. 2. `setCustomQueryAttribute`: is a shorthand to set an attribute within the variation query attribute. * Refactor and cleanup the JS demo code Specifically: 1. Creates a `constant.ts` file to store all shared constants. Currently, the default variation attributes. 2. Move the variations to their own directory. One file per variation. 3. Move the inspector controls into own file and create a conditional logic to allow showing only certain settings. * Update webpack config * Add ProductQuery class * Fix `QueryVariation` enum We had changed the Products on Sale variation slug to something else, but we had forgotten to update the proper enum. * Remove unused params from `update_query` The filter we added to Gutenberg will pass the block and the page, as we might need them in the future and we want to minimize the amount of changes we'll have to do upstream. However, we currently do not use those, so I removed them from our own inner function. Co-authored-by: Lucio Giannotta <lucio.giannotta@a8c.com>
@gigitux or @sunyatasattva, where is(are) the relevant PR(s) for the GB fork mentioned in this pull request? I recall someone sharing the PR link with me at some point but I've spent a few minutes trying to find it and I can't. If you could add it to the description in this PR to link the two together that'd be great! Also, it doesn't look like this PR was attached to the current Zenhub sprint, and Epic (no estimate either). Was that intentional? |
We didn't create any PR. We linked the branch that we have created. In any case, we can create it! @sunyatasattva can you take care of this? I don't have permission to create the PR.
Good point! I forgot to attach it to the current sprint. I'm going to update it! Thanks for catching it! |
Why not? Even if it's created as a draft, it can help with discussion around implementation for this use case. Code freeze for WP 6.1 will be coming soon, so it's important to surface API needs and begin these discussions early. Of course, it's possible I'm missing context around why the PR hasn't been created yet 😄 |
Yep, I'm taking care of the Gutenberg side of things. @nerrad The PR wasn't created yet as I am exploring the side-effects of our decisions. As a matter of fact, I have already found a pretty significant bug with our implementation. I thought it might have been blocking (as in, we needed to rethink the approach on the Gutenberg change a bit), but actually I managed to come up with a fix (PR coming soon), although I'm not too happy with that. The PR for this change doesn't worry me much, as we had already discussed it with the Gutenberg team and it's literally 1 LOC. I'll publish it as soon as I finish my in-depth exploration. The other PR which is required for us is going to be more interesting and will need to start as a draft. I have already got something and will publish that ASAP to start the discussion as you have mentioned. |
More info here: #6952 This is the only problem I have encountered with our approach of adding the filter there. The solution I've found doesn't sit with me really well, so if there are alternative suggestions, I'm glad to hear about them. Otherwise, we'll have to accept the trade-off and I'll open the first PR on Gutenberg as soon as we agree about this. |
This is related to this GB issue and I think it would be good to have a consistent way of Query Loop extenders to do some things. I've just created: WordPress/gutenberg#43632 to start some discussions. Do you think something like that in core would simplify extending the block? |
Nik, I'm really liking the direction of your PR to address the issue. I've left a comment in the PR. @gigitux and @sunyatasattva it'd be good to test & provide feedback on Nik's PR. |
The Product Query block is supposed to be the basis for all future blocks that will allow the merchants to select which products to show and how they are going to look. The block is based on creating a variation of the Gutenberg provided Query Loop block, and will have several WooCommerce-specific options.
Phase 1 is the first proof of concept, under an experimental flag, and will only be available within the experimental builds of the repo.
This PR introduces 3 main changes:
In order to achieve all of these three things, we had to extend the attributes that can be passed to the Query Loop block with our own. The way we decided to design this new API is the following:
__woocommerceVariationProps
).__woocommerceVariationProps
will have the shape of any otherBlockInstance
, except for the fact that all the props are optional. This allows to have things such as names, version, supports and other things that might be useful in the future (for now, we mostly use thename
, as variations can't otherwise have names that are readable at runtime to check if a variation itself is active). Inside of it, we have anattributes
object which will contain any further custom attributes that we might add in the future, making it very flexible. All of this is fully typed, to make sure we make our interfaces clear.Screen.Capture.on.2022-08-16.at.18-22-11.mp4
Testing
Product Query
block.Show only products on sale
.Products on sale
block.WooCommerce Visibility
Changelog