Skip to content

Commit

Permalink
Only control zoom level in useZoomOut if starting from non-zoomed state
Browse files Browse the repository at this point in the history 
This modifies the idea of the useZoomOut state to allow for a controlled mode. If the user begins in zoom out or manually toggles the zoom level, then we assume they are a user who understands the zoom levels and doesn't need this toggled for them. If they begin from a non-zoomed state, we can zoom in when they reach the patterns tab and remove the zoom level when they exit the inserter. If they use the zoom button when the inserter is open, this also removes the control mode, as they understand how to enter and exit zoom out.

The idea here is:
- it is jarring to have the zoom level automatically change for you, so default to not changing it unless it seems like the user doesn't know how to use the zoom button.
- have a smart default, but allow overrides
  • Loading branch information
@jeryj
jeryj committed Dec 3, 2024
1 parent ae81003 commit f0a717f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 57 deletions.
28 changes: 18 additions & 10 deletions packages/block-editor/src/hooks/use-zoom-out.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,21 @@ export function useZoomOut( zoomOut = true ) {
const { isZoomOut } = unlock( useSelect( blockEditorStore ) );

const toggleZoomOnUnmount = useRef( false );
const userZoomState = useRef( null );
const zoomStateOnMount = useRef( null );
const isZoomedOut = isZoomOut();
const controlZoomLevel = useRef( null );

useEffect( () => {
// Store the user's manually set zoom state on mount.
userZoomState.current = isZoomOut();
zoomStateOnMount.current = isZoomOut();

// If they start in a zoomed out state, then do not control the zoom state.
// This user knows about the zoom levels and can toggle it when they want,
// or they have manually toggled it on and want to stay there.
controlZoomLevel.current = ! isZoomOut();

return () => {
if ( ! toggleZoomOnUnmount.current ) {
if ( ! controlZoomLevel.current || ! toggleZoomOnUnmount.current ) {
return;
}

Expand All @@ -47,11 +53,12 @@ export function useZoomOut( zoomOut = true ) {
* update it when isZoomedOut changes.
*/
useEffect( () => {
// Requested zoom and current zoom states are different, so toggle the state.
if ( zoomOut !== isZoomedOut ) {
// If we are in controlled mode and the requested zoom and current zoom states
// are different, toggle the zoom state.
if ( controlZoomLevel.current && zoomOut !== isZoomedOut ) {
// If the requested zoomOut matches the user's manually set zoom state,
// do not toggle the zoom level on unmount.
if ( userZoomState.current === zoomOut ) {
if ( zoomStateOnMount.current === zoomOut ) {
toggleZoomOnUnmount.current = false;
} else {
toggleZoomOnUnmount.current = true;
Expand All @@ -63,6 +70,7 @@ export function useZoomOut( zoomOut = true ) {
setZoomLevel( 'auto-scaled' );
}
}

// Intentionally excluding isZoomedOut so this hook will not recursively udpate.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [ zoomOut, setZoomLevel, resetZoomLevel ] );
Expand All @@ -73,11 +81,11 @@ export function useZoomOut( zoomOut = true ) {
*/
useEffect( () => {
// If the zoom state changed (isZoomOut) and it does not match the requested zoom
// state (zoomOut), then it means the user manually changed the zoom state and we should
// not toggle the zoom level on unmount.
// state (zoomOut), then it means the user manually changed the zoom state while
// this hook was mounted, and we should no longer control the zoom state.
if ( isZoomedOut !== zoomOut ) {
toggleZoomOnUnmount.current = false;
userZoomState.current = zoomOut;
// Turn off all automatic zooming control.
controlZoomLevel.current = false;
}

// Intentionally excluding zoomOut from the dependency array. We want to catch instances where
Expand Down
51 changes: 4 additions & 47 deletions test/e2e/specs/site-editor/site-editor-inserter.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ test.describe( 'Site Editor Inserter', () => {
await expect( await InserterUtils.getZoomCanvas() ).toBeHidden();
} );

test( 'should return you to zoom out if starting from zoom out', async ( {
test( 'should not toggle zoom state between tabs if starting from zoom out', async ( {
InserterUtils,
} ) => {
const zoomOutButton = InserterUtils.getZoomOutButton();
Expand Down Expand Up @@ -147,16 +147,14 @@ test.describe( 'Site Editor Inserter', () => {
await blocksTab.click();
await expect( blocksTab ).toHaveAttribute( 'data-active-item', 'true' );

// Zoom out should disengage
await expect( await InserterUtils.getZoomCanvas() ).toBeHidden();
// Zoom out should still be egnaged
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();

// Close the inserter
await inserterButton.click();
await expect( blockLibrary ).toBeHidden();

// We should return to zoom out since the inserter was opened with
// zoom out engaged, and it was automatically disengaged when selecting
// the blocks tab
// We should stay in zoom out state since it was manually engaged from the start
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();
} );

Expand Down Expand Up @@ -226,47 +224,6 @@ test.describe( 'Site Editor Inserter', () => {
// We should stay in zoomed out state since it was manually engaged
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();
} );

// Covers bug found in https://github.com/WordPress/gutenberg/pull/66381#issuecomment-2441540851
test( 'should return to initial zoom out state after switching between multiple tabs', async ( {
InserterUtils,
} ) => {
const zoomOutButton = InserterUtils.getZoomOutButton();
const inserterButton = InserterUtils.getInserterButton();
const blockLibrary = InserterUtils.getBlockLibrary();

await zoomOutButton.click();
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();

await inserterButton.click();
const patternsTab = InserterUtils.getBlockLibraryTab( 'Patterns' );
const blocksTab = InserterUtils.getBlockLibraryTab( 'Blocks' );
const mediaTab = InserterUtils.getBlockLibraryTab( 'Media' );

// Should start with pattern tab selected in zoom out state
await expect( patternsTab ).toHaveAttribute(
'data-active-item',
'true'
);
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();

// Go to blocks tab which should exit zoom out
await blocksTab.click();
await expect( blocksTab ).toHaveAttribute( 'data-active-item', 'true' );
await expect( await InserterUtils.getZoomCanvas() ).toBeHidden();

// Go to media tab which should enter zoom out again since that's the starting state
await mediaTab.click();
await expect( mediaTab ).toHaveAttribute( 'data-active-item', 'true' );
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();

// Close the inserter
await inserterButton.click();
await expect( blockLibrary ).toBeHidden();

// We should re-enter zoomed out state since it was the starting point
await expect( await InserterUtils.getZoomCanvas() ).toBeVisible();
} );
} );

class InserterUtils {
Expand Down

0 comments on commit f0a717f

Please sign in to comment.