Refactor collection editor state handling (#38133)

app/javascript/mastodon/features/collections/detail/index.tsx

@@ -3,7 +3,7 @@ import { useCallback, useEffect } from 'react';
 import { defineMessages, FormattedMessage, useIntl } from 'react-intl';
 
 import { Helmet } from 'react-helmet';
-import { useLocation, useParams } from 'react-router';
+import { useHistory, useLocation, useParams } from 'react-router';
 
 import { openModal } from '@/mastodon/actions/modal';
 import ListAltIcon from '@/material-icons/400-24px/list_alt.svg?react';
@@ -84,6 +84,7 @@ const CollectionHeader: React.FC<{ collection: ApiCollectionJSON }> = ({
   const intl = useIntl();
   const { name, description, tag, account_id } = collection;
   const dispatch = useAppDispatch();
+  const history = useHistory();
 
   const handleShare = useCallback(() => {
     dispatch(
@@ -97,12 +98,14 @@ const CollectionHeader: React.FC<{ collection: ApiCollectionJSON }> = ({
   }, [collection, dispatch]);
 
   const location = useLocation<{ newCollection?: boolean } | undefined>();
-  const wasJustCreated = location.state?.newCollection;
+  const isNewCollection = location.state?.newCollection;
   useEffect(() => {
-    if (wasJustCreated) {
+    if (isNewCollection) {
+      // Replace with current pathname to clear `newCollection` state
+      history.replace(location.pathname);
       handleShare();
     }
-  }, [handleShare, wasJustCreated]);
+  }, [history, handleShare, isNewCollection, location.pathname]);
 
   return (
     <div className={classes.header}>

app/javascript/mastodon/features/collections/detail/share_modal.tsx

@@ -64,7 +64,7 @@ export const CollectionShareModal: React.FC<{
     onClose();
     dispatch(changeCompose(shareMessage));
     dispatch(focusCompose());
-  }, [collectionLink, dispatch, intl, isOwnCollection, onClose]);
+  }, [onClose, collectionLink, dispatch, intl, isOwnCollection]);
 
   return (
     <ModalShell>

app/javascript/mastodon/features/collections/editor/accounts.tsx

@@ -2,7 +2,7 @@ import { useCallback, useId, useMemo, useState } from 'react';
 
 import { FormattedMessage, useIntl } from 'react-intl';
 
-import { useHistory, useLocation } from 'react-router-dom';
+import { useHistory } from 'react-router-dom';
 
 import CancelIcon from '@/material-icons/400-24px/cancel.svg?react';
 import CheckIcon from '@/material-icons/400-24px/check.svg?react';
@@ -30,12 +30,12 @@ import { useAccount } from 'mastodon/hooks/useAccount';
 import { me } from 'mastodon/initial_state';
 import {
   addCollectionItem,
+  getCollectionItemIds,
   removeCollectionItem,
+  updateCollectionEditorField,
 } from 'mastodon/reducers/slices/collections';
 import { store, useAppDispatch, useAppSelector } from 'mastodon/store';
 
-import type { TempCollectionState } from './state';
-import { getCollectionEditorState } from './state';
 import classes from './styles.module.scss';
 import { WizardStepHeader } from './wizard_step_header';
 
@@ -52,9 +52,8 @@ function isOlderThanAWeek(date?: string): boolean {
 
 const AddedAccountItem: React.FC<{
   accountId: string;
-  isRemovable: boolean;
   onRemove: (id: string) => void;
-}> = ({ accountId, isRemovable, onRemove }) => {
+}> = ({ accountId, onRemove }) => {
   const intl = useIntl();
   const account = useAccount(accountId);
 
@@ -86,17 +85,15 @@ const AddedAccountItem: React.FC<{
       id={accountId}
       extraAccountInfo={lastPostHint}
     >
-      {isRemovable && (
-        <IconButton
-          title={intl.formatMessage({
-            id: 'collections.remove_account',
-            defaultMessage: 'Remove this account',
-          })}
-          icon='remove'
-          iconComponent={CancelIcon}
-          onClick={handleRemoveAccount}
-        />
-      )}
+      <IconButton
+        title={intl.formatMessage({
+          id: 'collections.remove_account',
+          defaultMessage: 'Remove this account',
+        })}
+        icon='remove'
+        iconComponent={CancelIcon}
+        onClick={handleRemoveAccount}
+      />
     </Account>
   );
 };
@@ -139,28 +136,25 @@ export const CollectionAccounts: React.FC<{
   const intl = useIntl();
   const dispatch = useAppDispatch();
   const history = useHistory();
-  const location = useLocation<TempCollectionState>();
-  const { id, initialItemIds } = getCollectionEditorState(
-    collection,
-    location.state,
-  );
-  const isEditMode = !!id;
-  const collectionItems = collection?.items;
 
-  const [searchValue, setSearchValue] = useState('');
-  // This state is only used when creating a new collection.
-  // In edit mode, the collection will be updated instantly
-  const [addedAccountIds, setAccountIds] = useState(initialItemIds);
+  const { id, items } = collection ?? {};
+  const isEditMode = !!id;
+  const collectionItems = items;
+
+  const addedAccountIds = useAppSelector(
+    (state) => state.collections.editor.accountIds,
+  );
+
+  // In edit mode, we're bypassing state and just return collection items directly,
+  // since they're edited "live", saving after each addition/deletion
   const accountIds = useMemo(
     () =>
-      isEditMode
-        ? (collectionItems
-            ?.map((item) => item.account_id)
-            .filter((id): id is string => !!id) ?? [])
-        : addedAccountIds,
+      isEditMode ? getCollectionItemIds(collectionItems) : addedAccountIds,
     [isEditMode, collectionItems, addedAccountIds],
   );
 
+  const [searchValue, setSearchValue] = useState('');
+
   const hasMaxAccounts = accountIds.length === MAX_ACCOUNT_COUNT;
 
   const {
@@ -233,28 +227,41 @@ export const CollectionAccounts: React.FC<{
     [dispatch, relationships],
   );
 
-  const removeAccountItem = useCallback((accountId: string) => {
-    setAccountIds((ids) => ids.filter((id) => id !== accountId));
-  }, []);
+  const removeAccountItem = useCallback(
+    (accountId: string) => {
+      dispatch(
+        updateCollectionEditorField({
+          field: 'accountIds',
+          value: accountIds.filter((id) => id !== accountId),
+        }),
+      );
+    },
+    [accountIds, dispatch],
+  );
 
   const addAccountItem = useCallback(
     (accountId: string) => {
       confirmFollowStatus(accountId, () => {
-        setAccountIds((ids) => [...ids, accountId]);
+        dispatch(
+          updateCollectionEditorField({
+            field: 'accountIds',
+            value: [...accountIds, accountId],
+          }),
+        );
       });
     },
-    [confirmFollowStatus],
+    [accountIds, confirmFollowStatus, dispatch],
   );
 
   const toggleAccountItem = useCallback(
     (item: SuggestionItem) => {
-      if (addedAccountIds.includes(item.id)) {
+      if (accountIds.includes(item.id)) {
         removeAccountItem(item.id);
       } else {
         addAccountItem(item.id);
       }
     },
-    [addAccountItem, addedAccountIds, removeAccountItem],
+    [accountIds, addAccountItem, removeAccountItem],
   );
 
   const instantRemoveAccountItem = useCallback(
@@ -406,7 +413,6 @@ export const CollectionAccounts: React.FC<{
               >
                 <AddedAccountItem
                   accountId={accountId}
-                  isRemovable={!isEditMode}
                   onRemove={handleRemoveAccountItem}
                 />
               </Article>

app/javascript/mastodon/features/collections/editor/details.tsx

@@ -1,13 +1,12 @@
-import { useCallback, useState } from 'react';
+import { useCallback } from 'react';
 
 import { FormattedMessage } from 'react-intl';
 
-import { useHistory, useLocation } from 'react-router-dom';
+import { useHistory } from 'react-router-dom';
 
 import { isFulfilled } from '@reduxjs/toolkit';
 
 import type {
-  ApiCollectionJSON,
   ApiCreateCollectionPayload,
   ApiUpdateCollectionPayload,
 } from 'mastodon/api_types/collections';
@@ -23,70 +22,77 @@ import { TextInputField } from 'mastodon/components/form_fields/text_input_field
 import {
   createCollection,
   updateCollection,
+  updateCollectionEditorField,
 } from 'mastodon/reducers/slices/collections';
-import { useAppDispatch } from 'mastodon/store';
+import { useAppDispatch, useAppSelector } from 'mastodon/store';
 
-import type { TempCollectionState } from './state';
-import { getCollectionEditorState } from './state';
 import classes from './styles.module.scss';
 import { WizardStepHeader } from './wizard_step_header';
 
-export const CollectionDetails: React.FC<{
-  collection?: ApiCollectionJSON | null;
-}> = ({ collection }) => {
+export const CollectionDetails: React.FC = () => {
   const dispatch = useAppDispatch();
   const history = useHistory();
-  const location = useLocation<TempCollectionState>();
-
-  const {
-    id,
-    initialName,
-    initialDescription,
-    initialTopic,
-    initialItemIds,
-    initialDiscoverable,
-    initialSensitive,
-  } = getCollectionEditorState(collection, location.state);
-
-  const [name, setName] = useState(initialName);
-  const [description, setDescription] = useState(initialDescription);
-  const [topic, setTopic] = useState(initialTopic);
-  const [discoverable, setDiscoverable] = useState(initialDiscoverable);
-  const [sensitive, setSensitive] = useState(initialSensitive);
+  const { id, name, description, topic, discoverable, sensitive, accountIds } =
+    useAppSelector((state) => state.collections.editor);
 
   const handleNameChange = useCallback(
     (event: React.ChangeEvent<HTMLInputElement>) => {
-      setName(event.target.value);
+      dispatch(
+        updateCollectionEditorField({
+          field: 'name',
+          value: event.target.value,
+        }),
+      );
     },
-    [],
+    [dispatch],
   );
 
   const handleDescriptionChange = useCallback(
     (event: React.ChangeEvent<HTMLTextAreaElement>) => {
-      setDescription(event.target.value);
+      dispatch(
+        updateCollectionEditorField({
+          field: 'description',
+          value: event.target.value,
+        }),
+      );
     },
-    [],
+    [dispatch],
   );
 
   const handleTopicChange = useCallback(
     (event: React.ChangeEvent<HTMLInputElement>) => {
-      setTopic(event.target.value);
+      dispatch(
+        updateCollectionEditorField({
+          field: 'topic',
+          value: event.target.value,
+        }),
+      );
     },
-    [],
+    [dispatch],
   );
 
   const handleDiscoverableChange = useCallback(
     (event: React.ChangeEvent<HTMLInputElement>) => {
-      setDiscoverable(event.target.value === 'public');
+      dispatch(
+        updateCollectionEditorField({
+          field: 'discoverable',
+          value: event.target.value === 'public',
+        }),
+      );
     },
-    [],
+    [dispatch],
   );
 
   const handleSensitiveChange = useCallback(
     (event: React.ChangeEvent<HTMLInputElement>) => {
-      setSensitive(event.target.checked);
+      dispatch(
+        updateCollectionEditorField({
+          field: 'sensitive',
+          value: event.target.checked,
+        }),
+      );
     },
-    [],
+    [dispatch],
   );
 
   const handleSubmit = useCallback(
@@ -112,7 +118,7 @@ export const CollectionDetails: React.FC<{
           description,
           discoverable,
           sensitive,
-          account_ids: initialItemIds,
+          account_ids: accountIds,
         };
         if (topic) {
           payload.tag_name = topic;
@@ -124,9 +130,7 @@ export const CollectionDetails: React.FC<{
           }),
         ).then((result) => {
           if (isFulfilled(result)) {
-            history.replace(
-              `/collections/${result.payload.collection.id}/edit/details`,
-            );
+            history.replace(`/collections`);
             history.push(`/collections/${result.payload.collection.id}`, {
               newCollection: true,
             });
@@ -143,7 +147,7 @@ export const CollectionDetails: React.FC<{
       sensitive,
       dispatch,
       history,
-      initialItemIds,
+      accountIds,
     ],
   );
 

app/javascript/mastodon/features/collections/editor/index.tsx

@@ -16,7 +16,10 @@ import ListAltIcon from '@/material-icons/400-24px/list_alt.svg?react';
 import { Column } from 'mastodon/components/column';
 import { ColumnHeader } from 'mastodon/components/column_header';
 import { LoadingIndicator } from 'mastodon/components/loading_indicator';
-import { fetchCollection } from 'mastodon/reducers/slices/collections';
+import {
+  collectionEditorActions,
+  fetchCollection,
+} from 'mastodon/reducers/slices/collections';
 import { useAppDispatch, useAppSelector } from 'mastodon/store';
 
 import { CollectionAccounts } from './accounts';
@@ -68,6 +71,7 @@ export const CollectionEditorPage: React.FC<{
   const collection = useAppSelector((state) =>
     id ? state.collections.collections[id] : undefined,
   );
+  const editorStateId = useAppSelector((state) => state.collections.editor.id);
   const isEditMode = !!id;
   const isLoading = isEditMode && !collection;
 
@@ -77,6 +81,18 @@ export const CollectionEditorPage: React.FC<{
     }
   }, [dispatch, id]);
 
+  useEffect(() => {
+    if (id !== editorStateId) {
+      void dispatch(collectionEditorActions.reset());
+    }
+  }, [dispatch, editorStateId, id]);
+
+  useEffect(() => {
+    if (collection) {
+      void dispatch(collectionEditorActions.init(collection));
+    }
+  }, [dispatch, collection]);
+
   const pageTitle = intl.formatMessage(usePageTitle(id));
 
   return (
@@ -104,7 +120,7 @@ export const CollectionEditorPage: React.FC<{
               exact
               path={`${path}/details`}
               // eslint-disable-next-line react/jsx-no-bind
-              render={() => <CollectionDetails collection={collection} />}
+              render={() => <CollectionDetails />}
             />
           </Switch>
         )}

app/javascript/mastodon/features/collections/editor/state.ts

@@ -1,52 +0,0 @@
-import type {
-  ApiCollectionJSON,
-  ApiCreateCollectionPayload,
-} from '@/mastodon/api_types/collections';
-
-/**
- * Temporary editor state across creation steps,
- * kept in location state
- */
-export type TempCollectionState =
-  | Partial<ApiCreateCollectionPayload>
-  | undefined;
-
-/**
- * Resolve initial editor state. Temporary location state
- * trumps stored data, otherwise initial values are returned.
- */
-export function getCollectionEditorState(
-  collection: ApiCollectionJSON | null | undefined,
-  locationState: TempCollectionState,
-) {
-  const {
-    id,
-    name = '',
-    description = '',
-    tag,
-    language = '',
-    discoverable = true,
-    sensitive = false,
-    items,
-  } = collection ?? {};
-
-  const collectionItemIds =
-    items?.map((item) => item.account_id).filter(onlyExistingIds) ?? [];
-
-  const initialItemIds = (
-    locationState?.account_ids ?? collectionItemIds
-  ).filter(onlyExistingIds);
-
-  return {
-    id,
-    initialItemIds,
-    initialName: locationState?.name ?? name,
-    initialDescription: locationState?.description ?? description,
-    initialTopic: locationState?.tag_name ?? tag?.name ?? '',
-    initialLanguage: locationState?.language ?? language,
-    initialDiscoverable: locationState?.discoverable ?? discoverable,
-    initialSensitive: locationState?.sensitive ?? sensitive,
-  };
-}
-
-const onlyExistingIds = (id?: string): id is string => !!id;