Skip to content

Commit

Permalink
Make notices push down content (#13614)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
jasmussen authored and youknowriad committed Mar 6, 2019
1 parent 508a46c commit b892d6a
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 44 deletions.
6 changes: 3 additions & 3 deletions assets/stylesheets/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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
74 changes: 39 additions & 35 deletions packages/edit-post/src/components/layout/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion packages/editor/src/components/block-list/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
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 b892d6a

Please sign in to comment.