DataViews: remove selection correcting, set initial state (#62796)

Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
WordPress · Jun 25, 2024 · db9bf3c · db9bf3c
1 parent 70ff7c8
commit db9bf3c
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 91 deletions.
diff --git a/packages/dataviews/src/bulk-actions.tsx b/packages/dataviews/src/bulk-actions.tsx
@@ -7,7 +7,7 @@ import {
 	Modal,
 } from '@wordpress/components';
 import { __, sprintf, _n } from '@wordpress/i18n';
-import { useMemo, useState, useCallback, useEffect } from '@wordpress/element';
+import { useMemo, useState, useCallback } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -194,37 +194,16 @@ export default function BulkActions< Item extends AnyItem >( {
 	}, [ data, bulkActions ] );
 
 	const numberSelectableItems = selectableItems.length;
-	const areAllSelected =
-		selection && selection.length === numberSelectableItems;
 
 	const selectedItems = useMemo( () => {
-		return data.filter( ( item ) =>
-			selection.includes( getItemId( item ) )
+		return data.filter(
+			( item ) =>
+				selection.includes( getItemId( item ) ) &&
+				selectableItems.includes( item )
 		);
-	}, [ selection, data, getItemId ] );
+	}, [ selection, data, getItemId, selectableItems ] );
 
-	const hasNonSelectableItemSelected = useMemo( () => {
-		return selectedItems.some( ( item ) => {
-			return ! selectableItems.includes( item );
-		} );
-	}, [ selectedItems, selectableItems ] );
-	useEffect( () => {
-		if ( hasNonSelectableItemSelected ) {
-			onSelectionChange(
-				selectedItems.filter( ( selectedItem ) => {
-					return selectableItems.some( ( item ) => {
-						return getItemId( selectedItem ) === getItemId( item );
-					} );
-				} )
-			);
-		}
-	}, [
-		hasNonSelectableItemSelected,
-		selectedItems,
-		selectableItems,
-		getItemId,
-		onSelectionChange,
-	] );
+	const areAllSelected = selectedItems.length === numberSelectableItems;
 
 	if ( bulkActions.length === 0 ) {
 		return null;
@@ -243,15 +222,15 @@ export default function BulkActions< Item extends AnyItem >( {
 						variant="tertiary"
 						size="compact"
 					>
-						{ selection.length
+						{ selectedItems.length
 							? sprintf(
 									/* translators: %d: Number of items. */
 									_n(
 										'Edit %d item',
 										'Edit %d items',
-										selection.length
+										selectedItems.length
 									),
-									selection.length
+									selectedItems.length
 							  )
 							: __( 'Bulk edit' ) }
 					</Button>

diff --git a/packages/dataviews/src/dataviews.tsx b/packages/dataviews/src/dataviews.tsx
@@ -7,7 +7,7 @@ import type { ComponentType } from 'react';
  * WordPress dependencies
  */
 import { __experimentalHStack as HStack } from '@wordpress/components';
-import { useMemo, useState, useCallback, useEffect } from '@wordpress/element';
+import { useMemo, useState, useCallback } from '@wordpress/element';
 
 /**
  * Internal dependencies
@@ -92,25 +92,6 @@ export default function DataViews< Item extends AnyItem >( {
 	}
 	const [ openedFilter, setOpenedFilter ] = useState< string | null >( null );
 
-	useEffect( () => {
-		if (
-			selection.length > 0 &&
-			selection.some(
-				( id ) => ! data.some( ( item ) => getItemId( item ) === id )
-			)
-		) {
-			const newSelection = selection.filter( ( id ) =>
-				data.some( ( item ) => getItemId( item ) === id )
-			);
-			setSelection( newSelection );
-			onSelectionChange(
-				data.filter( ( item ) =>
-					newSelection.includes( getItemId( item ) )
-				)
-			);
-		}
-	}, [ selection, data, getItemId, onSelectionChange ] );
-
 	const onSetSelection = useCallback(
 		( items: Item[] ) => {
 			setSelection( items.map( ( item ) => getItemId( item ) ) );
@@ -127,6 +108,11 @@ export default function DataViews< Item extends AnyItem >( {
 		actions,
 		data
 	);
+	const _selection = useMemo( () => {
+		return selection.filter( ( id ) =>
+			data.some( ( item ) => getItemId( item ) === id )
+		);
+	}, [ selection, data, getItemId ] );
 	return (
 		<div className="dataviews-wrapper">
 			<HStack
@@ -160,7 +146,7 @@ export default function DataViews< Item extends AnyItem >( {
 							actions={ actions }
 							data={ data }
 							onSelectionChange={ onSetSelection }
-							selection={ selection }
+							selection={ _selection }
 							getItemId={ getItemId }
 						/>
 					) }
@@ -179,7 +165,7 @@ export default function DataViews< Item extends AnyItem >( {
 				isLoading={ isLoading }
 				onChangeView={ onChangeView }
 				onSelectionChange={ onSetSelection }
-				selection={ selection }
+				selection={ _selection }
 				setOpenedFilter={ setOpenedFilter }
 				view={ view }
 			/>
@@ -193,7 +179,7 @@ export default function DataViews< Item extends AnyItem >( {
 					<BulkActionsToolbar
 						data={ data }
 						actions={ actions }
-						selection={ selection }
+						selection={ _selection }
 						onSelectionChange={ onSetSelection }
 						getItemId={ getItemId }
 					/>

diff --git a/packages/dataviews/src/single-selection-checkbox.tsx b/packages/dataviews/src/single-selection-checkbox.tsx
@@ -29,7 +29,7 @@ export default function SingleSelectionCheckbox< Item extends AnyItem >( {
 	disabled,
 }: SingleSelectionCheckboxProps< Item > ) {
 	const id = getItemId( item );
-	const isSelected = selection.includes( id );
+	const isSelected = ! disabled && selection.includes( id );
 	let selectionLabel;
 	if ( primaryField?.getValue && item ) {
 		// eslint-disable-next-line @wordpress/valid-sprintf

diff --git a/packages/dataviews/src/view-table.tsx b/packages/dataviews/src/view-table.tsx
@@ -75,6 +75,7 @@ interface BulkSelectionCheckboxProps< Item extends AnyItem > {
 	onSelectionChange: ( items: Item[] ) => void;
 	data: Item[];
 	actions: Action< Item >[];
+	getItemId: ( item: Item ) => string;
 }
 
 interface TableRowProps< Item extends AnyItem > {
@@ -249,6 +250,7 @@ function BulkSelectionCheckbox< Item extends AnyItem >( {
 	onSelectionChange,
 	data,
 	actions,
+	getItemId,
 }: BulkSelectionCheckboxProps< Item > ) {
 	const selectableItems = useMemo( () => {
 		return data.filter( ( item ) => {
@@ -259,13 +261,18 @@ function BulkSelectionCheckbox< Item extends AnyItem >( {
 			);
 		} );
 	}, [ data, actions ] );
-	const areAllSelected = selection.length === selectableItems.length;
+	const selectedItems = data.filter(
+		( item ) =>
+			selection.includes( getItemId( item ) ) &&
+			selectableItems.includes( item )
+	);
+	const areAllSelected = selectedItems.length === selectableItems.length;
 	return (
 		<CheckboxControl
 			className="dataviews-view-table-selection-checkbox"
 			__nextHasNoMarginBottom
 			checked={ areAllSelected }
-			indeterminate={ ! areAllSelected && !! selection.length }
+			indeterminate={ ! areAllSelected && !! selectedItems.length }
 			onChange={ () => {
 				if ( areAllSelected ) {
 					onSelectionChange( [] );
@@ -293,7 +300,7 @@ function TableRow< Item extends AnyItem >( {
 	data,
 }: TableRowProps< Item > ) {
 	const hasPossibleBulkAction = useHasAPossibleBulkAction( actions, item );
-	const isSelected = selection.includes( id );
+	const isSelected = hasPossibleBulkAction && selection.includes( id );
 
 	const [ isHovered, setIsHovered ] = useState( false );
 
@@ -323,6 +330,9 @@ function TableRow< Item extends AnyItem >( {
 				isTouchDevice.current = true;
 			} }
 			onClick={ () => {
+				if ( ! hasPossibleBulkAction ) {
+					return;
+				}
 				if (
 					! isTouchDevice.current &&
 					document.getSelection()?.type !== 'Range'
@@ -495,6 +505,7 @@ function ViewTable< Item extends AnyItem >( {
 									onSelectionChange={ onSelectionChange }
 									data={ data }
 									actions={ actions }
+									getItemId={ getItemId }
 								/>
 							</th>
 						) }

diff --git a/packages/edit-site/src/components/page-pages/index.js b/packages/edit-site/src/components/page-pages/index.js
@@ -39,6 +39,7 @@ import AddNewPageModal from '../add-new-page';
 import Media from '../media';
 import { unlock } from '../../lock-unlock';
 import { useEditPostAction } from '../dataviews-actions';
+import { usePrevious } from '@wordpress/compose';
 
 const { usePostActions } = unlock( editorPrivateApis );
 const { useLocation, useHistory } = unlock( routerPrivateApis );
@@ -201,43 +202,18 @@ function FeaturedImage( { item, viewType } ) {
 	);
 }
 
-function usePostIdLinkInSelection(
-	selection,
-	setSelection,
-	isLoadingItems,
-	items
-) {
-	const {
-		params: { postId },
-	} = useLocation();
-	const [ postIdToSelect, setPostIdToSelect ] = useState( postId );
-	useEffect( () => {
-		if ( postId ) {
-			setPostIdToSelect( postId );
-		}
-	}, [ postId ] );
-
-	useEffect( () => {
-		if ( ! postIdToSelect ) {
-			return;
-		}
-		// Only try to select an item if the loading is complete and we have items.
-		if ( ! isLoadingItems && items && items.length ) {
-			// If the item is not in the current selection, select it.
-			if ( selection.length !== 1 || selection[ 0 ] !== postIdToSelect ) {
-				setSelection( [ postIdToSelect ] );
-			}
-			setPostIdToSelect( undefined );
-		}
-	}, [ postIdToSelect, selection, setSelection, isLoadingItems, items ] );
+function getItemId( item ) {
+	return item.id.toString();
 }
 
 export default function PagePages() {
 	const postType = 'page';
 	const [ view, setView ] = useView( postType );
 	const history = useHistory();
-
-	const [ selection, setSelection ] = useState( [] );
+	const {
+		params: { postId },
+	} = useLocation();
+	const [ selection, setSelection ] = useState( [ postId ] );
 
 	const onSelectionChange = useCallback(
 		( items ) => {
@@ -299,7 +275,19 @@ export default function PagePages() {
 		totalPages,
 	} = useEntityRecords( 'postType', postType, queryArgs );
 
-	usePostIdLinkInSelection( selection, setSelection, isLoadingPages, pages );
+	const ids = pages?.map( ( page ) => getItemId( page ) ) ?? [];
+	const prevIds = usePrevious( ids ) ?? [];
+	const deletedIds = prevIds.filter( ( id ) => ! ids.includes( id ) );
+	const postIdWasDeleted = deletedIds.includes( postId );
+
+	useEffect( () => {
+		if ( postIdWasDeleted ) {
+			history.push( {
+				...history.getLocationWithParams().params,
+				postId: undefined,
+			} );
+		}
+	}, [ postIdWasDeleted, history ] );
 
 	const { records: authors, isResolving: isLoadingAuthors } =
 		useEntityRecords( 'root', 'user', { per_page: -1 } );
@@ -569,7 +557,7 @@ export default function PagePages() {
 				selection={ selection }
 				setSelection={ setSelection }
 				onSelectionChange={ onSelectionChange }
-				getItemId={ ( item ) => item.id.toString() }
+				getItemId={ getItemId }
 			/>
 		</Page>
 	);