Rename current to selected to reflect the usage

Ensure the latest revision is highlighted even when there is no selected id Add useEffect dependencies Updating tests Align text to left for long strings
WordPress · May 18, 2023 · b604b97 · b604b97
1 parent 671d727
commit b604b97
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 58 deletions.
diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/index.js b/packages/edit-site/src/components/global-styles/screen-revisions/index.js
@@ -42,19 +42,11 @@ function ScreenRevisions() {
 			blocks: select( blockEditorStore ).getBlocks(),
 		};
 	}, [] );
-
 	const { revisions, isLoading, hasUnsavedChanges } =
 		useGlobalStylesRevisions();
+	const [ selectedRevisionId, setSelectedRevisionId ] = useState();
 	const [ globalStylesRevision, setGlobalStylesRevision ] =
 		useState( userConfig );
-
-	const [ currentRevisionId, setCurrentRevisionId ] = useState(
-		/*
-		 * We need this for the first render,
-		 * otherwise the unsaved changes haven't been merged into the revisions array yet.
-		 */
-		hasUnsavedChanges ? 'unsaved' : revisions?.[ 0 ]?.id
-	);
 	const [
 		isLoadingRevisionWithUnsavedChanges,
 		setIsLoadingRevisionWithUnsavedChanges,
@@ -89,7 +81,7 @@ function ScreenRevisions() {
 			settings: revision?.settings,
 			id: revision?.id,
 		} );
-		setCurrentRevisionId( revision?.id );
+		setSelectedRevisionId( revision?.id );
 	};
 
 	const isLoadButtonEnabled =
@@ -117,7 +109,7 @@ function ScreenRevisions() {
 			<div className="edit-site-global-styles-screen-revisions">
 				<RevisionsButtons
 					onChange={ selectRevision }
-					currentRevisionId={ currentRevisionId }
+					selectedRevisionId={ selectedRevisionId }
 					userRevisions={ revisions }
 				/>
 				{ isLoadButtonEnabled && (

diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js b/packages/edit-site/src/components/global-styles/screen-revisions/revisions-buttons.js
@@ -18,9 +18,8 @@ import { dateI18n, getDate, humanTimeDiff, getSettings } from '@wordpress/date';
  */
 function getRevisionLabel( revision ) {
 	const authorDisplayName = revision?.author?.name || __( 'User' );
-	const isUnsaved = 'unsaved' === revision?.id;
 
-	if ( isUnsaved ) {
+	if ( 'unsaved' === revision?.id ) {
 		return sprintf(
 			/* translators: %(name)s author display name */
 			__( 'Unsaved changes by %(name)s' ),
@@ -57,45 +56,42 @@ function getRevisionLabel( revision ) {
  * Returns a rendered list of revisions buttons.
  *
  * @typedef {Object} props
- * @property {Array<Object>} userRevisions     A collection of user revisions.
- * @property {number}        currentRevisionId Callback fired when the modal is closed or action cancelled.
- * @property {Function}      onChange          Callback fired when a revision is selected.
+ * @property {Array<Object>} userRevisions      A collection of user revisions.
+ * @property {number}        selectedRevisionId The id of the currently-selected revision.
+ * @property {Function}      onChange           Callback fired when a revision is selected.
  *
- * @param    {props}         Component         props.
+ * @param    {props}         Component          props.
  * @return {JSX.Element} The modal component.
  */
-function RevisionsButtons( { userRevisions, currentRevisionId, onChange } ) {
+function RevisionsButtons( { userRevisions, selectedRevisionId, onChange } ) {
 	return (
 		<ol
 			className="edit-site-global-styles-screen-revisions__revisions-list"
 			aria-label={ __( 'Global styles revisions' ) }
 			role="group"
 		>
-			{ userRevisions.map( ( revision ) => {
-				const { id, author, isLatest, modified } = revision;
+			{ userRevisions.map( ( revision, index ) => {
+				const { id, author, modified } = revision;
 				const authorDisplayName = author?.name || __( 'User' );
 				const authorAvatar = author?.avatar_urls?.[ '48' ];
-				/*
-				 * If the currentId hasn't been selected yet, the first revision is
-				 * the current one so long as the API returns revisions in descending order.
-				 */
-				const isActive = !! currentRevisionId
-					? id === currentRevisionId
-					: isLatest;
+				const isUnsaved = 'unsaved' === revision?.id;
+				const isSelected = selectedRevisionId
+					? selectedRevisionId === revision?.id
+					: index === 0;
 
 				return (
 					<li
 						className={ classnames(
 							'edit-site-global-styles-screen-revisions__revision-item',
 							{
-								'is-current': isActive,
+								'is-selected': isSelected,
 							}
 						) }
 						key={ id }
 					>
 						<Button
 							className="edit-site-global-styles-screen-revisions__revision-button"
-							disabled={ isActive }
+							disabled={ isSelected }
 							onClick={ () => {
 								onChange( revision );
 							} }
@@ -106,13 +102,25 @@ function RevisionsButtons( { userRevisions, currentRevisionId, onChange } ) {
 									{ humanTimeDiff( modified ) }
 								</time>
 								<span className="edit-site-global-styles-screen-revisions__meta">
-									{ sprintf(
-										/* translators: %(name)s author display name */
-										__( 'Changes saved by %(name)s' ),
-										{
-											name: authorDisplayName,
-										}
-									) }
+									{ isUnsaved
+										? sprintf(
+												/* translators: %(name)s author display name */
+												__(
+													'Unsaved changes by %(name)s'
+												),
+												{
+													name: authorDisplayName,
+												}
+										  )
+										: sprintf(
+												/* translators: %(name)s author display name */
+												__(
+													'Changes saved by %(name)s'
+												),
+												{
+													name: authorDisplayName,
+												}
+										  ) }
 
 									<img
 										alt={ author?.name }

diff --git a/packages/edit-site/src/components/global-styles/screen-revisions/style.scss b/packages/edit-site/src/components/global-styles/screen-revisions/style.scss
@@ -36,7 +36,7 @@
 		left: 0;
 		transform: translate(-50%, -50%);
 	}
-	&.is-current::before {
+	&.is-selected::before {
 		background: var(--wp-components-color-accent, var(--wp-admin-theme-color, #007cba));
 	}
 }
@@ -56,7 +56,7 @@
 	}
 }
 
-.is-current {
+.is-selected {
 	.edit-site-global-styles-screen-revisions__revision-button {
 		color: var(--wp-components-color-accent, var(--wp-admin-theme-color, #007cba));
 		opacity: 1;
@@ -86,6 +86,7 @@
 	justify-content: space-between;
 	width: 100%;
 	align-items: center;
+	text-align: left;
 
 	img {
 		width: $grid-unit-20;

diff --git a/...it-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js b/...it-site/src/components/global-styles/screen-revisions/test/use-global-styles-revisions.js
@@ -49,7 +49,6 @@ describe( 'useGlobalStylesRevisions', () => {
 				styles: {},
 			},
 		],
-		isLoading: false,
 	};
 
 	it( 'returns loaded revisions with no unsaved changes', () => {
@@ -109,10 +108,24 @@ describe( 'useGlobalStylesRevisions', () => {
 		] );
 	} );
 
-	it( 'returns empty revisions when still loading', () => {
+	it( 'returns empty revisions', () => {
 		useSelect.mockImplementation( () => ( {
 			...selectValue,
-			isLoading: true,
+			revisions: [],
+		} ) );
+
+		const { result } = renderHook( () => useGlobalStylesRevisions() );
+		const { revisions, isLoading, hasUnsavedChanges } = result.current;
+
+		expect( isLoading ).toBe( true );
+		expect( hasUnsavedChanges ).toBe( false );
+		expect( revisions ).toEqual( [] );
+	} );
+
+	it( 'returns empty revisions when authors are not yet available', () => {
+		useSelect.mockImplementation( () => ( {
+			...selectValue,
+			authors: [],
 		} ) );
 
 		const { result } = renderHook( () => useGlobalStylesRevisions() );

diff --git a/...es/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js b/...es/edit-site/src/components/global-styles/screen-revisions/use-global-styles-revisions.js
@@ -24,49 +24,41 @@ const SITE_EDITOR_AUTHORS_QUERY = {
 const { GlobalStylesContext } = unlock( blockEditorPrivateApis );
 export default function useGlobalStylesRevisions() {
 	const { user: userConfig } = useContext( GlobalStylesContext );
-	const { authors, currentUser, isDirty, revisions, isLoading } = useSelect(
+	const { authors, currentUser, isDirty, revisions } = useSelect(
 		( select ) => {
 			const {
 				__experimentalGetDirtyEntityRecords,
 				getCurrentUser,
 				getUsers,
 				getCurrentThemeGlobalStylesRevisions,
-				isResolving,
 			} = select( coreStore );
 			const dirtyEntityRecords = __experimentalGetDirtyEntityRecords();
 			const _currentUser = getCurrentUser();
 			const _isDirty = dirtyEntityRecords.length > 0;
 			const globalStylesRevisions =
 				getCurrentThemeGlobalStylesRevisions() || [];
-			const _authors = getUsers( SITE_EDITOR_AUTHORS_QUERY );
+			const _authors = getUsers( SITE_EDITOR_AUTHORS_QUERY ) || [];
 
 			return {
 				authors: _authors,
 				currentUser: _currentUser,
 				isDirty: _isDirty,
 				revisions: globalStylesRevisions,
-				isLoading:
-					! globalStylesRevisions.length ||
-					isResolving( 'getUsers', [ SITE_EDITOR_AUTHORS_QUERY ] ),
 			};
 		},
 		[]
 	);
 	return useMemo( () => {
 		let _modifiedRevisions = [];
-		if ( isLoading || ! revisions.length ) {
+		if ( ! authors.length || ! revisions.length ) {
 			return {
 				revisions: _modifiedRevisions,
 				hasUnsavedChanges: isDirty,
-				isLoading,
+				isLoading: true,
 			};
 		}
-		/*
-		 * Adds a flag to the first revision, which is the latest.
-		 * Also adds author information to the revision.
-		 * Then, if there are unsaved changes in the editor, create a
-		 * new "revision" item that represents the unsaved changes.
-		 */
+
+		// Adds author details to each revision.
 		_modifiedRevisions = revisions.map( ( revision ) => {
 			return {
 				...revision,
@@ -76,10 +68,12 @@ export default function useGlobalStylesRevisions() {
 			};
 		} );
 
-		if ( _modifiedRevisions[ 0 ]?.id !== 'unsaved' ) {
+		// Flags the most current saved revision.
+		if ( _modifiedRevisions[ 0 ].id !== 'unsaved' ) {
 			_modifiedRevisions[ 0 ].isLatest = true;
 		}
 
+		// Adds an item for unsaved changes.
 		if ( isDirty && ! isEmpty( userConfig ) && currentUser ) {
 			const unsavedRevision = {
 				id: 'unsaved',
@@ -94,10 +88,11 @@ export default function useGlobalStylesRevisions() {
 
 			_modifiedRevisions.unshift( unsavedRevision );
 		}
+
 		return {
 			revisions: _modifiedRevisions,
 			hasUnsavedChanges: isDirty,
-			isLoading,
+			isLoading: false,
 		};
-	}, [ revisions.length, isDirty, isLoading ] );
+	}, [ isDirty, revisions, currentUser, authors, userConfig ] );
 }