-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Revisions: Specify block level diff status via aria-label #77779
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
8afb2a8
97e5f1a
f58a0e5
1b202c9
20e03c4
97aef97
7f5268e
1564b71
c07f0fa
3547840
877b4ac
b931940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ import clsx from 'clsx'; | |
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { __, _x, isRTL } from '@wordpress/i18n'; | ||
| import { __, _x, isRTL, sprintf } from '@wordpress/i18n'; | ||
| import { | ||
| ToolbarButton, | ||
| ToggleControl, | ||
|
|
@@ -119,6 +119,18 @@ function ParagraphBlock( { | |
| style: { direction }, | ||
| } ); | ||
| const blockEditingMode = useBlockEditingMode(); | ||
| const { 'aria-label': injectedAriaLabel, ...restBlockProps } = blockProps; | ||
| // translators: %s: block type title e.g. "Paragraph" | ||
| const blockLabel = sprintf( __( 'Block: %s' ), blockProps[ 'data-title' ] ); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's a good idea to force blocks to change. Why are moving this here?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we apply the label after spreading rich text props in the block props?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applying it after the spread was overriding the injected diff labels. |
||
|
|
||
| let ariaLabel = __( 'Block: Paragraph' ); | ||
| if ( injectedAriaLabel !== blockLabel ) { | ||
| ariaLabel = injectedAriaLabel; | ||
| } else if ( RichText.isEmpty( content ) ) { | ||
| ariaLabel = __( | ||
| 'Empty block; start writing or type forward slash to choose a block' | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
|
|
@@ -143,21 +155,15 @@ function ParagraphBlock( { | |
| <RichText | ||
| identifier="content" | ||
| tagName="p" | ||
| { ...blockProps } | ||
| { ...restBlockProps } | ||
| value={ content } | ||
| onChange={ ( newContent ) => | ||
| setAttributes( { content: newContent } ) | ||
| } | ||
| onMerge={ mergeBlocks } | ||
| onReplace={ onReplace } | ||
| onRemove={ onRemove } | ||
| aria-label={ | ||
| RichText.isEmpty( content ) | ||
| ? __( | ||
| 'Empty block; start writing or type forward slash to choose a block' | ||
| ) | ||
| : __( 'Block: Paragraph' ) | ||
| } | ||
| aria-label={ ariaLabel } | ||
| data-empty={ RichText.isEmpty( content ) } | ||
| placeholder={ placeholder || __( 'Type / to choose a block' ) } | ||
| data-custom-placeholder={ placeholder ? true : undefined } | ||
|
|
||
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.
We should be careful not to introduce a new API here. If it wasn't possible before for extenders to set an aria label, then I'm not sure if we should add that. Perhaps we can check the store if revisions mode is on, or add some React context?
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.
Hi @ellatrix, thanks for the feedback!
What do you think about using a private React context (e.g., BlockAriaLabelOverrideContext exported via lock-unlock)? We could provide the diff label from revisions-canvas.js and consume it directly in useBlockProps.
This approach lets us completely revert the changes to the paragraph block and avoids passing anything through wrapperProps.