Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make notices push down content #13614

Merged
merged 3 commits into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 );
} );
} );