Focus 1st parent block on block remove, if no previous block is available #48204
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
draganescu
changed the title
|
Size Change: +774 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
draganescu
requested review from
youknowriad,
Mamaduka,
afercia and
ellatrix
draganescu
added
[Type] Bug
draganescu
added
[Priority] High
draganescu
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
Flaky tests detected in e0304f6bef7991d14dbd48d8b86453ca3c015418. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4282471922
|
draganescu
removed request for
juanmaguitar,
ajitbohra,
ryanwelcher and
fabiankaegy
|
@draganescu Focus issue is confirmed fixed on Windows 10 Chrome/Firefox. This is working great. As a follow-up, we should look at the same thing evaluated for the navigation block. When deleting a block from the document overview, focus is switched back to the canvas instead of remaining in the list view. I think it would be helpful to simply move focus in the list view if the block was deleted from the list view. I have heard this will not be an easy task but I think the navigation block does this now so maybe it will be easy enough as a follow-up. Thanks. |
ntsekouras
reviewed
Feb 18, 2023
There was a problem hiding this comment.
Thanks for the PR @draganescu! I just left a comment and will have a closer look next week.
ellatrix
reviewed
Feb 20, 2023
This comment was marked as resolved.
This comment was marked as resolved.
ellatrix
reviewed
Feb 20, 2023
draganescu
force-pushed
the
fix/block-remove-focus-management-no-siblings
branch
from
39135d9 to
7d1b30b
Compare
Co-authored-by: Dave Smith <444434+getdave@users.noreply.github.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
draganescu
force-pushed
the
fix/block-remove-focus-management-no-siblings
branch
from
5054ece to
cbacabf
Compare
alexstine
reviewed
Feb 28, 2023
|
@alexstine test added |
alexstine
approved these changes
Feb 28, 2023
|
@Mamaduka can you confirm this is making it into backports? |
ntsekouras
reviewed
Mar 2, 2023
ntsekouras
added
the
Needs Dev Note
ntsekouras
pushed a commit
that referenced
this pull request
Mar 7, 2023
…able (#48204) * change default selection on remove blocks action to include selecting first available parent if there is no previous block * revert renaming the selectPreviousBlock API and add a param to also select the parent when needed * Fix removeBlock call in list item block * updates remove block test to account for new selectPreviousBlock param * Try to fix editing-widgets e2e test * focus back on widget body * Addressed feedback for documentation being more explanative Co-authored-by: Dave Smith <444434+getdave@users.noreply.github.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> * remove superfluous fix attewmpt * adds a test for parent selection by default --------- Co-authored-by: Ella van Durpe <ella@vandurpe.com> Co-authored-by: Tetsuaki Hamano <tetsuaki.hamano@gmail.com> Co-authored-by: Dave Smith <444434+getdave@users.noreply.github.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
|
I just cherry-picked this PR to the wp/6.2 branch to get it included in the next release: 411a5c2 |
ntsekouras
removed
the
Backport to WP 6.7 Beta/RC
47 tasks
|
Drafting a Dev Note: Fixed focus issue after Block removal. In WordPress 6.2 the behavior of removeBlock() changed. Until then, when a block was removed, the previous block was selected (= gained focus). Occasionally, if there wasn't any previous block available, it resulted in focus loss, which is particular grave when using a screen reader. With this update, the remove block behavior was augmented to select the first parent instead. For developer writing Custom Blocks, that might require a need to update their code, so they also account for the possibly that sometimes the parent is selected on block removal. There is a way to opt-out of this behavior by passing false as the second argument to Example code from the core/list block. Details in #48204 |
bph
added
has dev note
|
Second review notes: 2nd sentence: "Occasionally, if there wasn't any previous block available, it resulted in focus loss, which is particular grave when using a screen reader." Change to "Occasionally..., which is particularly difficult when using a screen reader." (Grave will not be understood globally, and should be 'particularly') 3rd sentence: "With this update, the remove block behavior was augmented to select the first parent instead." Suggest "With this update, the behavior of the remove block was changed to select the first parent instead. 4th sentence: "For developer writing Custom Blocks, that might require a need to update their code, so they also account for the possibly that sometimes the parent is selected on block removal. Change to: "For a developer writing Custom Blocks, this might require an update to their code, to account for the possibility that sometimes the parent is selected on block removal." |
|
Fixed focus issue after Block removal. In WordPress 6.2 the behavior of the With this update, the behavior of the remove block action was changed to select the first parent when there is no previous block available. For a developer writing Custom Blocks or otherwise depending on the result of the Example code: Details in #48204 |
|
@bph updated the devnote with a few tweaks and the suggestions from @abhansnuk 🙏🏻 |
|
@draganescu Where did you make your edits? I went ahead and published it on the Misc Editor dev note already - with Abha's suggestions. Sorry, we were on a deadline. |
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.
What?
Closes #48017
Trying to fix a focus loss issue with the navigation block (see #48105) I found that at all times when we remove the last child of a block we end up with focus loss - focus moves to the
bodyof the editor'siframe.This PR fixes this and also the original issue.
Why?
Focus loss is one of the worst bugs in terms of using the UI via assistive technology. We already handle this by:
How?
This PR selects the 1st parent if the current block to be removed has no previous block to be selected. It also solves the same for the list view and the experimental off canvas editor component by updating the focus management logic in:
selectPreviousBlockaction of the block editor store, making it intoselectPreviousBlockOrFirstParentupdateSelectionAfterRemovecallback of theBlockSettingsDropdowncomponentTesting Instructions
In a post, a template or a template part:
To do