Product search block as core/search variation #6191
Conversation
rubikuserbot
requested review from
a team and
Aljullu
and removed request for
a team
Script Dependencies ReportThe
This comment was automatically generated by the |
|
Size Change: +499 B (0%) Total Size: 868 kB
ℹ️ View Unchanged
|
tomasztunik
force-pushed
the
try/product-search-as-variation
branch
from
26f73c1
to
6a163ba
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
1 similar comment
Script Dependencies ReportThe
This comment was automatically generated by the |
tomasztunik
force-pushed
the
try/product-search-as-variation
branch
from
867c945
to
d8ea416
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
- make old block deprecate more gracefully, - update to typescript - use Warning component to create cleaner experience of updating the block
Script Dependencies ReportThe
This comment was automatically generated by the |
Co-authored-by: Albert Juhé Lluveras <[email protected]>
|
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 |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
dinhtungdu
added
the
needs: design
|
Apologies on the delay on feedback @dinhtungdu 🙏
Are we required to use the term Re: the design, I think there's more we could do for this block - I'm going to check if there's another pattern other teams are using. Otherwise I agree with @Aljullu 's comment on the vertical alignment. |
|
@vivialice @Aljullu I updated the Warning component copy and spacing in c672048. Here is how it looks now:
|
|
Alignment looks good! 😄 I think I need to change the copy slightly. To confirm: the product search block will not work if the user doesn't upgrade? Or will it still appear on the front end? If so, I think we should update to Also cc @sunyatasattva for copy alignment if required! |
@vivialice The legacy block will continue working until users upgrade. So I think Update: Even the (old) search block works without users' action, I think we still should use |
There was a problem hiding this comment.
LGTM 
Thanks @vivialice for the design feedback and @dinhtungdu for your work moving this forward. 👏
github-actions
bot
added
status: ready to merge
and removed
status: needs review
labels
|
@Aljullu I would like to pause merging this PR to think about the post-deprecation process. Eventually, we want to remove the My plan is we will just remove those components once WP 6.1 is released due to our L0 support policy. I'm wondering if there is any better alternative and if there is anything we can do now to make the transition easier. |
|
That's a good point, thanks for raising this, @dinhtungdu!
I'm not sure if I agree to consider this a blocker, though. That component is only loaded in the editor, and its performance impact should be negligible, right? However, I agree that it might be good to 'clean up the code at some point, I wonder if we could do something like what Gutenberg did with the Gallery block: WordPress/gutenberg#36191. I didn't take a deep look, but they seem to have deprecated several past versions. How do you feel about doing a 1-day research spike to see if there is anything from that PR (or from other GB blocks) that we can reuse? Based on that, we can decide whether we merge this PR as-is or not. (Even though, if possible, I would prefer to merge this PR and work in an auto-migration in another PR) |
Yes, it's just a concern about code maintenance.
@Aljullu I agree. Let's merge this one and create an issue to track the research and development of auto-migration. It may be the solution for not only the search block but filter blocks as well. |
tarhi-saad
added
type: enhancement
Implements Search Block as a variation of Gutenberg core/search block:
Compatibility:
Should be tested against Gutenberg ≥ 13.4
Limitations:
core/searchvariation, it's out of our control.Fixes #6170
Accessibility
prefers-reduced-motionOther Checks
Screenshots
Screen.Recording.2022-07-29.at.12.57.56.mov
Testing
Automated Tests
User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
trunk), not a variation of the Search block. See the block is editable in the editor and working as expected on the front end.Performance Impact
N/A
Changelog