Image block: avoid key to focus caption #25493
Conversation
|
Size Change: +29 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
| if ( url && ! prevUrl && isSelected ) { | ||
| captionRef.current.focus(); | ||
| } | ||
| }, [ url, prevUrl ] ); |
There was a problem hiding this comment.
I understand now, thanks for the separate PR. I think this implementation is better but I wonder if focusing the block (the container) is actually better after selecting an image.
There was a problem hiding this comment.
Maybe @jasmussen has ideas.
There was a problem hiding this comment.
Can you walk me through the challenge?
This is what I see in the shipping product:
This is what I see in this branch:
They seem the same, and seem okay to me. There's a piece I'm missing.
There was a problem hiding this comment.
@youknowriad To be honest, I would love if we focus the container instead. This makes much more sense to be and it's something I've proposed before as a general behaviour (focus container, and if it's editable it automatically sets the caret).
@jasmussen No behaviour change to master in the branch. The question is if the placeholder => image focus change to caption is good.
There was a problem hiding this comment.
Are we ok with merging this PR in the meantime?
There was a problem hiding this comment.
The focus-the-container vs. focus-the-caption-field sounds like a good accessibility question. Pinging @afercia to check this out.
There was a problem hiding this comment.
Thanks for the ping. To my understanding there's no practical change to the keyboard interaction, right? @ellatrix
On a side note: I think we discussed but never established a good principle for the "initial focus" on the blocks. Generally, setting focus on a place that makes great part of the block UI "skipped" isn't ideal. Not to mention that The image block isn't fully operable with keyboard yet. For exampe, I can't find a way to move focus to the caption toolbar.
|
Thanks! |
Description
There's no reason to re-render the whole image block just to re-focus the caption.
How has this been tested?
Screenshots
Types of changes
Checklist: