diff --git a/packages/block-editor/src/hooks/use-zoom-out.js b/packages/block-editor/src/hooks/use-zoom-out.js index bcf5d9ff882f7..adcea8b605aeb 100644 --- a/packages/block-editor/src/hooks/use-zoom-out.js +++ b/packages/block-editor/src/hooks/use-zoom-out.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { useSelect, useDispatch } from '@wordpress/data'; -import { useEffect } from '@wordpress/element'; +import { useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -12,32 +12,64 @@ import { unlock } from '../lock-unlock'; /** * A hook used to set the editor mode to zoomed out mode, invoking the hook sets the mode. + * Concepts: + * - If we most recently changed the zoom level for them (in or out), we always resetZoomLevel() level when unmounting. + * - If the user most recently changed the zoom level (manually toggling), we do nothing when unmounting. * - * @param {boolean} zoomOut If we should enter into zoomOut mode or not + * @param {boolean} enabled If we should enter into zoomOut mode or not */ -export function useZoomOut( zoomOut = true ) { +export function useZoomOut( enabled = true ) { const { setZoomLevel, resetZoomLevel } = unlock( useDispatch( blockEditorStore ) ); - const { isZoomOut } = unlock( useSelect( blockEditorStore ) ); + /** + * We need access to both the value and the function. The value is to trigger a useEffect hook + * and the function is to check zoom out within another hook without triggering a re-render. + */ + const { isZoomedOut, isZoomOut } = useSelect( ( select ) => { + const { isZoomOut: _isZoomOut } = unlock( select( blockEditorStore ) ); + return { + isZoomedOut: _isZoomOut(), + isZoomOut: _isZoomOut, + }; + }, [] ); + + const controlZoomLevelRef = useRef( false ); + const isEnabledRef = useRef( enabled ); + + /** + * This hook tracks if the zoom state was changed manually by the user via clicking + * the zoom out button. We only want this to run when isZoomedOut changes, so we use + * a ref to track the enabled state. + */ useEffect( () => { - const isZoomOutOnMount = isZoomOut(); + // 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 while + // this hook was mounted, and we should no longer control the zoom state. + if ( isZoomedOut !== isEnabledRef.current ) { + controlZoomLevelRef.current = false; + } + }, [ isZoomedOut ] ); - return () => { - if ( isZoomOutOnMount ) { + useEffect( () => { + isEnabledRef.current = enabled; + + if ( enabled !== isZoomOut() ) { + controlZoomLevelRef.current = true; + + if ( enabled ) { setZoomLevel( 'auto-scaled' ); } else { resetZoomLevel(); } - }; - }, [] ); - - useEffect( () => { - if ( zoomOut ) { - setZoomLevel( 'auto-scaled' ); - } else { - resetZoomLevel(); } - }, [ zoomOut, setZoomLevel, resetZoomLevel ] ); + + return () => { + // If we are controlling zoom level and are zoomed out, reset the zoom level. + if ( controlZoomLevelRef.current && isZoomOut() ) { + resetZoomLevel(); + } + }; + }, [ enabled, isZoomOut, resetZoomLevel, setZoomLevel ] ); } diff --git a/test/e2e/specs/site-editor/preload.spec.js b/test/e2e/specs/site-editor/preload.spec.js index 2cd61283fbd9e..e731e932e3052 100644 --- a/test/e2e/specs/site-editor/preload.spec.js +++ b/test/e2e/specs/site-editor/preload.spec.js @@ -6,6 +6,7 @@ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'Preload', () => { test.beforeAll( async ( { requestUtils } ) => { await requestUtils.activateTheme( 'emptytheme' ); + await requestUtils.resetPreferences(); } ); test.afterAll( async ( { requestUtils } ) => { diff --git a/test/e2e/specs/site-editor/site-editor-inserter.spec.js b/test/e2e/specs/site-editor/site-editor-inserter.spec.js index 04075cbedab30..a730367d841bf 100644 --- a/test/e2e/specs/site-editor/site-editor-inserter.spec.js +++ b/test/e2e/specs/site-editor/site-editor-inserter.spec.js @@ -5,8 +5,9 @@ const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); test.describe( 'Site Editor Inserter', () => { test.beforeAll( async ( { requestUtils } ) => { + // We need the theme to have a section root so zoom out is enabled await Promise.all( [ - requestUtils.activateTheme( 'emptytheme' ), + requestUtils.activateTheme( 'twentytwentyfour' ), requestUtils.deleteAllTemplates( 'wp_template' ), requestUtils.deleteAllTemplates( 'wp_template_part' ), ] ); @@ -21,47 +22,303 @@ test.describe( 'Site Editor Inserter', () => { await editor.canvas.locator( 'body' ).click(); } ); + test.use( { + InserterUtils: async ( { editor, page }, use ) => { + await use( new InserterUtils( { editor, page } ) ); + }, + } ); + + // eslint-disable-next-line playwright/expect-expect test( 'inserter toggle button should toggle global inserter', async ( { - page, + InserterUtils, } ) => { - await page.click( 'role=button[name="Block Inserter"i]' ); - - // Visibility check - await expect( - page.locator( 'role=searchbox[name="Search"i]' ) - ).toBeVisible(); - await page.click( 'role=button[name="Block Inserter"i]' ); - //Hidden State check - await expect( - page.locator( 'role=searchbox[name="Search"i]' ) - ).toBeHidden(); + await InserterUtils.openBlockLibrary(); + await InserterUtils.closeBlockLibrary(); } ); // A test for https://github.com/WordPress/gutenberg/issues/43090. test( 'should close the inserter when clicking on the toggle button', async ( { - page, editor, + InserterUtils, } ) => { - const inserterButton = page.getByRole( 'button', { - name: 'Block Inserter', - exact: true, - } ); - const blockLibrary = page.getByRole( 'region', { - name: 'Block Library', - } ); - const beforeBlocks = await editor.getBlocks(); - await inserterButton.click(); - await blockLibrary.getByRole( 'tab', { name: 'Blocks' } ).click(); - await blockLibrary.getByRole( 'option', { name: 'Buttons' } ).click(); + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await InserterUtils.blockLibrary + .getByRole( 'option', { name: 'Buttons' } ) + .click(); await expect .poll( editor.getBlocks ) .toMatchObject( [ ...beforeBlocks, { name: 'core/buttons' } ] ); - await inserterButton.click(); + await InserterUtils.closeBlockLibrary(); + } ); + + test.describe( 'Inserter Zoom Level UX', () => { + test.use( { + ZoomUtils: async ( { editor, page }, use ) => { + await use( new ZoomUtils( { editor, page } ) ); + }, + } ); + + test( 'should intialize correct active tab based on zoom level', async ( { + InserterUtils, + ZoomUtils, + } ) => { + await test.step( 'should open the inserter to blocks tab from default zoom level', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + + // Zoom canvas should not be active + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom canvas should not be active + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + + await test.step( 'should open the inserter to patterns tab if zoomed out', async () => { + await ZoomUtils.enterZoomOut(); + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Patterns' ); + + // Zoom canvas should still be active + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + await InserterUtils.closeBlockLibrary(); + + // We should still be in Zoom Out + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + } ); + } ); + + test( 'should set the correct zoom level when changing tabs', async ( { + InserterUtils, + ZoomUtils, + } ) => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await test.step( 'should zoom out when activating patterns tab', async () => { + await InserterUtils.activateTab( 'Patterns' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + } ); + + await test.step( 'should reset zoom level when activating blocks tab', async () => { + await InserterUtils.activateTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + + await test.step( 'should zoom out when activating media tab', async () => { + await InserterUtils.activateTab( 'Media' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + } ); + } ); + + test( 'should reset the zoom level when closing the inserter if we most recently changed the zoom level', async ( { + InserterUtils, + ZoomUtils, + } ) => { + await test.step( 'should reset zoom when closing from patterns tab', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Patterns' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom Level should be reset + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + + await test.step( 'should preserve default zoom level when closing from blocks tab', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Media' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + await InserterUtils.activateTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom Level should stay at default level + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + + await test.step( 'should preserve default zoom level when closing from blocks tab even if user manually toggled zoom level on previous tab', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Media' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + // Toggle zoom level manually + await ZoomUtils.exitZoomOut(); + + await InserterUtils.activateTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom Level should stay at default level + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); - await expect( blockLibrary ).toBeHidden(); + await test.step( 'should preserve default zoom level when closing from blocks tab even if user manually toggled zoom level on previous tab twice', async () => { + // Open inserter + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Media' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + // Toggle zoom level manually twice + await ZoomUtils.exitZoomOut(); + await ZoomUtils.enterZoomOut(); + + await InserterUtils.activateTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom Level should stay at default level + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + } ); + + test( 'should keep the zoom level manually set by the user if the user most recently set the zoom level', async ( { + InserterUtils, + ZoomUtils, + } ) => { + await test.step( 'should respect manual zoom level set when closing from patterns tab', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Patterns' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + await ZoomUtils.exitZoomOut(); + + await InserterUtils.closeBlockLibrary(); + + // Zoom Level should stay reset + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + } ); + + await test.step( 'should respect manual zoom level set when closing from patterns tab when toggled twice', async () => { + await InserterUtils.openBlockLibrary(); + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await InserterUtils.activateTab( 'Patterns' ); + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + await ZoomUtils.exitZoomOut(); + + await ZoomUtils.enterZoomOut(); + + await InserterUtils.closeBlockLibrary(); + + // Should stay zoomed out since it was manually engaged + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + + // Reset test + await ZoomUtils.exitZoomOut(); + } ); + + await test.step( 'should not reset zoom level if zoom level manually changed from blocks tab', async () => { + await InserterUtils.openBlockLibrary(); + await expect( InserterUtils.blockLibrary ).toBeVisible(); + + await InserterUtils.expectActiveTab( 'Blocks' ); + await expect( ZoomUtils.zoomCanvas ).toBeHidden(); + + await ZoomUtils.enterZoomOut(); + + await InserterUtils.closeBlockLibrary(); + + // Should stay zoomed out since it was manually engaged + await expect( ZoomUtils.zoomCanvas ).toBeVisible(); + } ); + } ); } ); } ); + +class InserterUtils { + constructor( { editor, page } ) { + this.editor = editor; + this.page = page; + this.blockLibrary = this.page.getByRole( 'region', { + name: 'Block Library', + } ); + this.inserterButton = this.page.getByRole( 'button', { + name: 'Block Inserter', + exact: true, + } ); + } + + // Manually naming as open and close these makes it clearer when reading + // through the test instead of using a toggle method with a boolean + async openBlockLibrary() { + await this.inserterButton.click(); + await expect( this.blockLibrary ).toBeVisible(); + } + + async closeBlockLibrary() { + await this.inserterButton.click(); + await expect( this.blockLibrary ).toBeHidden(); + } + + getBlockLibraryTab( name ) { + return this.page.getByRole( 'tab', { name } ); + } + + async expectActiveTab( name ) { + await expect( this.getBlockLibraryTab( name ) ).toHaveAttribute( + 'aria-selected', + 'true' + ); + } + + async activateTab( name ) { + await this.getBlockLibraryTab( name ).click(); + // For brevity, adding this check here. It should always be done after the tab is clicked + await this.expectActiveTab( name ); + } +} + +class ZoomUtils { + constructor( { editor, page } ) { + this.editor = editor; + this.page = page; + this.zoomCanvas = this.page.locator( '.is-zoomed-out' ); + this.zoomButton = this.page.getByRole( 'button', { + name: 'Zoom Out', + exact: true, + } ); + } + + // Manually naming as enter and exit these makes it clearer when reading + // through the test instead of using a toggle method with a boolean + async enterZoomOut() { + await this.zoomButton.click(); + await expect( this.zoomCanvas ).toBeVisible(); + } + + async exitZoomOut() { + await this.zoomButton.click(); + await expect( this.zoomCanvas ).toBeHidden(); + } +}