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

Try using snackbar notices instead of prominent ones for saving/failure notices #15594

Merged
merged 25 commits into from
May 29, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bd07cde
Try using snackbar notices instead of prominent ones for saving/failu…
youknowriad May 13, 2019
36f8419
Auto-remove the snackbar notice after 10 seconds
youknowriad May 14, 2019
7f6992e
No support for the HTML content in the snackbar notices
youknowriad May 16, 2019
b94bfcc
Status-less snackbar notices
youknowriad May 16, 2019
dde6c7a
Announce the notices as 'polite'
youknowriad May 16, 2019
0faf67d
Add a README for the snackbar component
youknowriad May 16, 2019
e0a1ea2
Update the changelogs
youknowriad May 16, 2019
cd90b4c
Avoid showing the error messages as snackbars
youknowriad May 16, 2019
a8f465e
Fix unit tests
youknowriad May 16, 2019
daefc52
Fix e2e tests
youknowriad May 16, 2019
24a0584
Animate the snackbar notifications
youknowriad May 17, 2019
95b919a
Revert "Animate the snackbar notifications"
youknowriad May 17, 2019
8d9ac40
Adding the snackbar component to the docs manifest
youknowriad May 17, 2019
cd0eafc
Remove the reversing of the notice list
youknowriad May 20, 2019
a566adf
Support snackbar onClick properly
youknowriad May 20, 2019
82a6994
Dismiss notice label
youknowriad May 22, 2019
8172262
Tweak the design/spacing/width of the notices
youknowriad May 22, 2019
c4ea9ab
Improve spacing of elements inside of snackbar.
sarahmonster May 22, 2019
2a28fd9
Ensure baselines line up properly and elements have the correct padding.
sarahmonster May 22, 2019
6f4a1d5
On mobile screens, show full-width snackbar.
sarahmonster May 22, 2019
266cff1
Attempt to centre snackbar notice within editing window.
sarahmonster May 22, 2019
b2b47ed
Ensure that multiple snackbars stack well.
sarahmonster May 22, 2019
b0b9fa4
Reduce max-width for better legibility of long lines.
sarahmonster May 22, 2019
fc5408c
Move positioning styles to layout file and nix unneeded styles.
sarahmonster May 23, 2019
12cde84
Restore left aligned snackbars
youknowriad May 29, 2019
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
4 changes: 4 additions & 0 deletions assets/stylesheets/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ $z-layers: (
// .edit-post-header { z-index: 30 }
".components-notice-list": 29,


// Show snackbars above everything (similar to popovers)
".components-snackbar-list": 100000,

// Show modal under the wp-admin menus and the popover
".components-modal__screen-overlay": 100000,

Expand Down
6 changes: 6 additions & 0 deletions docs/manifest-devhub.json
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,12 @@
"markdown_source": "../packages/components/src/slot-fill/README.md",
"parent": "components"
},
{
"title": "Snackbar",
"slug": "snackbar",
"markdown_source": "../packages/components/src/snackbar/README.md",
"parent": "components"
},
{
"title": "Spinner",
"slug": "spinner",
Expand Down
6 changes: 6 additions & 0 deletions docs/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,12 @@
"markdown_source": "https://raw.githubusercontent.com/WordPress/gutenberg/master/packages/components/src/slot-fill/README.md",
"parent": "components"
},
{
"title": "Snackbar",
"slug": "snackbar",
"markdown_source": "https://raw.githubusercontent.com/WordPress/gutenberg/master/packages/components/src/snackbar/README.md",
"parent": "components"
},
{
"title": "Spinner",
"slug": "spinner",
Expand Down
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### New Features

- Added a new `HorizontalRule` component.
- Added a new `Snackbar` component.
youknowriad marked this conversation as resolved.
Show resolved Hide resolved

### Bug Fixes

Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export { default as ResizableBox } from './resizable-box';
export { default as ResponsiveWrapper } from './responsive-wrapper';
export { default as SandBox } from './sandbox';
export { default as SelectControl } from './select-control';
export { default as Snackbar } from './snackbar';
export { default as SnackbarList } from './snackbar/list';
export { default as Spinner } from './spinner';
export { default as ServerSideRender } from './server-side-render';
export { default as TabPanel } from './tab-panel';
Expand Down
48 changes: 48 additions & 0 deletions packages/components/src/snackbar/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Snackbar

Use Snackbars to communicate low priority, non-interruptive messages to the user.

## Table of contents

1. [Design guidelines](#design-guidelines)
2. [Development guidelines](#development-guidelines)
3. [Related components](#related-components)
Copy link
Contributor Author

@youknowriad youknowriad May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a good idea to start adding "Accessibility guidelines" to all these component READMEs similar to dev and design. cc @audrasjb @LukePettway

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea!


## Design guidelines

A Snackbar displays a succinct message that is cleared out after a small delay. It can also offer the user options, like viewing a published post but these options should also be available elsewhere in the UI.

## Development guidelines

### Usage

To display a plain snackbar, pass the message as a `children` prop:

```jsx
const MySnackbarNotice = () => (
<Snackbar>
Post published successfully.
</Snackbar>
);
```

For more complex markup, you can pass any JSX element:

```jsx
const MySnackbarNotice = () => (
<Snackbar>
<p>An error occurred: <code>{ errorDetails }</code>.</p>
</Snackbar>
);
```

#### Props

The following props are used to control the display of the component.

* `onRemove`: function called when dismissing the notice.
* `actions`: (array) an array of action objects. Each member object should contain a `label` and either a `url` link string or `onClick` callback function. A `className` property can be used to add custom classes to the button styles.

## Related components

- To create a prominent message that requires a higher-level of attention, use a Notice.
80 changes: 80 additions & 0 deletions packages/components/src/snackbar/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* External dependencies
*/
import { flow, noop } from 'lodash';
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { useEffect } from '@wordpress/element';

/**
* Internal dependencies
*/
import { Button } from '../';

const stopPropagation = ( event ) => event.stopPropagation();
const NOTICE_TIMEOUT = 10000;

function Snackbar( {
className,
children,
actions = [],
onRemove = noop,
} ) {
useEffect( () => {
// This rule doesn't account yet for React Hooks
// eslint-disable-next-line @wordpress/react-no-unsafe-timeout
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rule doesn't account for React Hooks. It should be possible to use and cancel timeouts in useEffect or other hooks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked here #15622

const timeoutHandle = setTimeout( () => {
onRemove();
}, NOTICE_TIMEOUT );

return () => clearTimeout( timeoutHandle );
}, [] );
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't have to be done in this pull, but one thing that could get around having to use the effect rule exclusion here is to create a useTimeout hook that components like this can use (and the only place the es-lint rule would need skipped is in with that custom hook definition).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I can see how this can be useful. Maybe we could try it once we find a second use-case?


const classes = classnames( className, 'components-snackbar' );

return (
<div
className={ classes }
onClick={ onRemove }
tabIndex="0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those notices are announced with wp.a11y.speak method like all other types of notices. They are also automatically being dismissed after 10 seconds. Do they really need to be focusable? What are the odds that keyboard-only user will want to tab to this message to dismiss it before it gets removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pressing the ESC key while one of the snackbar’s child elements has focus (e.g., the action button) will dismiss the snackbar.

Yes, no strong opinions from me but I had this feedback earlier.

role="button"
onKeyPress={ onRemove }
>
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
<div className="components-snackbar__content">
{ children }
{ actions.map(
(
{
className: buttonCustomClasses,
label,
onClick,
url,
},
index
) => {
return (
<Button
key={ index }
href={ url }
isTertiary
onClick={ url ? stopPropagation : flow( stopPropagation, onClick ) }
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
className={ classnames(
'components-snackbar__action',
buttonCustomClasses
) }
>
{ label }
</Button>
);
}

) }
</div>
</div>
);
}

export default Snackbar;
42 changes: 42 additions & 0 deletions packages/components/src/snackbar/list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* External dependencies
*/
import classnames from 'classnames';
import { omit, noop } from 'lodash';

/**
* Internal dependencies
*/
import Snackbar from './';

/**
* Renders a list of notices.
*
* @param {Object} $0 Props passed to the component.
* @param {Array} $0.notices Array of notices to render.
* @param {Function} $0.onRemove Function called when a notice should be removed / dismissed.
* @param {Object} $0.className Name of the class used by the component.
* @param {Object} $0.children Array of children to be rendered inside the notice list.
* @return {Object} The rendered notices list.
*/
function SnackbarList( { notices, className, children, onRemove = noop } ) {
className = classnames( 'components-snackbar-list', className );
const removeNotice = ( id ) => () => onRemove( id );
youknowriad marked this conversation as resolved.
Show resolved Hide resolved

return (
<div className={ className }>
{ children }
{ [ ...notices ].reverse().map( ( notice ) => (
youknowriad marked this conversation as resolved.
Show resolved Hide resolved
<Snackbar
{ ...omit( notice, [ 'content' ] ) }
key={ notice.id }
onRemove={ removeNotice( notice.id ) }
>
{ notice.content }
</Snackbar>
) ) }
</div>
);
}

export default SnackbarList;
50 changes: 50 additions & 0 deletions packages/components/src/snackbar/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
.components-snackbar {
font-family: $default-font;
font-size: $default-font-size;
background-color: $dark-gray-700;
border-radius: $radius-round-rectangle;
box-shadow: 0 2px 4px rgba(0, 0, 0, 0.3);
color: $white;
padding: 10px;
min-width: 200px;
max-width: 400px;

&:hover {
background-color: $dark-gray-900;
}

&:focus {
background-color: $dark-gray-900;
box-shadow:
0 0 0 1px $white,
0 0 0 3px $blue-medium-focus;
}
}

.components-snackbar__action.components-button {
margin-left: 20px;
color: $white;

&:not(:disabled):not([aria-disabled="true"]):not(.is-default) {
text-decoration: underline;

&:hover {
color: $white;
text-decoration: none;
}
}
}

.components-snackbar__content {
display: flex;
align-items: center;
}

.components-snackbar-list {
position: absolute;
z-index: z-index(".components-snackbar-list");

.components-snackbar {
margin-top: 10px;
}
}
1 change: 1 addition & 0 deletions packages/components/src/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
@import "./sandbox/style.scss";
@import "./scroll-lock/style.scss";
@import "./select-control/style.scss";
@import "./snackbar/style.scss";
@import "./spinner/style.scss";
@import "./text-control/style.scss";
@import "./textarea-control/style.scss";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
*/
export async function publishPostWithPrePublishChecksDisabled() {
await page.click( '.editor-post-publish-button' );
return page.waitForSelector( '.components-notice.is-success' );
return page.waitForSelector( '.components-snackbar' );
}
2 changes: 1 addition & 1 deletion packages/e2e-test-utils/src/publish-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ export async function publishPost() {
await page.click( '.editor-post-publish-button' );

// A success notice should show up
return page.waitForSelector( '.components-notice.is-success' );
return page.waitForSelector( '.components-snackbar' );
}
8 changes: 4 additions & 4 deletions packages/e2e-tests/specs/reusable-blocks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe( 'Reusable Blocks', () => {

// Wait for creation to finish
await page.waitForXPath(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be wrapped with an e2e test util to simplify usage in different places? It might be too early, but at least I can see the value of having a local function which simplifies his logic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya I agree, it'd be handy to have a test util for verifying notices appear (would be good if it allows for notice type as well as content in the signature).

'//*[contains(@class, "components-notice") and contains(@class, "is-success")]/*[text()="Block created."]'
'//*[contains(@class, "components-snackbar")]/*[text()="Block created."]'
);

// Select all of the text in the title field by triple-clicking on it. We
Expand Down Expand Up @@ -84,7 +84,7 @@ describe( 'Reusable Blocks', () => {

// Wait for creation to finish
await page.waitForXPath(
'//*[contains(@class, "components-notice") and contains(@class, "is-success")]/*[text()="Block created."]'
'//*[contains(@class, "components-snackbar")]/*[text()="Block created."]'
);

// Save the reusable block
Expand Down Expand Up @@ -184,7 +184,7 @@ describe( 'Reusable Blocks', () => {

// Wait for deletion to finish
await page.waitForXPath(
'//*[contains(@class, "components-notice") and contains(@class, "is-success")]/*[text()="Block deleted."]'
'//*[contains(@class, "components-snackbar")]/*[text()="Block deleted."]'
);

// Check that we have an empty post again
Expand Down Expand Up @@ -221,7 +221,7 @@ describe( 'Reusable Blocks', () => {

// Wait for creation to finish
await page.waitForXPath(
'//*[contains(@class, "components-notice") and contains(@class, "is-success")]/*[text()="Block created."]'
'//*[contains(@class, "components-snackbar")]/*[text()="Block created."]'
);

// Select all of the text in the title field by triple-clicking on it. We
Expand Down
3 changes: 1 addition & 2 deletions packages/edit-post/src/components/layout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ function Layout( {
aria-label={ __( 'Editor content' ) }
tabIndex="-1"
>
<EditorNotices dismissible={ false } className="is-pinned" />
<EditorNotices dismissible={ true } />
<EditorNotices />
<PreserveScrollInReorder />
<EditorModeKeyboardShortcuts />
<KeyboardShortcutHelpModal />
Expand Down
35 changes: 7 additions & 28 deletions packages/edit-post/src/components/layout/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,6 @@
.edit-post-layout {
position: relative;

.components-notice-list {
position: sticky;
top: $header-height;
right: 0;
color: $dark-gray-900;

@include break-small {
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__dismiss {
margin: 10px 5px;
}
}

// 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;
Expand All @@ -54,6 +26,13 @@
}
}

// Adjust the position of the notices
@include editor-left(".edit-post-layout__content .components-editor-notices__snackbar");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming this mixin is not needed anymore if you want to center them

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Definitely had no clue what this was for. 😜

.edit-post-layout__content .components-editor-notices__snackbar {
position: fixed;
padding-left: 20px;
}

.edit-post-layout__content {
display: flex;
flex-direction: column;
Expand Down
Loading