Search block: switch interactivity to the Interactivity API #53343
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbravobernal
added
[Type] Code Quality
|
Size Change: +8.4 kB (+1%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
We now use WP Store to handle ARIA labels for button functionality, doing away with hard-coding labels. The labels now adjust based on the state of the search field, providing more explicit instructions for users. Co-authored-by: David Arenas <david.arenas@automattic.com>
|
As mentioned in #53292 (comment):
@DAreRodz and I implemented this for the |
|
I made some commits to support what I shared in this comment: link. I still have to review it properly, but it seems to be working fine, now. I want to:
|
Regarding this, it seems that, as the I added |
|
Flaky tests detected in 72426b0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6074323676
|
SantosGuillamot
requested review from
luisherranz,
michalczaplinski and
DAreRodz
Comment on lines
193
to
195
| 'ariaLabel' => 'true' === $open_by_default ? $aria_label_expanded : $aria_label_collapsed, | ||
| 'ariaControls' => 'true' === $open_by_default ? null : $input_id, | ||
| 'type' => 'true' === $open_by_default ? 'submit' : 'button', |
There was a problem hiding this comment.
One thing's not clear to me:
If $open_by_default is hardcoded to be false on this line and never changed as far as I can see, then why do we still need those conditionals?
There was a problem hiding this comment.
I added those conditionals so they follow the same logic we have in the client. I thought that this way, if for any reason we decide to change the $open_by_default in some scenarios, it is clear which attributes would be affected.
There was a problem hiding this comment.
Oh, got it. For me, it was slightly confusing because the existence of the $open_by_default variable implies that there are cases when it's not open by default. Just a comment above this line mentioning that, for now, open_by_default is a constant should be enough to clarify that.
There was a problem hiding this comment.
I just added a comment to reflect that 🙂
luisherranz
changed the title
luisherranz
added
the
[Feature] Interactivity API
21 tasks
SantosGuillamot
requested review from
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
I've updated a bit the code so I'd appreciate a new review and (potentially) an approval. cc: @luisherranz @DAreRodz This pull request not only moves the Search block to use the Interactivity API but also solves a bug with the current implementation: Search.block.bug.mp4 |
24 tasks
luisherranz
approved these changes
Sep 11, 2023
There was a problem hiding this comment.
Everything works as expected and the code is clean and easy to follow. Great work! 🙂
I've added a couple of minor comments below.
A note about accessibility: we have copied the previous accessibility implementation, so everything should keep working as it was and I don't think we need to bother contributors to check again what we already checked. But if anyone finds any accessibility issue in the future in this block and finds this PR, please open a new issue and ping any of us, and we will fix it. Thanks!
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
SantosGuillamot
deleted the
experiment/move-search-block-interactivity
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Fixes #53292
Why?
To add consistency to all Core blocks that needs frontend interactivity.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast