-
Notifications
You must be signed in to change notification settings - Fork 4.7k
FontSizeControl (Next): Fix SelectDropdown voiceover focus issues #28408
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
Conversation
|
Size Change: -9.58 kB (-1%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
|
@ItsJonQ Looking good. Only one of the items I mentioned still seems to be an issue. When the Font Size dropdown (button element) initially receives focus it doesn't seem to have the Compared to when the dropdown has been toggled open and closed at least once: |
|
@talldan Wonderful!! Thank you for that feedback 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Tested with NVDA and JAWS on Firefox and the reset button focus has been fixed. The only remaining issue from @talldan's list is the lack of aria-expanded in the first render.
|
@talldan + @diegohaz Fixing it here: Will create a release and make an update in this PR as soon as I can 🙏 |
|
@gziolo Hmm! It looks like there's something in the latest rtlcss V3.0.0 that's causing issues with babel (specifically with https://github.com/WordPress/gutenberg/runs/1750693127?check_suite_focus=true |
I can see it in the source of the package. It looks like they write code to target code in Node.js mostly. It looks like @jsnajdr was right that it's inevitable that we will have to start transpiling some of the packages in |
|
Not sure what to do with the failing E2E test :(. I think there's something to it, but I'm not sure what. Replicating the actions locally, it works as expected. Running it locally, the "reset" related ones pass for me. There's another failure, but that's related to the theme's font setting for what a "small" size (test is looking for |
|
cc'ing @gziolo. Would you know what we can do for the E2E failure? The hardest thing is that it's inconsistent between my local machine and what's on Github actions |
Update @wp-g2 packages to the latest v0.0.146
16e63df to
1a30b14
Compare
|
I rebased this branch. It fails for me locally for the following test:
It passes in the interactive mode. There needs to be something wrong with the data propagation when the check is done very quickly. The one you mentioned that fails for you, it passes for me 🤷🏻 It fails locally when you run an interactive mode but without delay: npm run test-e2e packages/e2e-tests/specs/editor/various/font-size-picker.test.js -- --puppeteer-interactive --puppeteer-slowmo=1You can record it and check it in slow motion what happened. I need to relocate now :) |
|
This should be resolved in #28707. |
|
Resolved in #28707 |




This update fixes the voiceover focus interactions for the (next)
FontSizeControlcomponent (recently merged #27594)The solution was to update to the latest
@wp-g2version (v0.0.143):https://github.com/ItsJonQ/g2/releases
These changes include updates to
z-indexand interaction fixes to the underlyingSelectDropdowncomponent (ItsJonQ/g2#239).Potentially resolves #28411
How has this been tested?
Tested locally on Gutenberg with Chrome + Firefox + Voiceover (Mac OS)
Also tested in the
@wp-g2pull request in a similar manner.To test:
npm run devChecklist: