Skip to content

Commit

Permalink
Plugins: Ensure sidebar plugins do not get auto-closed when opened on… (
Browse files Browse the repository at this point in the history
#17712)

* Plugins: Ensure sidebar plugins do not get auto-closed when opened on small screens

* Tests: Add e2e test which ensures that the plugin sidebar gets operned on medium screens

* 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.

* fix code style issue

* Add changelog entry for edit-post package
  • Loading branch information
gziolo authored Oct 3, 2019
1 parent eb3a82f commit 8c0c7b7
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 16 deletions.
1 change: 1 addition & 0 deletions packages/e2e-test-utils/src/set-browser-viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`] = `"<div class=\\"components-panel__header edit-post-sidebar-header__small\\"><span class=\\"edit-post-sidebar-header__title\\">(no title)</span><button type=\\"button\\" aria-label=\\"Close plugin\\" class=\\"components-button components-icon-button\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></div><div class=\\"components-panel__header edit-post-sidebar-header\\"><strong>Sidebar title plugin</strong><button type=\\"button\\" aria-label=\\"Unpin from toolbar\\" aria-expanded=\\"true\\" class=\\"components-button components-icon-button is-toggled\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-star-filled\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M10 1l3 6 6 .75-4.12 4.62L16 19l-6-3-6 3 1.13-6.63L1 7.75 7 7z\\"></path></svg></button><button type=\\"button\\" aria-label=\\"Close plugin\\" class=\\"components-button components-icon-button\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></div><div class=\\"components-panel\\"><div class=\\"components-panel__body is-opened\\"><div class=\\"components-panel__row\\"><label for=\\"title-plain-text\\">Title:</label><textarea class=\\"editor-plain-text block-editor-plain-text\\" id=\\"title-plain-text\\" placeholder=\\"(no title)\\" rows=\\"1\\" style=\\"overflow: hidden; overflow-wrap: break-word; resize: none; height: 18px;\\"></textarea></div><div class=\\"components-panel__row\\"><button type=\\"button\\" class=\\"components-button is-button is-primary\\">Reset</button></div></div></div>"`;

exports[`Using Plugins API Sidebar Should open plugins sidebar using More Menu item and render content 1`] = `"<div class=\\"components-panel__header edit-post-sidebar-header__small\\"><span class=\\"edit-post-sidebar-header__title\\">(no title)</span><button type=\\"button\\" aria-label=\\"Close plugin\\" class=\\"components-button components-icon-button\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></div><div class=\\"components-panel__header edit-post-sidebar-header\\"><strong>Sidebar title plugin</strong><button type=\\"button\\" aria-label=\\"Unpin from toolbar\\" aria-expanded=\\"true\\" class=\\"components-button components-icon-button is-toggled\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-star-filled\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M10 1l3 6 6 .75-4.12 4.62L16 19l-6-3-6 3 1.13-6.63L1 7.75 7 7z\\"></path></svg></button><button type=\\"button\\" aria-label=\\"Close plugin\\" class=\\"components-button components-icon-button\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" class=\\"dashicon dashicons-no-alt\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></div><div class=\\"components-panel\\"><div class=\\"components-panel__body is-opened\\"><div class=\\"components-panel__row\\"><label for=\\"title-plain-text\\">Title:</label><textarea class=\\"editor-plain-text block-editor-plain-text\\" id=\\"title-plain-text\\" placeholder=\\"(no title)\\" rows=\\"1\\" style=\\"overflow: hidden; overflow-wrap: break-word; resize: none; height: 18px;\\"></textarea></div><div class=\\"components-panel__row\\"><button type=\\"button\\" class=\\"components-button is-button is-primary\\">Reset</button></div></div></div>"`;
18 changes: 18 additions & 0 deletions packages/e2e-tests/specs/plugins/plugins-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
openDocumentSettingsSidebar,
openPublishPanel,
publishPost,
setBrowserViewport,
} from '@wordpress/e2e-test-utils';

describe( 'Using Plugins API', () => {
Expand Down Expand Up @@ -75,6 +76,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', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/edit-post/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] );
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
} );
Expand All @@ -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 );
Expand Down

0 comments on commit 8c0c7b7

Please sign in to comment.