Skip to content

Commit

Permalink
Make notices push down content
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Joen Asmussen committed Jan 31, 2019
1 parent 3fd2218 commit 9deb5de
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 24 deletions.
6 changes: 3 additions & 3 deletions assets/stylesheets/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,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,
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/notice/list.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* External dependencies
*/
import classnames from 'classnames';
import { noop, omit } from 'lodash';

/**
Expand All @@ -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 (
<div className={ className }>
{ children }
Expand Down
26 changes: 26 additions & 0 deletions packages/components/src/notice/test/list.js
Original file line number Diff line number Diff line change
@@ -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( <NoticeList notices={ [] } className="is-ok" /> );

const classes = new TokenList( renderer.getRenderOutput().props.className );
expect( classes.contains( 'is-ok' ) ).toBe( true );
expect( classes.contains( 'components-notice-list' ) ).toBe( true );
} );
} );
3 changes: 2 additions & 1 deletion packages/edit-post/src/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ function Layout( {
aria-label={ __( 'Editor content' ) }
tabIndex="-1"
>
<EditorNotices />
<EditorNotices dismissible={ false } className="is-pinned" />
<EditorNotices dismissible={ true } />
<PreserveScrollInReorder />
<EditorModeKeyboardShortcuts />
<KeyboardShortcutHelpModal />
Expand Down
39 changes: 23 additions & 16 deletions packages/edit-post/src/components/layout/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 14 additions & 3 deletions packages/editor/src/components/editor-notices/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* External dependencies
*/
import { filter } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -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 (
<NoticeList { ...props }>
<TemplateValidationNotice />
<NoticeList notices={ notices } { ...props }>
{ dismissible !== false && (
<TemplateValidationNotice />
) }
</NoticeList>
);
}
Expand Down
35 changes: 35 additions & 0 deletions packages/editor/src/components/editor-notices/test/index.js
Original file line number Diff line number Diff line change
@@ -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( <EditorNotices notices={ notices } /> );
expect( wrapper.prop( 'notices' ) ).toHaveLength( 3 );
expect( wrapper.children() ).toHaveLength( 1 );
} );

it( 'renders only dismissible notices', () => {
const wrapper = shallow( <EditorNotices notices={ notices } dismissible={ true } /> );
expect( wrapper.prop( 'notices' ) ).toHaveLength( 2 );
expect( wrapper.children() ).toHaveLength( 1 );
} );

it( 'renders only non-dismissible notices', () => {
const wrapper = shallow( <EditorNotices notices={ notices } dismissible={ false } /> );
expect( wrapper.prop( 'notices' ) ).toHaveLength( 1 );
expect( wrapper.children() ).toHaveLength( 0 );
} );
} );

0 comments on commit 9deb5de

Please sign in to comment.