Skip to content
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

Editing newly created Nav Link text using sidebar causes focus to move to previous link item #68035

Open
3 of 6 tasks
getdave opened this issue Dec 16, 2024 · 8 comments · May be fixed by #68044 or #68063
Open
3 of 6 tasks

Editing newly created Nav Link text using sidebar causes focus to move to previous link item #68035

getdave opened this issue Dec 16, 2024 · 8 comments · May be fixed by #68044 or #68063
Assignees
Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@getdave
Copy link
Contributor

getdave commented Dec 16, 2024

Description

When creating a new Navigation link item you may wish to amend the label text using the block's sidebar controls. However, when you do so the focus is placed back on the previous Navigation item.

It's hard to explain so please watch the video to understand.

Step-by-step reproduction instructions

  1. Add some links to a Navigation block
  2. Ensure block's sidebar is open.
  3. Add a new link to /.
  4. Attempt to edit the label of the block using the Text field in the sidebar controls
  5. See that as soon as it's focused the previous block is selected.

The culprit is almost certainly this line

} else {
// Fallback
selectPreviousBlock( clientId, true );
}

...which was added in 0cea924

Screenshots, screen recording, code snippet

Screen.Capture.on.2024-12-16.at.17-35-26.mp4

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@getdave getdave added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block [Block] Navigation Link Affects the Navigation Link Block labels Dec 16, 2024
@getdave
Copy link
Contributor Author

getdave commented Dec 16, 2024

@jeryj Do you have any more context on why we are selecting the previous block as a fallback? There's no context in the code except the comment "fallback".

0cea924

@getdave
Copy link
Contributor Author

getdave commented Dec 16, 2024

What we actually want to do here is not micro manage the focus if the close is in the sidebar. We could do this by adding a state to track if the focus is in the inspector controls but I"m wondering if thats a bit heavy handed.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Dec 17, 2024
@Mamaduka
Copy link
Member

I think @alexstine was working on something similar. See #61374.

@getdave
Copy link
Contributor Author

getdave commented Dec 17, 2024

Thank you. I'm glad to see a more detailed solution is/was in the works.

However, that effort seems to have totally stalled and has a number of potential knock on effects.

I wonder whether shipping #68044 as a "hotfix" for Navigation and then looking again at #61374 would be a compromise folks would be happy with in order to this bug fixed for users of WordPress in the short term?

@jeryj
Copy link
Contributor

jeryj commented Dec 17, 2024

tl;dr: There's a deeper issue here. The hotfixes we've added to cover the root issue are causing the problem, so let's fix the root issue.

The code causing this issue is a hotfix covering a deeper issue that @ajlende and I are looking at: The block list appender doesn't receive focus when the quick inserter closes.

We've figured out it's due to the useFocusReturn sending focus to the iframe and not the appender because, to the popover which is outside the modal, the iframe is what is focused.

I'm hopeful we can figure out a solution of either:

@getdave
Copy link
Contributor Author

getdave commented Dec 17, 2024

tl;dr: There's a deeper issue here. The hotfixes we've added to cover the root issue are causing the problem, so let's fix the root issue.

Absolutely. It seems there are several interconnecting streams here.

I'd much rather avoid "hotfixes" but by the same token I'd rather not see WP 6.8 land with this bug still in evidence.

Let's see how we're faring at Beta 1.

Thanks for all the efforts around this.

@jeryj
Copy link
Contributor

jeryj commented Dec 17, 2024

Hopefully we can have a PR up this week that helps remove these hotfixes.

@jeryj
Copy link
Contributor

jeryj commented Dec 17, 2024

It doesn't fix this issue, but I think we can build off it to fix this issue properly. Needs review: #68060

@talldan talldan added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Link Affects the Navigation Link Block [Block] Navigation Affects the Navigation Block [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
4 participants