-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Block editor: scroll selected block only if it has no focus #30924
Conversation
@@ -102,6 +103,8 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { | |||
const mergedRefs = useMergeRefs( [ | |||
props.ref, | |||
useFocusFirstElement( clientId ), | |||
// Must happen after focus because we check for focus in the block. | |||
useScrollIntoView( clientId ), |
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.
Ideally we have a hook that does combines this: if a selected block has no focus, either focus the first element (if so desired) or scroll the block into view.
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.
(If we do set focus, the browser will automatically scroll it into view if needed.)
Size Change: -3 B (0%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
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.
This works great in my testing. It's also a great refactoring.
Awesome, thanks for the improvement :) |
Thanks! |
Description
Introduced by #28191.
Fixes #20764.
Currently, when you select any block that is even slightly out of view, the page will be automatically scroll the block so that the top of the block is visible.
This can be very disorientating especially when dealing with large blocks.
For example: clicking at the bottom of a large cover block would scroll the top into view and the bottom out of view.
Generally, we should not be scrolling the page at all when a block is selected by the user.
This PR only allows scrolling only if the focus is not within the block (and thus not selected by the user directly).
I moved this behaviour to the block component so that we no longer need
getBlockDOMNode
.How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).