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.

Sign up for GitHub

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

Zoom Out: Use the block editor for insertion point data #63934

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Zoom Out: Use the block editor for insertion point data #63934

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5833b5
Zoom Out: Use the block editor for insertion point data
scruffian Jul 25, 2024
ed597f0
remove unlock
scruffian Jul 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 16 additions & 18 deletions packages/block-editor/src/components/block-tools/zoom-out-mode-inserters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';
import { useSelect, useDispatch } from '@wordpress/data';
import { useEffect, useState } from '@wordpress/element';

/**
Expand All @@ -16,32 +16,29 @@ function ZoomOutModeInserters() {
const [ isReady, setIsReady ] = useState( false );
const {
hasSelection,
blockInsertionPoint,
blockOrder,
insertionPoint,
blockInsertionPointVisible,
setInserterIsOpened,
sectionRootClientId,
selectedBlockClientId,
hoveredBlockClientId,
} = useSelect( ( select ) => {
const {
getSettings,
getBlockInsertionPoint,
getBlockOrder,
getSelectionStart,
getSelectedBlockClientId,
getHoveredBlockClientId,
isBlockInsertionPointVisible,
} = select( blockEditorStore );
const { sectionRootClientId: root } = unlock( getSettings() );
// To do: move ZoomOutModeInserters to core/editor.
// Or we perhaps we should move the insertion point state to the
// block-editor store. I'm not sure what it was ever moved to the editor
// store, because all the inserter components all live in the
// block-editor package.
// eslint-disable-next-line @wordpress/data-no-store-string-literals
const editor = select( 'core/editor' );
return {
hasSelection: !! getSelectionStart().clientId,
blockInsertionPoint: getBlockInsertionPoint(),
blockOrder: getBlockOrder( root ),
insertionPoint: unlock( editor ).getInsertionPoint(),
blockInsertionPointVisible: isBlockInsertionPointVisible(),
sectionRootClientId: root,
setInserterIsOpened:
getSettings().__experimentalSetIsInserterOpened,
Expand All @@ -50,6 +47,8 @@ function ZoomOutModeInserters() {
};
}, [] );

const blockEditorDispatch = useDispatch( blockEditorStore );

// Defer the initial rendering to avoid the jumps due to the animation.
useEffect( () => {
const timeout = setTimeout( () => {
Expand All @@ -65,14 +64,8 @@ function ZoomOutModeInserters() {
}

return [ undefined, ...blockOrder ].map( ( clientId, index ) => {
const shouldRenderInserter = insertionPoint.insertionIndex !== index;

const shouldRenderInsertionPoint =
insertionPoint.insertionIndex === index;

if ( ! shouldRenderInserter && ! shouldRenderInsertionPoint ) {
return null;
}
blockInsertionPointVisible && blockInsertionPoint.index === index;

const previousClientId = clientId;
const nextClientId = blockOrder[ index ];
Expand All @@ -86,6 +79,8 @@ function ZoomOutModeInserters() {
hoveredBlockClientId === previousClientId ||
hoveredBlockClientId === nextClientId;

const { showInsertionPoint } = unlock( blockEditorDispatch );
Copy link
Member

@Mamaduka Mamaduka Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can unlock the action once instead of doing it in a loop.

This should also work:

const { showInsertionPoint } = unlock( useDispatch( blockEditorStore ) );

Edit: The showInsertionPoint looks to be a public action. Why are we unlocking it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I did it earlier in the component I got this error from the linter:

Variables should not be assigned until just prior its first reference. An early return statement may leave this variable unused

And when it's later I get:

React Hook "useDispatch" is called conditionally. React Hooks must be called in the exact same order in every component render. Did you accidentally call a React Hook after an early return?

That's why I split it in two!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it only triggers that error when using unlock. It doesn't do it without it. Probably worth investigating separately.

Now that unlocking isn't needed, you can safely do const { showInsertionPoint } = useDispatch( blockEditorStore ); earlier in the component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scruffian, it looks like you already merged PR, so here's the quick follow-up - #63936

I'm also curious if you tested this with page content focus - #63934 (comment).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow up. I was keen to merge so that we can move forward with #63896.

I'll make sure to also look at page content focus on #63896.


return (
<BlockPopoverInbetween
key={ index }
Expand All @@ -104,7 +99,7 @@ function ZoomOutModeInserters() {
className="block-editor-block-list__insertion-point-indicator"
/>
) }
{ shouldRenderInserter && (
{ ! shouldRenderInsertionPoint && (
<ZoomOutModeInserterButton
isVisible={ isSelected || isHovered }
onClick={ () => {
Expand All @@ -114,6 +109,9 @@ function ZoomOutModeInserters() {
tab: 'patterns',
category: 'all',
} );
showInsertionPoint( sectionRootClientId, index, {
operation: 'insert',
} );
} }
/>
) }
Expand Down
Loading