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

Jump to bottom

Restores setting zoom out mode to useZoomOut hook #65999

Merged
merged 2 commits into from
Oct 14, 2024
Merged

Restores setting zoom out mode to useZoomOut hook #65999

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7930be8
Restores setting zoom out mode to useZoomOut hook
jeryj Oct 9, 2024
80a63cb
Fix test
talldan Oct 14, 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
4 changes: 2 additions & 2 deletions packages/block-editor/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,11 +1077,11 @@ _Parameters_

### useZoomOut

A hook used to set the zoomed out view, invoking the hook sets the mode.
A hook used to set the editor mode to zoomed out mode, invoking the hook sets the mode.
Copy link
Contributor

Choose a reason for hiding this comment

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

To note that the "zoom out mode" is already replaced by edit mode.


_Parameters_

- _zoomOut_ `boolean`: If we should zoom out or not.
- _zoomOut_ `boolean`: If we should enter into zoomOut mode or not

### Warning

Expand Down
56 changes: 36 additions & 20 deletions packages/block-editor/src/hooks/use-zoom-out.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,55 @@ import { useEffect, useRef } from '@wordpress/element';
*/
import { store as blockEditorStore } from '../store';
import { unlock } from '../lock-unlock';

/**
* A hook used to set the zoomed out view, invoking the hook sets the mode.
* A hook used to set the editor mode to zoomed out mode, invoking the hook sets the mode.
*
* @param {boolean} zoomOut If we should zoom out or not.
* @param {boolean} zoomOut If we should enter into zoomOut mode or not
*/
export function useZoomOut( zoomOut = true ) {
const { setZoomLevel } = unlock( useDispatch( blockEditorStore ) );
const { isZoomOut } = unlock( useSelect( blockEditorStore ) );
const { __unstableSetEditorMode, setZoomLevel } = unlock(
useDispatch( blockEditorStore )
);
const { __unstableGetEditorMode } = unlock( useSelect( blockEditorStore ) );

const originalIsZoomOutRef = useRef( null );
const originalEditingModeRef = useRef( null );
const mode = __unstableGetEditorMode();

useEffect( () => {
// Only set this on mount so we know what to return to when we unmount.
if ( ! originalIsZoomOutRef.current ) {
originalIsZoomOutRef.current = isZoomOut();
if ( ! originalEditingModeRef.current ) {
originalEditingModeRef.current = mode;
}

// The effect opens the zoom-out view if we want it open and the canvas is not currently zoomed-out.
if ( zoomOut && isZoomOut() === false ) {
return () => {
// We need to use __unstableGetEditorMode() here and not `mode`, as mode may not update on unmount
if (
__unstableGetEditorMode() === 'zoom-out' &&
__unstableGetEditorMode() !== originalEditingModeRef.current
) {
__unstableSetEditorMode( originalEditingModeRef.current );
setZoomLevel( 100 );
}
};
}, [] );

// The effect opens the zoom-out view if we want it open and it's not currently in zoom-out mode.
useEffect( () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I came across this PR while searching for the cause of #66205. I feel like we need to check that it's not a mobile layout here, otherwise the width won't be set correctly when we open the Patterns tab in mobile view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we'll need to wait and see if #66141 gets backported. However, @PARTHVATALIYA's idea to add it to the inserter useZoomOut conditional should work well for either case. If that route will fix the issue, I think that would work well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming that we are not intending to backport #66141

if ( zoomOut && mode !== 'zoom-out' ) {
__unstableSetEditorMode( 'zoom-out' );
setZoomLevel( 50 );
} else if (
! zoomOut &&
isZoomOut() &&
originalIsZoomOutRef.current !== isZoomOut()
__unstableGetEditorMode() === 'zoom-out' &&
originalEditingModeRef.current !== mode
) {
setZoomLevel( originalIsZoomOutRef.current ? 50 : 100 );
__unstableSetEditorMode( originalEditingModeRef.current );
setZoomLevel( 100 );
}

return () => {
if ( isZoomOut() && isZoomOut() !== originalIsZoomOutRef.current ) {
setZoomLevel( originalIsZoomOutRef.current ? 50 : 100 );
}
};
}, [ isZoomOut, setZoomLevel, zoomOut ] );
}, [
__unstableGetEditorMode,
__unstableSetEditorMode,
zoomOut,
setZoomLevel,
] ); // Mode is deliberately excluded from the dependencies so that the effect does not run when mode changes.
}
Loading