Image block: Caption toolbar is not dismissed when cropping image in place #30407
Conversation
…we have to add an onclick to the cropper container. This is similar to what we do when not editing the image. We're also blurring away from the caption field when the caption field loses focus.
ramonjd
force-pushed
the
add/image-block-onclick-to-blur-and-close-caption
branch
from
889b05d
to
7e15bd8
Compare
- Added a key event handler to the cropper wrapper - Added a role attribute with a value of img (A container for a collection of elements that form an image.) an descriptive aria label
ramonjd
force-pushed
the
add/image-block-onclick-to-blur-and-close-caption
branch
from
7e15bd8
to
9733917
Compare
| @@ -51,6 +52,10 @@ export default function ImageCropper( { | |||
| width: width || clientWidth, | |||
| height: editedHeight, | |||
| } } | |||
| onClick={ onClick } | |||
| onKeyDown={ onClick } | |||
There was a problem hiding this comment.
I'm not entirely sure how accessible this is via keyboard anyway.
Are folks happy with adding a
// eslint-disable-next-line jsx-a11y/no-static-element-interactions
to disable for now?
There was a problem hiding this comment.
I don't have any objections to disabling the no-static-element-interactions linting for this. It appears to be done in a dozen or more places already.
The accessibility concerns here aren't my strong suit so happy to defer to others there.
There was a problem hiding this comment.
This is testing well for me 👍
The simplicity of being able to click on the image to blur the caption field feels intuitive. Even if the caption field retains focus when selecting options from the cropping toolbar, it is simple to click the image if the caption toolbar is in the way.
I'm happy to give this a tentative approval now but will be good to get some extra eyes on it all the same 🙂
glendaviesnz
added
[Block] Image
|
@ramonjd I wonder if we should look for a solution at the richtext toolbar level as the same issue occurs with gallery caption, and I am picking anywhere this component is used?
|
|
Looks like RichText focus management is a longstanding issue - #9740 |
Thanks for looking more closely at this @glendaviesnz I agree it makes sense to try to address it at the higher level and start a discussion on the best way to approach it. |
| @@ -540,6 +540,13 @@ export default function Image( { | |||
| ); | |||
| } | |||
|
|
|||
| function onCaptionBlur() { | |||
| if ( captionFocused ) { | |||
| captionRef.current.blur(); | |||
There was a problem hiding this comment.
I don't think we need this ref.blur if #30587 lands.
|
On hold until we resolve a neat way to handle onBlue on RichText components. See: #30587 |
Description
When editing an image caption, clicking on the image itself dismisses the caption toolbar. This is normal and expected.
However, when we click on the image's caption field while cropping an image in place, we can't dismiss the toolbar without cancelling image crop mode.
Furthermore the caption field refuses to yield focus. 🤷
This PR passes an
onBlurhandler down to the RichText package, and blurs the caption field when the field loses focus.This needs to be tested in conjunction with #30587 as the RichText package doesn't currently handle an onBlur callback
We're also blurring away from the caption field when the caption field loses focus.
Resolves #30334
Also remarked upon in #23350 (comment)
How has this been tested?
Manually on desktop and a mobile emulator.
Testing
The caption toolbar should now disappear and the caption field should lose focus.
Screenshots
Before
After
Types of changes
This is a minor bugfix for an existing block (image).
Checklist:
*.native.jsfiles for terms that need renaming or removal).