From fe25e7dba812915481a51d4122f7be7f8ddcffe8 Mon Sep 17 00:00:00 2001 From: Grzegorz Ziolkowski Date: Wed, 2 Oct 2019 14:44:42 +0200 Subject: [PATCH 1/5] Plugins: Ensure sidebar plugins do not get auto-closed when opened on small screens --- .../editor-initialization/listener-hooks.js | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/edit-post/src/components/editor-initialization/listener-hooks.js b/packages/edit-post/src/components/editor-initialization/listener-hooks.js index 56d7761270eac4..51b1b2920e67b9 100644 --- a/packages/edit-post/src/components/editor-initialization/listener-hooks.js +++ b/packages/edit-post/src/components/editor-initialization/listener-hooks.js @@ -54,27 +54,35 @@ export const useBlockSelectionListener = ( postId ) => { * @param {number} postId The current post id. */ export const useAdjustSidebarListener = ( postId ) => { - const { isSmall, sidebarToReOpenOnExpand } = useSelect( + const { isSmall, activeGeneralSidebarName } = useSelect( ( select ) => ( { isSmall: select( 'core/viewport' ).isViewportMatch( '< medium' ), - sidebarToReOpenOnExpand: select( STORE_KEY ).getActiveGeneralSidebarName(), + activeGeneralSidebarName: select( STORE_KEY ).getActiveGeneralSidebarName(), } ), [ postId ] ); const { openGeneralSidebar, closeGeneralSidebar } = useDispatch( STORE_KEY ); - const previousOpenedSidebar = useRef( '' ); + const previousIsSmall = useRef( isSmall ); + const sidebarToReOpenOnExpand = useRef( null ); useEffect( () => { - if ( isSmall && sidebarToReOpenOnExpand ) { - previousOpenedSidebar.current = sidebarToReOpenOnExpand; - closeGeneralSidebar(); - } else if ( ! isSmall && previousOpenedSidebar.current ) { - openGeneralSidebar( previousOpenedSidebar.current ); - previousOpenedSidebar.current = ''; + if ( previousIsSmall.current === isSmall ) { + return; + } + previousIsSmall.current = isSmall; + + if ( isSmall ) { + sidebarToReOpenOnExpand.current = activeGeneralSidebarName; + if ( activeGeneralSidebarName ) { + closeGeneralSidebar(); + } + } else if ( sidebarToReOpenOnExpand.current && ! activeGeneralSidebarName ) { + openGeneralSidebar( sidebarToReOpenOnExpand.current ); + sidebarToReOpenOnExpand.current = null; } - }, [ isSmall, sidebarToReOpenOnExpand ] ); + }, [ isSmall, activeGeneralSidebarName ] ); }; /** From 8d15eb528298ba76e5fe67fc49af7470a564d3f0 Mon Sep 17 00:00:00 2001 From: Grzegorz Ziolkowski Date: Wed, 2 Oct 2019 15:16:11 +0200 Subject: [PATCH 2/5] Tests: Add e2e test which ensures that the plugin sidebar gets operned on medium screens --- .../src/set-browser-viewport.js | 1 + .../__snapshots__/plugins-api.test.js.snap | 2 ++ .../specs/plugins/plugins-api.test.js | 19 ++++++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/e2e-test-utils/src/set-browser-viewport.js b/packages/e2e-test-utils/src/set-browser-viewport.js index f6421215e00aa3..b35eaa58910247 100644 --- a/packages/e2e-test-utils/src/set-browser-viewport.js +++ b/packages/e2e-test-utils/src/set-browser-viewport.js @@ -11,6 +11,7 @@ import { waitForWindowDimensions } from './wait-for-window-dimensions'; export async function setBrowserViewport( type ) { const allowedDimensions = { large: { width: 960, height: 700 }, + medium: { width: 768, height: 700 }, small: { width: 600, height: 700 }, }; const currentDimension = allowedDimensions[ type ]; diff --git a/packages/e2e-tests/specs/plugins/__snapshots__/plugins-api.test.js.snap b/packages/e2e-tests/specs/plugins/__snapshots__/plugins-api.test.js.snap index 2faff688fbd77a..c2baa4270d086f 100644 --- a/packages/e2e-tests/specs/plugins/__snapshots__/plugins-api.test.js.snap +++ b/packages/e2e-tests/specs/plugins/__snapshots__/plugins-api.test.js.snap @@ -2,4 +2,6 @@ exports[`Using Plugins API Document Setting Custom Panel Should render a custom panel inside Document Setting sidebar 1`] = `"My Custom Panel"`; +exports[`Using Plugins API Sidebar Medium screen Should open plugins sidebar using More Menu item and render content 1`] = `"
(no title)
Sidebar title plugin
"`; + exports[`Using Plugins API Sidebar Should open plugins sidebar using More Menu item and render content 1`] = `"
(no title)
Sidebar title plugin
"`; diff --git a/packages/e2e-tests/specs/plugins/plugins-api.test.js b/packages/e2e-tests/specs/plugins/plugins-api.test.js index 904fcaf0914e0d..445216e5920a63 100644 --- a/packages/e2e-tests/specs/plugins/plugins-api.test.js +++ b/packages/e2e-tests/specs/plugins/plugins-api.test.js @@ -9,7 +9,7 @@ import { deactivatePlugin, openDocumentSettingsSidebar, openPublishPanel, - publishPost, + publishPost, setBrowserViewport, } from '@wordpress/e2e-test-utils'; describe( 'Using Plugins API', () => { @@ -75,6 +75,23 @@ describe( 'Using Plugins API', () => { const pluginSidebarClosed = await page.$( '.edit-post-sidebar' ); expect( pluginSidebarClosed ).toBeNull(); } ); + + describe( 'Medium screen', () => { + beforeAll( async () => { + await setBrowserViewport( 'medium' ); + } ); + + afterAll( async () => { + await setBrowserViewport( 'large' ); + } ); + + it( 'Should open plugins sidebar using More Menu item and render content', async () => { + await clickOnMoreMenuItem( 'Sidebar title plugin' ); + + const pluginSidebarContent = await page.$eval( '.edit-post-sidebar', ( el ) => el.innerHTML ); + expect( pluginSidebarContent ).toMatchSnapshot(); + } ); + } ); } ); describe( 'Document Setting Custom Panel', () => { From 0529f07d05d1dce1f3fce59146341d6ded4a94b4 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Wed, 2 Oct 2019 14:26:50 -0400 Subject: [PATCH 3/5] update tests due to new expectations. With the fix implemented in the tested hook, expecations for initial state of the hook changes since the effect bails early when the current state of `isSmall` from the viewport store matches the previous state stored on the ref. On initial render, this hook will always bail early. I also added some inline comments to help explain what is happening in the test and what is expected. --- .../test/listener-hooks.js | 44 ++++++++++++++++--- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/packages/edit-post/src/components/editor-initialization/test/listener-hooks.js b/packages/edit-post/src/components/editor-initialization/test/listener-hooks.js index 350a23f5b0ca39..87c2bc616e0bf5 100644 --- a/packages/edit-post/src/components/editor-initialization/test/listener-hooks.js +++ b/packages/edit-post/src/components/editor-initialization/test/listener-hooks.js @@ -137,10 +137,11 @@ describe( 'listener hook tests', () => { getSpyedFunction( STORE_KEY, 'closeGeneralSidebar' ) ).not.toHaveBeenCalled(); } ); - it( 'closes sidebar if viewport is small and there is an active ' + - 'sidebar name available', () => { + it( 'does not close sidebar if viewport is small and there is an active ' + + 'sidebar name available on initial render', () => { setMockReturnValue( 'core/viewport', 'isViewportMatch', true ); setMockReturnValue( STORE_KEY, 'getActiveGeneralSidebarName', 'foo' ); + // initial render does nothing (and sidebar will be closed already) act( () => { renderComponent( useAdjustSidebarListener, 10 ); } ); @@ -149,22 +150,53 @@ describe( 'listener hook tests', () => { ).not.toHaveBeenCalled(); expect( getSpyedFunction( STORE_KEY, 'closeGeneralSidebar' ) - ).toHaveBeenCalledTimes( 1 ); + ).not.toHaveBeenCalled(); } ); - it( 'opens sidebar if viewport is not small, and there is a cached sidebar to ' + - 'reopen on expand', () => { + it( 'closes sidebar if viewport is small and there is an active ' + + 'sidebar name available when viewport size changes', () => { + setMockReturnValue( 'core/viewport', 'isViewportMatch', false ); + setMockReturnValue( STORE_KEY, 'getActiveGeneralSidebarName', 'foo' ); + // initial render does nothing and sidebar will be open already. + act( () => { + renderComponent( useAdjustSidebarListener, 10 ); + } ); setMockReturnValue( 'core/viewport', 'isViewportMatch', true ); + // This render should result in the sidebar closing because viewport is + // now small triggering a change. + act( () => { + subscribeTrigger(); + } ); + expect( + getSpyedFunction( STORE_KEY, 'openGeneralSidebar' ) + ).not.toHaveBeenCalled(); + expect( + getSpyedFunction( STORE_KEY, 'closeGeneralSidebar' ) + ).toHaveBeenCalledTimes( 1 ); + } ); + it( 'opens sidebar if viewport is not small, and there is a cached sidebar ' + + 'to reopen on expand', () => { + setMockReturnValue( 'core/viewport', 'isViewportMatch', false ); setMockReturnValue( STORE_KEY, 'getActiveGeneralSidebarName', 'foo' ); + // initial render does nothing and sidebar should be open. act( () => { renderComponent( useAdjustSidebarListener, 10 ); } ); + setMockReturnValue( 'core/viewport', 'isViewportMatch', true ); + setMockReturnValue( STORE_KEY, 'getActiveGeneralSidebarName', 'bar' ); + // next render should close the sidebar and active sidebar at time of + // closing is cached. + act( () => { + subscribeTrigger(); + } ); setMockReturnValue( 'core/viewport', 'isViewportMatch', false ); + setMockReturnValue( STORE_KEY, 'getActiveGeneralSidebarName', '' ); + // next render should open the sidebar to the cached general sidebar name. act( () => { subscribeTrigger(); } ); expect( getSpyedFunction( STORE_KEY, 'openGeneralSidebar' ) - ).toHaveBeenCalledWith( 'foo' ); + ).toHaveBeenCalledWith( 'bar' ); expect( getSpyedFunction( STORE_KEY, 'openGeneralSidebar' ) ).toHaveBeenCalledTimes( 1 ); From b41396deb8233094cd1ff53289f6b3714ab7eef9 Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Wed, 2 Oct 2019 14:30:23 -0400 Subject: [PATCH 4/5] fix code style issue --- packages/e2e-tests/specs/plugins/plugins-api.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/plugins/plugins-api.test.js b/packages/e2e-tests/specs/plugins/plugins-api.test.js index 445216e5920a63..fedf3f25503c4d 100644 --- a/packages/e2e-tests/specs/plugins/plugins-api.test.js +++ b/packages/e2e-tests/specs/plugins/plugins-api.test.js @@ -9,7 +9,8 @@ import { deactivatePlugin, openDocumentSettingsSidebar, openPublishPanel, - publishPost, setBrowserViewport, + publishPost, + setBrowserViewport, } from '@wordpress/e2e-test-utils'; describe( 'Using Plugins API', () => { From e52c9426fd0c41132813590946fbced990a3c1ff Mon Sep 17 00:00:00 2001 From: Darren Ethier Date: Wed, 2 Oct 2019 14:34:03 -0400 Subject: [PATCH 5/5] Add changelog entry for edit-post package --- packages/edit-post/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/edit-post/CHANGELOG.md b/packages/edit-post/CHANGELOG.md index 2a17afc9ad02b8..69dd699b23426e 100644 --- a/packages/edit-post/CHANGELOG.md +++ b/packages/edit-post/CHANGELOG.md @@ -1,3 +1,9 @@ +## Master + +### Bug Fixes + +- Fix regression introduced by EditorInitializer component which auto-closed sidebar plugins when opened on small screens. ([#17712](https://github.com/WordPress/gutenberg/pull/17712)) + ## 3.6.0 (2019-08-05) ### Refactor