Add column count to the column block label #30248
Merged
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
diegohaz
added
[Type] Enhancement
diegohaz
commented
Mar 25, 2021
|
Size Change: +5.45 kB (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
|
I have tested the PR with NVDA in Chrome on Windows 10. When the columns block (the parent) is selected, it reads the labels for all inner columns: clickable Block: Columns grouping and so on. When I navigate between the column blocks the counter is announced correctly. -In the video, how did you navigate between the inner columns? Update: I can only reproduce this with NVDA. So maybe it is just about me not being sure how to navigate when the screen reader is active 🙏 |
carolinan
reviewed
Mar 26, 2021
| @@ -73,6 +76,10 @@ function ColumnEdit( { | |||
| : InnerBlocks.ButtonBlockAppender, | |||
| } ); | |||
|
|
|||
| const columnsCount = columnsIds.length; | |||
| const currentColumnPosition = columnsIds.indexOf( clientId ) + 1; | |||
There was a problem hiding this comment.
Would getBlockIndex and getBlockOrder work for this? When I tried earlier this week I was not able to solve it.
|
Just interested, does anyone think this PR could conflict with what I am trying to do here? I believe I have used getBlockCount() there to grab the total. I just wonder if this would create a collision of sorts later down the road. |
|
Thanks for reviewing this @carolinan!
That's indeed a problem. I believe it's due to the use of the
Yes! I can confirm this. I created #30620 to track the issue as it seems to be a different thing. In the video, I used VoiceOver and VO+arrow keys to navigate through the columns. I guess we could experiment with the For now, I think this PR is good to be approved/merged as a simpler solution for the label issue. What do you think? |
ntsekouras
reviewed
Apr 12, 2021
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.
Description
Closes #29804
This PR modifies the default label on the column block so it includes the position and the total number of columns in the Columns block.
How has this been tested?
Followed instructions on #29804 using VoiceOver.
ScreenFlow.mp4