From 33ddecfc2ae97b60b694aabd1f1534077b9883da Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Thu, 31 Jan 2019 11:55:18 +0100 Subject: [PATCH] Make notices push down content This PR restores the good stuff from #12301. That is: it allows notices to push down content. Dismissible notices are sticky at the top, non-dismisible notices scroll out of view. This is mostly an exact copy of the other PR, but fresh. The behavior has a number of benefits: - If you have multiple non-dismissible notices, you can still actually use the editor. - Notices no longer cover the scrollbar. - Notices no longer cover the permalink interface. - Notices now only cover content if you do not dismiss the notices. --- assets/stylesheets/_z-index.scss | 6 +-- packages/components/src/notice/list.js | 5 ++- packages/components/src/notice/test/list.js | 26 +++++++++++++ .../edit-post/src/components/layout/index.js | 3 +- .../src/components/layout/style.scss | 39 +++++++++++-------- .../src/components/editor-notices/index.js | 17 ++++++-- .../components/editor-notices/test/index.js | 35 +++++++++++++++++ 7 files changed, 107 insertions(+), 24 deletions(-) create mode 100644 packages/components/src/notice/test/list.js create mode 100644 packages/editor/src/components/editor-notices/test/index.js diff --git a/assets/stylesheets/_z-index.scss b/assets/stylesheets/_z-index.scss index b8f00482e84af7..462824c88c578c 100644 --- a/assets/stylesheets/_z-index.scss +++ b/assets/stylesheets/_z-index.scss @@ -67,9 +67,9 @@ $z-layers: ( // but bellow #adminmenuback { z-index: 100 } ".edit-post-sidebar {greater than small}": 90, - // Show notices below expanded wp-admin submenus: - // #adminmenuwrap { z-index: 9990 } - ".components-notice-list": 9989, + // Show notices below expanded editor bar + // .edit-post-header { z-index: 30 } + ".components-notice-list": 29, // Show modal under the wp-admin menus and the popover ".components-modal__screen-overlay": 100000, diff --git a/packages/components/src/notice/list.js b/packages/components/src/notice/list.js index 4ff12e2a451181..dcb39dc49b5828 100644 --- a/packages/components/src/notice/list.js +++ b/packages/components/src/notice/list.js @@ -1,6 +1,7 @@ /** * External dependencies */ +import classnames from 'classnames'; import { noop, omit } from 'lodash'; /** @@ -18,9 +19,11 @@ import Notice from './'; * @param {Object} $0.children Array of children to be rendered inside the notice list. * @return {Object} The rendered notices list. */ -function NoticeList( { notices, onRemove = noop, className = 'components-notice-list', children } ) { +function NoticeList( { notices, onRemove = noop, className, children } ) { const removeNotice = ( id ) => () => onRemove( id ); + className = classnames( 'components-notice-list', className ); + return (
{ children } diff --git a/packages/components/src/notice/test/list.js b/packages/components/src/notice/test/list.js new file mode 100644 index 00000000000000..034ff9b7dfd828 --- /dev/null +++ b/packages/components/src/notice/test/list.js @@ -0,0 +1,26 @@ +/** + * External dependencies + */ +import ShallowRenderer from 'react-test-renderer/shallow'; + +/** + * WordPress dependencies + */ +import TokenList from '@wordpress/token-list'; + +/** + * Internal dependencies + */ +import NoticeList from '../list'; + +describe( 'NoticeList', () => { + it( 'should merge className', () => { + const renderer = new ShallowRenderer(); + + renderer.render( ); + + const classes = new TokenList( renderer.getRenderOutput().props.className ); + expect( classes.contains( 'is-ok' ) ).toBe( true ); + expect( classes.contains( 'components-notice-list' ) ).toBe( true ); + } ); +} ); diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index 1c67baed29d22c..f0876f46722872 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -78,7 +78,8 @@ function Layout( { aria-label={ __( 'Editor content' ) } tabIndex="-1" > - + + diff --git a/packages/edit-post/src/components/layout/style.scss b/packages/edit-post/src/components/layout/style.scss index f848a8cfd7ccd5..7671aa3788dbb6 100644 --- a/packages/edit-post/src/components/layout/style.scss +++ b/packages/edit-post/src/components/layout/style.scss @@ -13,18 +13,24 @@ color: $dark-gray-900; @include break-small { - position: fixed; - top: inherit; + top: 0; } - .components-notice { - margin: 0 0 5px; - padding: 6px 12px; - min-height: $panel-header-height; + // Non-dismissible notices. + &.is-pinned.is-pinned { + position: relative; + left: 0; + top: 0; + } + } - .components-notice__dismiss { - margin: 10px 5px; - } + .components-notice { + margin: 0 0 5px; + padding: 6px 12px; + min-height: $panel-header-height; + + .components-notice__dismiss { + margin: 10px 5px; } } @@ -53,9 +59,6 @@ } } -@include editor-left(".components-notice-list"); -@include editor-right(".components-notice-list"); - .edit-post-layout__metaboxes:not(:empty) { border-top: $border-width solid $light-gray-500; margin-top: 10px; @@ -127,10 +130,14 @@ padding-bottom: 0; } - // On mobile the main content area has to scroll otherwise you can invoke - // the overscroll bounce on the non-scrolling container, causing - // (ノಠ益ಠ)ノ彡┻━┻ - overflow-y: auto; + // On mobile the main content (html or body) area has to scroll. + // If, like we do on the desktop, scroll an element (.edit-post-layout__content) you can invoke + // the overscroll bounce on the non-scrolling container, causing for a frustrating scrolling experience. + // The following rule enables this scrolling beyond the mobile breakpoint, because on the desktop + // breakpoints scrolling an isolated element helps avoid scroll bleed. + @include break-small() { + overflow-y: auto; + } -webkit-overflow-scrolling: touch; // This rule ensures that if you've scrolled to the end of a container, diff --git a/packages/editor/src/components/editor-notices/index.js b/packages/editor/src/components/editor-notices/index.js index f11c76181cbb56..f899f4a61e0ff2 100644 --- a/packages/editor/src/components/editor-notices/index.js +++ b/packages/editor/src/components/editor-notices/index.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { filter } from 'lodash'; + /** * WordPress dependencies */ @@ -10,10 +15,16 @@ import { compose } from '@wordpress/compose'; */ import TemplateValidationNotice from '../template-validation-notice'; -function EditorNotices( props ) { +export function EditorNotices( { dismissible, notices, ...props } ) { + if ( dismissible !== undefined ) { + notices = filter( notices, { isDismissible: dismissible } ); + } + return ( - - + + { dismissible !== false && ( + + ) } ); } diff --git a/packages/editor/src/components/editor-notices/test/index.js b/packages/editor/src/components/editor-notices/test/index.js new file mode 100644 index 00000000000000..655a990aa174fe --- /dev/null +++ b/packages/editor/src/components/editor-notices/test/index.js @@ -0,0 +1,35 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; + +/** + * Internal dependencies + */ +import { EditorNotices } from '../'; + +describe( 'EditorNotices', () => { + const notices = [ + { content: 'Eat your vegetables!', isDismissible: true }, + { content: 'Brush your teeth!', isDismissible: true }, + { content: 'Existence is fleeting!', isDismissible: false }, + ]; + + it( 'renders all notices', () => { + const wrapper = shallow( ); + expect( wrapper.prop( 'notices' ) ).toHaveLength( 3 ); + expect( wrapper.children() ).toHaveLength( 1 ); + } ); + + it( 'renders only dismissible notices', () => { + const wrapper = shallow( ); + expect( wrapper.prop( 'notices' ) ).toHaveLength( 2 ); + expect( wrapper.children() ).toHaveLength( 1 ); + } ); + + it( 'renders only non-dismissible notices', () => { + const wrapper = shallow( ); + expect( wrapper.prop( 'notices' ) ).toHaveLength( 1 ); + expect( wrapper.children() ).toHaveLength( 0 ); + } ); +} );