-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Scroll to previous location in document with editor back button #73737
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
Changes from all commits
e01cbd6
223f08f
1e99e7f
1fc015c
58012a6
9344278
eaf603f
02d8407
d5059c1
7f5458c
073510e
5c65a62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,19 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { useSelect } from '@wordpress/data'; | ||
| import { useSelect, useDispatch } from '@wordpress/data'; | ||
| import { store as coreStore } from '@wordpress/core-data'; | ||
| import { Notice } from '@wordpress/components'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { useEffect } from '@wordpress/element'; | ||
| import { store as blockEditorStore } from '@wordpress/block-editor'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { store as editorStore } from '../../store'; | ||
| import { TEMPLATE_POST_TYPE } from '../../store/constants'; | ||
| import { useRestoreBlockFromPath } from '../../utils/block-selection-path'; | ||
| import EditorInterface from '../editor-interface'; | ||
| import { ExperimentalEditorProvider } from '../provider'; | ||
| import Sidebar from '../sidebar'; | ||
|
|
@@ -25,6 +28,7 @@ function Editor( { | |
| settings, | ||
| children, | ||
| initialEdits, | ||
| initialSelection, | ||
|
|
||
| // This could be part of the settings. | ||
| onActionPerformed, | ||
|
|
@@ -94,6 +98,32 @@ function Editor( { | |
| [ postType, postId, templateId ] | ||
| ); | ||
|
|
||
| const { selectBlock } = useDispatch( blockEditorStore ); | ||
| const restoreBlockFromPath = useRestoreBlockFromPath(); | ||
|
|
||
| // Restore initial block selection if provided (e.g., from navigation) | ||
| useEffect( () => { | ||
| if ( ! initialSelection || ! hasLoadedPost || ! post ) { | ||
| return; | ||
| } | ||
|
|
||
| // Use setTimeout to ensure blocks are fully rendered before selecting | ||
| const timeoutId = setTimeout( () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm guessing the timeout is needed because this code needs to run after the We should consider whether we should add a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, maybe checking if the block is still the tree? Pretty edge casey I guess. I'm just thinking if block is deleted or changed (collaborative editing?) since navigation
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If that happens nothing gets selected, which should be fine.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand. What would be the right place to select the block if not the Editor component?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The component responsible for actually inserting the blocks into the canvas aka
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean passing the previously selected block as the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly an alternative to If the store isn't ready, it returns Once blocks are available, blocks becomes an array and triggers the Would that be more idiomatic to React? // Check if blocks are available in the store
const blocks = useSelect(
( select ) => {
if ( ! hasLoadedPost || ! post ) {
return null;
}
// Try to get blocks - returns empty array if not ready
try {
return select( blockEditorStore ).getBlocks();
} catch {
return null;
}
},
[ hasLoadedPost, post ]
);
// Restore initial block selection when blocks are available
useEffect( () => {
if ( ! initialSelection || ! hasLoadedPost || ! post || ! blocks ) {
return;
}
// Blocks are now available, try to restore selection
const clientId = restoreBlockFromPath( initialSelection );
if ( clientId ) {
selectBlock( clientId );
}
}, [
initialSelection,
hasLoadedPost,
post,
blocks,
selectBlock,
restoreBlockFromPath,
] );
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's cheaper to use a timeout than query the store for blocks 😅 Maybe the need for the timeout points to underlying issues.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, fair call. 👍🏻 |
||
| const clientId = restoreBlockFromPath( initialSelection ); | ||
| if ( clientId ) { | ||
| selectBlock( clientId ); | ||
| } | ||
| }, 0 ); | ||
|
|
||
| return () => clearTimeout( timeoutId ); | ||
| }, [ | ||
| initialSelection, | ||
| hasLoadedPost, | ||
| post, | ||
| selectBlock, | ||
| restoreBlockFromPath, | ||
| ] ); | ||
|
|
||
| return ( | ||
| <> | ||
| { hasLoadedPost && ! post && ( | ||
|
|
||
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.
The one thing that I'm uncertain about is that we pass "postId" to the
Editorcomponent. This means the "blocks" property of the "post" might or might not be present (the post might not have parsed the blocks yet).I guess we're making an assumption that the the blocks are already parsed and clientIds stable. IT seems ok in the context of entity navigation but wanted to highlight this.