diff --git a/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx b/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx index 882879e7ba..e34768d212 100644 --- a/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx +++ b/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx @@ -170,11 +170,13 @@ export const GalleryBarAndListHeader: React.FC = ({ } onCollectionCast={() => setOpenAlbumCastDialog(true)} /> - ) : ( + ) : activePerson ? ( + ) : ( + <> ), itemType: ITEM_TYPE.HEADER, height: 68, diff --git a/web/apps/photos/src/pages/gallery.tsx b/web/apps/photos/src/pages/gallery.tsx index 05a3b03577..3cbf5ef0cd 100644 --- a/web/apps/photos/src/pages/gallery.tsx +++ b/web/apps/photos/src/pages/gallery.tsx @@ -565,8 +565,9 @@ export default function Gallery() { let filteredPeople = people ?? []; if (tempDeletedFileIds?.size ?? tempHiddenFileIds?.size) { // Prune the in-memory temp updates from the actual state to - // obtain the UI state. - filteredPeople = (people ?? []) + // obtain the UI state. Kept inside an preflight check to so + // that the common path remains fast. + filteredPeople = filteredPeople .map((p) => ({ ...p, fileIDs: p.fileIDs.filter( @@ -579,6 +580,8 @@ export default function Gallery() { } const activePerson = filteredPeople.find((p) => p.id == activePersonID) ?? + // We don't have an "All" pseudo-album in people mode currently, + // so default to the first person in the list. filteredPeople[0]; const pfSet = new Set(activePerson?.fileIDs ?? []); filteredFiles = getUniqueFiles( @@ -589,7 +592,6 @@ export default function Gallery() { ); galleryPeopleState = { activePerson, - activePersonID, people: filteredPeople, }; } else { @@ -681,31 +683,6 @@ export default function Gallery() { galleryPeopleState: undefined, }; - // Calling setState during rendering is frowned upon for good reasons, but - // it is not verboten, and it has documented semantics: - // - // > React will discard the currently rendering component's output and - // > immediately attempt to render it again with the new state. - // > - // > https://react.dev/reference/react/useState - // - // That said, we should try to refactor this code to use a reducer or some - // other store so that this is not needed. - if (barMode == "people" && galleryPeopleState?.people.length === 0) { - log.info( - "Resetting gallery to all section since people mode is no longer valid", - ); - setBarMode("albums"); - setActiveCollectionID(ALL_SECTION); - } - - // Derived1 is async, leading to even more workarounds. - const resolvedBarMode = galleryPeopleState - ? barMode - : barMode == "people" - ? "albums" - : barMode; - const selectAll = (e: KeyboardEvent) => { // ignore ctrl/cmd + a if the user is typing in a text field if ( @@ -736,14 +713,13 @@ export default function Gallery() { count: 0, collectionID: activeCollectionID, context: - resolvedBarMode == "people" && - galleryPeopleState?.activePersonID + barMode == "people" && galleryPeopleState?.activePerson?.id ? { mode: "people" as const, - personID: galleryPeopleState.activePersonID, + personID: galleryPeopleState.activePerson.id, } : { - mode: resolvedBarMode as "albums" | "hidden-albums", + mode: barMode as "albums" | "hidden-albums", collectionID: ensure(activeCollectionID), }, }; @@ -1114,12 +1090,7 @@ export default function Gallery() { }; const handleSelectPerson = (person: Person | undefined) => { - // The person bar currently does not have an "all" mode, so default to - // the first person when no specific person is provided. This can happen - // when the user clicks the "People" header in the search empty state (it - // is guaranteed that this header will only be shown if there is at - // least one person). - setActivePersonID(person?.id ?? galleryPeopleState?.people[0]?.id); + setActivePersonID(person?.id); setBarMode("people"); }; @@ -1214,7 +1185,7 @@ export default function Gallery() { marginBottom: "12px", }} > - {resolvedBarMode == "hidden-albums" ? ( + {barMode == "hidden-albums" ? ( @@ -1235,15 +1206,14 @@ export default function Gallery() { {showEmptySectionState && activeCollectionID === ALL_SECTION ? ( - ) : showEmptySectionState && resolvedBarMode == "people" ? ( + ) : showEmptySectionState && + barMode == "people" && + !galleryPeopleState?.activePerson ? (
Empty
) : ( )} = ({ : { type: "people" as const, people, - activePerson: ensure(activePerson), + activePerson, onSelectPerson, }, [ @@ -404,7 +402,7 @@ type ItemData = | { type: "people"; people: Person[]; - activePerson: Person; + activePerson: Person | undefined; onSelectPerson: (person: Person) => void; }; @@ -570,7 +568,7 @@ const ActiveIndicator = styled("div")` interface PersonCardProps { person: Person; - activePerson: Person; + activePerson: Person | undefined; onSelectPerson: (person: Person) => void; } @@ -588,6 +586,6 @@ const PersonCard: React.FC = ({ > {person.name && } - {activePerson.id === person.id && } + {activePerson?.id === person.id && } ); diff --git a/web/packages/new/photos/components/Gallery/PeopleHeader.tsx b/web/packages/new/photos/components/Gallery/PeopleHeader.tsx index ad6e71925f..8f13eb60c5 100644 --- a/web/packages/new/photos/components/Gallery/PeopleHeader.tsx +++ b/web/packages/new/photos/components/Gallery/PeopleHeader.tsx @@ -40,19 +40,12 @@ import { GalleryItemsHeaderAdapter, GalleryItemsSummary } from "./ListHeader"; */ export interface GalleryPeopleState { /** - * The ID of the currently selected person. + * The currently selected person, if any. * - * We do not have an empty state currently, so this is guaranteed to be - * present whenever the gallery is in the "people" mode. + * Whenever this is present, it is guaranteed to be one of the items from + * within {@link people}. */ - activePersonID: string; - /** - * The currently selected person. - * - * This is a convenience property that contains a direct reference to the - * active {@link Person} from amongst {@link people}. - */ - activePerson: Person; + activePerson: Person | undefined; /** * The list of people to show. */