From b892d6a197703ea02a108db01455469fad4ff2d6 Mon Sep 17 00:00:00 2001 From: Joen Asmussen Date: Thu, 14 Feb 2019 07:22:33 +0100 Subject: [PATCH] Make notices push down content (#13614) * 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. * Address top toolbar issues. * Remove the overly specific is-pinned. --- 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 | 74 ++++++++++--------- .../src/components/block-list/style.scss | 1 - .../src/components/editor-notices/index.js | 17 ++++- .../components/editor-notices/test/index.js | 35 +++++++++ 8 files changed, 123 insertions(+), 44 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 b8f00482e84af..462824c88c578 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 4ff12e2a45118..dcb39dc49b582 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 0000000000000..034ff9b7dfd82 --- /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 1c67baed29d22..f0876f4672287 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 f848a8cfd7ccd..dfbfaf77dd356 100644 --- a/packages/edit-post/src/components/layout/style.scss +++ b/packages/edit-post/src/components/layout/style.scss @@ -13,49 +13,36 @@ color: $dark-gray-900; @include break-small { - position: fixed; - top: inherit; + top: 0; + } + + // Non-dismissible notices. + &.is-pinned { + position: relative; + left: 0; + top: 0; } + } - .components-notice { - margin: 0 0 5px; - padding: 6px 12px; - min-height: $panel-header-height; + .components-notice { + margin: 0 0 5px; + padding: 6px 12px; + min-height: $panel-header-height; - .components-notice__dismiss { - margin: 10px 5px; - } + .components-notice__dismiss { + margin: 10px 5px; } } - // On mobile, toolbars behave differently. + // Beyond the mobile breakpoint, the editor bar is fixed, so make room for it eabove the layout content. @include break-small { padding-top: $header-height; } - &.has-fixed-toolbar { - .edit-post-layout__content { - padding-top: $block-controls-height; - } - - // On mobile, toolbars behave differently. - @include break-small { - padding-top: $header-height + $block-toolbar-height; - - .edit-post-layout__content { - padding-top: 0; - } - } - - @include break-large { - padding-top: $header-height; - } - } + // Beyond the medium breakpoint, the main scrolling area itself becomes fixed so the padding then becomes + // unnecessary, but until then it's still needed. } -@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; @@ -121,16 +108,33 @@ } } + // For users with the Top Toolbar option enabled, special rules apply to the height of the content area. + .has-fixed-toolbar & { + // From the medium breakpoint it sits below the editor bar. + @include break-medium() { + top: $header-height + $admin-bar-height + $block-controls-height; + } + + // From the xlarge breakpoint it sits in the editor bar. + @include break-xlarge() { + top: $header-height + $admin-bar-height; + } + } + // Pad the scroll box so content on the bottom can be scrolled up. padding-bottom: 50vh; @include break-small { 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/block-list/style.scss b/packages/editor/src/components/block-list/style.scss index a024bb2198d06..043ddd2613607 100644 --- a/packages/editor/src/components/block-list/style.scss +++ b/packages/editor/src/components/block-list/style.scss @@ -784,7 +784,6 @@ .editor-block-list__block { .editor-block-contextual-toolbar { - position: sticky; z-index: z-index(".editor-block-contextual-toolbar"); white-space: nowrap; text-align: left; diff --git a/packages/editor/src/components/editor-notices/index.js b/packages/editor/src/components/editor-notices/index.js index f11c76181cbb5..f899f4a61e0ff 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 0000000000000..655a990aa174f --- /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 ); + } ); +} );