Skip to content

Commit

Permalink
Add loading state message and reorder notifications in sidebar
Browse files Browse the repository at this point in the history
  • Loading branch information
delawski committed Mar 8, 2021
1 parent 924f812 commit 543ab48
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 22 deletions.
17 changes: 14 additions & 3 deletions assets/src/block-editor/components/sidebar-notification/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import PropTypes from 'prop-types';
* Internal dependencies
*/
import './style.css';
import { Loading } from '../../../components/loading';

/**
* Notification component used in the block editor sidebar.
Expand All @@ -16,19 +17,28 @@ import './style.css';
* @param {string|Object} props.action Call to action element.
* @param {string|Object} props.icon Status icon element.
* @param {boolean} props.isError Flag indicating if it's an error message.
* @param {boolean} props.isLoading Flag indicating if it's a loading message.
* @param {string} props.message Message text.
*/
export function SidebarNotification( {
action,
icon,
isError = false,
isLoading = false,
message,
} ) {
const iconElement = isLoading ? <Loading /> : icon;

return (
<div className={ classnames( 'sidebar-notification', { 'is-error': isError } ) }>
{ icon && (
<div className={
classnames( 'sidebar-notification', {
'is-error': isError,
'is-loading': isLoading,
} )
}>
{ iconElement && (
<div className="sidebar-notification__icon">
{ icon }
{ iconElement }
</div>
) }
<div className="sidebar-notification__content">
Expand All @@ -48,6 +58,7 @@ SidebarNotification.propTypes = {
action: PropTypes.oneOfType( [ PropTypes.element, PropTypes.node ] ),
icon: PropTypes.oneOfType( [ PropTypes.element, PropTypes.node ] ),
isError: PropTypes.bool,
isLoading: PropTypes.bool,
message: PropTypes.string.isRequired,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
overflow: hidden;
display: flex;
padding: 14px 18px;
min-height: 67px;
}

.sidebar-notification.is-error::before {
Expand All @@ -15,14 +16,18 @@
background: #d64e21;
}

.sidebar-notification.is-loading {
align-items: center;
}

.sidebar-notification__icon {
align-items: center;
border-radius: 50%;
display: flex;
flex: 0 0 auto;
height: 24px;
justify-content: center;
margin: 4px 12px 0 0;
margin: 0 12px 0 0;
position: relative;
width: 24px;
}
Expand All @@ -32,6 +37,12 @@
width: 100%;
}

.sidebar-notification__content {
display: flex;
flex-direction: column;
justify-content: center;
}

.sidebar-notification__content > p {
margin: 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useDispatch, useSelect } from '@wordpress/data';
import { Button } from '@wordpress/components';
import { usePrevious } from '@wordpress/compose';
import { useDispatch, useSelect } from '@wordpress/data';
import { useEffect, useState } from '@wordpress/element';

/**
* Internal dependencies
Expand All @@ -16,29 +18,65 @@ import { SidebarNotification } from '../../block-editor/components/sidebar-notif
* AMP re-validate status message.
*/
export default function AMPRevalidateNotification() {
const [ didFetchErrors, setDidFetchErrors ] = useState( false );
const [ loadingMessage, setLoadingMessage ] = useState( '' );
const { autosave, savePost } = useDispatch( 'core/editor' );

const {
hasActiveMetaboxes,
isDraft,
isFetchingErrors,
isEditedPostNew,
isPostDirty,
} = useSelect( ( select ) => ( {
hasActiveMetaboxes: select( 'core/edit-post' ).hasMetaBoxes(),
isDraft: [ 'draft', 'auto-draft' ].indexOf( select( 'core/editor' )?.getEditedPostAttribute( 'status' ) ) !== -1,
isEditedPostNew: select( 'core/editor' ).isEditedPostNew(),
isFetchingErrors: select( BLOCK_VALIDATION_STORE_KEY ).getIsFetchingErrors(),
isPostDirty: select( BLOCK_VALIDATION_STORE_KEY ).getIsPostDirty(),
} ), [] );

const wasEditedPostNew = usePrevious( isEditedPostNew );
const wasFetchingErrors = usePrevious( isFetchingErrors );

useEffect( () => {
if ( didFetchErrors ) {
return;
}

// Set up the state right after errors fetching has finished.
if ( ! isFetchingErrors && wasFetchingErrors ) {
setDidFetchErrors( true );
}
}, [ didFetchErrors, isFetchingErrors, wasFetchingErrors ] );

/**
* Display best-suited loading message depending if the post has been
* already validated or not, or the editor has just been opened.
*/
useEffect( () => {
if ( didFetchErrors ) {
setLoadingMessage( __( 'Re-validating page content.', 'amp' ) );
} else if ( isEditedPostNew || wasEditedPostNew ) {
setLoadingMessage( __( 'Validating page content.', 'amp' ) );
} else {
setLoadingMessage( __( 'Loading…', 'amp' ) );
}
}, [ didFetchErrors, isEditedPostNew, wasEditedPostNew ] );

if ( isFetchingErrors ) {
return null;
return (
<SidebarNotification message={ loadingMessage } isLoading={ true } />
);
}

// For posts where meta boxes are present, it's impossible to tell
// if a meta box content has changed or not. Because of that, a dirty
// state is ignored and it's always possible to save a post.
// Likewise, we always display the re-validate message if there are
// active meta boxes.
/**
* For posts where meta boxes are present, it's impossible to tell
* if a meta box content has changed or not. Because of that, a dirty
* state is ignored and it's always possible to save a post.
* Likewise, we always display the re-validate message if there are
* active meta boxes.
*/
if ( ! isPostDirty && ! hasActiveMetaboxes ) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { useSelect } from '@wordpress/data';
import { BLOCK_VALIDATION_STORE_KEY } from '../store';
import AMPValidationErrorsKeptIcon from '../../../images/amp-validation-errors-kept.svg';
import { StatusIcon } from '../icon';
import { Loading } from '../../components/loading';
import { SidebarNotification } from '../../block-editor/components/sidebar-notification';

/**
Expand All @@ -30,7 +29,7 @@ export default function AMPValidationStatusNotification() {
} ), [] );

if ( isFetchingErrors ) {
return <Loading />;
return null;
}

if ( ampCompatibilityBroken ) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe( 'AMPRevalidateNotification', () => {
expect( container.children ).toHaveLength( 0 );
} );

it( 'does not render when errors are being fetched', () => {
it( 'renders loading spinner when errors are being fetched', () => {
setupUseSelect( {
isFetchingErrors: true,
} );
Expand All @@ -67,7 +67,7 @@ describe( 'AMPRevalidateNotification', () => {
render( <AMPRevalidateNotification />, container );
} );

expect( container.children ).toHaveLength( 0 );
expect( container.querySelector( '.amp-spinner-container' ) ).not.toBeNull();
} );

it( 'renders revalidate message if post is dirty', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe( 'AMPValidationStatusNotification', () => {
expect( container.querySelector( '.is-error' ) ).toBeNull();
} );

it( 'renders loading spinner when errors are being fetched', () => {
it( 'does not render when errors are being fetched', () => {
setupUseSelect( {
isFetchingErrors: true,
} );
Expand All @@ -61,7 +61,7 @@ describe( 'AMPValidationStatusNotification', () => {
render( <AMPValidationStatusNotification />, container );
} );

expect( container.querySelector( '.amp-spinner-container' ) ).not.toBeNull();
expect( container.children ).toHaveLength( 0 );
} );

it( 'renders error message when AMP compatibility is broken', () => {
Expand Down
2 changes: 1 addition & 1 deletion assets/src/block-validation/sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ export function Sidebar() {
return (
<div className="amp-sidebar">
<SidebarNotificationsContainer isShady={ true }>
<AMPValidationStatusNotification />
<AMPRevalidateNotification />
<AMPValidationStatusNotification />
</SidebarNotificationsContainer>

{ 0 < validationErrors.length && (
Expand Down
4 changes: 4 additions & 0 deletions assets/src/components/loading/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@
display: flex;
justify-content: center;
}

.amp-spinner-container .components-spinner {
margin-top: 0;
}

0 comments on commit 543ab48

Please sign in to comment.