From c165fad0fbc7887b11f2610008813dd662a1766b Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 10:17:44 +0530 Subject: [PATCH 1/6] Tune --- web/packages/new/photos/components/SearchBar.tsx | 8 +------- web/packages/new/photos/components/gallery/reducer.ts | 9 +++++---- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/web/packages/new/photos/components/SearchBar.tsx b/web/packages/new/photos/components/SearchBar.tsx index 6eea8761143..0cb17204c0b 100644 --- a/web/packages/new/photos/components/SearchBar.tsx +++ b/web/packages/new/photos/components/SearchBar.tsx @@ -7,7 +7,6 @@ import { searchOptionsForString } from "@/new/photos/services/search"; import type { SearchOption } from "@/new/photos/services/search/types"; import { nullToUndefined } from "@/utils/transform"; import CalendarIcon from "@mui/icons-material/CalendarMonth"; -import ChevronRightIcon from "@mui/icons-material/ChevronRight"; import CloseIcon from "@mui/icons-material/Close"; import ImageIcon from "@mui/icons-material/Image"; import LocationIcon from "@mui/icons-material/LocationOn"; @@ -425,12 +424,7 @@ const EmptyState: React.FC> = ({ const SearchPeopleHeader: React.FC = ({ onClick }) => ( - - - {t("people")} - - - + {t("people")} ); diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index 4f58b07c80d..09834aba7b1 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -80,8 +80,9 @@ export type GalleryView = /** * The list of people to show in the gallery bar. * - * Note that this can be different from the underlying list of people, - * and can temporarily include a person from outside that list. + * Note that this can be different from the underlying list of + * visiblePeople in the {@link peopleState}, and can temporarily + * include a person from outside that list. */ visiblePeople: Person[]; /** @@ -1059,10 +1060,10 @@ const derivePeopleView = ( activePerson = findByID(people); if (activePerson) { // Temporarily add this person's entry to the list of people - // surfaced in the people section. + // surfaced in the people view. visiblePeople.push(activePerson); } else { - // We don't have an "All" pseudo-album in people mode, so default to + // We don't have an "All" pseudo-album in people view, so default to // the first person in the list (if any). activePerson = visiblePeople[0]; } From a6d96d542a10ec8748a3fce8b6caf08a9a0f8c57 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 10:30:22 +0530 Subject: [PATCH 2/6] filter in reducer - wip checkpoint --- .../new/photos/components/gallery/reducer.ts | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index 09834aba7b1..e56945a6cda 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -271,7 +271,7 @@ export interface GalleryState { /** * The files to show, uniqued and sorted appropriately. */ - filteredFiles: EnteFile[] | undefined; + filteredFiles: EnteFile[]; } export type GalleryAction = @@ -341,7 +341,7 @@ const initialGalleryState: GalleryState = { selectedPersonID: undefined, searchResults: undefined, view: undefined, - filteredFiles: undefined, + filteredFiles: [], isInSearchMode: false, }; @@ -358,6 +358,12 @@ const galleryReducer: React.Reducer = ( ); const archivedCollectionIDs = deriveArchivedCollectionIDs(collections); + const hiddenFileIDs = deriveHiddenFileIDs(action.hiddenFiles); + const view = { + type: "albums" as const, + activeCollectionSummaryID: ALL_SECTION, + activeCollection: undefined, + } return { ...state, user: action.user, @@ -370,7 +376,7 @@ const galleryReducer: React.Reducer = ( archivedCollectionIDs, defaultHiddenCollectionIDs: deriveDefaultHiddenCollectionIDs(hiddenCollections), - hiddenFileIDs: deriveHiddenFileIDs(action.hiddenFiles), + hiddenFileIDs, favoriteFileIDs: deriveFavoriteFileIDs( collections, action.files, @@ -391,11 +397,15 @@ const galleryReducer: React.Reducer = ( hiddenCollections, action.hiddenFiles, ), - view: { - type: "albums", - activeCollectionSummaryID: ALL_SECTION, - activeCollection: undefined, - }, + view, + filteredFiles: deriveAlbumsFilteredFiles( + action.files, + archivedCollectionIDs, + hiddenFileIDs, + state.tempDeletedFileIDs, + state.tempHiddenFileIDs, + view, + ) }; } case "setNormalCollections": { @@ -1093,18 +1103,14 @@ export const deriveAlbumishFilteredFiles = (state: GalleryState) => { * in the "albums" view. */ export const deriveAlbumsFilteredFiles = ( - state: GalleryState, + files: GalleryState["files"], + archivedCollectionIDs: GalleryState["archivedCollectionIDs"], + hiddenFileIDs: GalleryState["hiddenFileIDs"], + tempDeletedFileIDs: GalleryState["tempDeletedFileIDs"], + tempHiddenFileIDs: GalleryState["tempHiddenFileIDs"], view: Extract, ) => { - const { - files, - archivedCollectionIDs, - hiddenFileIDs, - tempDeletedFileIDs, - tempHiddenFileIDs, - } = state; const activeCollectionSummaryID = view.activeCollectionSummaryID; - const filteredFiles = files.filter((file) => { if (tempDeletedFileIDs.has(file.id)) return false; if (hiddenFileIDs.has(file.id)) return false; @@ -1156,13 +1162,12 @@ export const deriveTrashFilteredFiles = ({ * in the "hidden-albums" view. */ export const deriveHiddenAlbumsFilteredFiles = ( - state: GalleryState, + hiddenFiles: GalleryState["hiddenFiles"], + defaultHiddenCollectionIDs: GalleryState["defaultHiddenCollectionIDs"], + tempDeletedFileIDs: GalleryState["tempDeletedFileIDs"], view: Extract, ) => { - const { hiddenFiles, defaultHiddenCollectionIDs, tempDeletedFileIDs } = - state; const activeCollectionSummaryID = view.activeCollectionSummaryID; - const filteredFiles = hiddenFiles.filter((file) => { if (tempDeletedFileIDs.has(file.id)) return false; From 3f1ee5e7cd2f59c2184a249c6fa3c020af8e0e50 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 11:23:24 +0530 Subject: [PATCH 3/6] wip checkpoint --- .../new/photos/components/gallery/reducer.ts | 194 +++++++++++------- 1 file changed, 120 insertions(+), 74 deletions(-) diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index e56945a6cda..f4d32c05ec2 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -363,7 +363,7 @@ const galleryReducer: React.Reducer = ( type: "albums" as const, activeCollectionSummaryID: ALL_SECTION, activeCollection: undefined, - } + }; return { ...state, user: action.user, @@ -400,19 +400,20 @@ const galleryReducer: React.Reducer = ( view, filteredFiles: deriveAlbumsFilteredFiles( action.files, + action.trashedFiles, archivedCollectionIDs, hiddenFileIDs, state.tempDeletedFileIDs, state.tempHiddenFileIDs, view, - ) + ), }; } case "setNormalCollections": { const archivedCollectionIDs = deriveArchivedCollectionIDs( action.collections, ); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, collections: action.collections, archivedCollectionIDs, @@ -430,13 +431,13 @@ const galleryReducer: React.Reducer = ( state.trashedFiles, archivedCollectionIDs, ), - }; + }); } case "setAllCollections": { const archivedCollectionIDs = deriveArchivedCollectionIDs( action.collections, ); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, collections: action.collections, hiddenCollections: action.hiddenCollections, @@ -463,11 +464,11 @@ const galleryReducer: React.Reducer = ( action.hiddenCollections, state.hiddenFiles, ), - }; + }); } case "setFiles": { const files = sortFiles(mergeMetadata(action.files)); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, files, favoriteFileIDs: deriveFavoriteFileIDs( @@ -482,7 +483,7 @@ const galleryReducer: React.Reducer = ( state.trashedFiles, state.archivedCollectionIDs, ), - }; + }); } case "fetchFiles": { const files = sortFiles( @@ -490,7 +491,7 @@ const galleryReducer: React.Reducer = ( getLatestVersionFiles([...state.files, ...action.files]), ), ); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, files, favoriteFileIDs: deriveFavoriteFileIDs( @@ -505,11 +506,11 @@ const galleryReducer: React.Reducer = ( state.trashedFiles, state.archivedCollectionIDs, ), - }; + }); } case "uploadFile": { const files = sortFiles([...state.files, action.file]); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, files, favoriteFileIDs: deriveFavoriteFileIDs( @@ -526,11 +527,11 @@ const galleryReducer: React.Reducer = ( state.trashedFiles, state.archivedCollectionIDs, ), - }; + }); } case "setHiddenFiles": { const hiddenFiles = sortFiles(mergeMetadata(action.hiddenFiles)); - return { + return refreshingFilteredFilesIfShowingHiddenAlbums({ ...state, hiddenFiles, hiddenFileIDs: deriveHiddenFileIDs(hiddenFiles), @@ -539,7 +540,7 @@ const galleryReducer: React.Reducer = ( state.hiddenCollections, hiddenFiles, ), - }; + }); } case "fetchHiddenFiles": { const hiddenFiles = sortFiles( @@ -550,7 +551,7 @@ const galleryReducer: React.Reducer = ( ]), ), ); - return { + return refreshingFilteredFilesIfShowingHiddenAlbums({ ...state, hiddenFiles, hiddenFileIDs: deriveHiddenFileIDs(hiddenFiles), @@ -559,10 +560,10 @@ const galleryReducer: React.Reducer = ( state.hiddenCollections, hiddenFiles, ), - }; + }); } case "setTrashedFiles": - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, trashedFiles: action.trashedFiles, collectionSummaries: deriveCollectionSummaries( @@ -572,33 +573,39 @@ const galleryReducer: React.Reducer = ( action.trashedFiles, state.archivedCollectionIDs, ), - }; + }); case "setPeopleState": return { ...state, peopleState: action.peopleState }; case "markTempDeleted": - return { + return refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums({ ...state, tempDeletedFileIDs: new Set( [...state.tempDeletedFileIDs].concat( action.files.map((f) => f.id), ), ), - }; + }); case "clearTempDeleted": - return { ...state, tempDeletedFileIDs: new Set() }; + return refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums({ + ...state, + tempDeletedFileIDs: new Set(), + }); case "markTempHidden": - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, tempHiddenFileIDs: new Set( [...state.tempHiddenFileIDs].concat( action.files.map((f) => f.id), ), ), - }; + }); case "clearTempHidden": - return { ...state, tempHiddenFileIDs: new Set() }; + return refreshingFilteredFilesIfShowingAlbums({ + ...state, + tempHiddenFileIDs: new Set(), + }); case "showAll": - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, searchResults: undefined, selectedCollectionSummaryID: undefined, @@ -608,9 +615,9 @@ const galleryReducer: React.Reducer = ( activeCollection: undefined, }, isInSearchMode: false, - }; + }); case "showHidden": - return { + return refreshingFilteredFilesIfShowingHiddenAlbums({ ...state, searchResults: undefined, selectedCollectionSummaryID: undefined, @@ -620,7 +627,7 @@ const galleryReducer: React.Reducer = ( activeCollection: undefined, }, isInSearchMode: false, - }; + }); case "showAlbums": { const { view, selectedCollectionSummaryID } = deriveAlbumsViewAndSelectedID( @@ -628,16 +635,16 @@ const galleryReducer: React.Reducer = ( state.collectionSummaries, state.selectedCollectionSummaryID, ); - return { + return refreshingFilteredFilesIfShowingAlbums({ ...state, searchResults: undefined, selectedCollectionSummaryID, view, isInSearchMode: false, - }; + }); } case "showNormalOrHiddenCollectionSummary": - return { + return refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums({ ...state, searchResults: undefined, selectedCollectionSummaryID: action.collectionSummaryID, @@ -656,7 +663,7 @@ const galleryReducer: React.Reducer = ( .find(({ id }) => id === action.collectionSummaryID), }, isInSearchMode: false, - }; + }); case "showPeople": { const view = derivePeopleView( state.peopleState, @@ -732,7 +739,7 @@ export const uniqueFilesByID = (files: EnteFile[]) => { }; /** - * Helper function to compute archived collection IDs from their dependencies. + * Compute archived collection IDs from their dependencies. */ const deriveArchivedCollectionIDs = (collections: Collection[]) => new Set( @@ -742,20 +749,19 @@ const deriveArchivedCollectionIDs = (collections: Collection[]) => ); /** - * Helper function to compute the default hidden collection IDs from theirq - * dependencies. + * Compute the default hidden collection IDs from their dependencies. */ const deriveDefaultHiddenCollectionIDs = (hiddenCollections: Collection[]) => findDefaultHiddenCollectionIDs(hiddenCollections); /** - * Helper function to compute hidden file IDs from their dependencies. + * Compute hidden file IDs from their dependencies. */ const deriveHiddenFileIDs = (hiddenFiles: EnteFile[]) => new Set(hiddenFiles.map((f) => f.id)); /** - * Helper function to compute favorite file IDs from their dependencies. + * Compute favorite file IDs from their dependencies. */ const deriveFavoriteFileIDs = ( collections: Collection[], @@ -774,7 +780,7 @@ const deriveFavoriteFileIDs = ( }; /** - * Helper function to compute collection summaries from their dependencies. + * Compute collection summaries from their dependencies. */ export const deriveCollectionSummaries = ( user: User, @@ -828,8 +834,7 @@ const pseudoCollectionOptionsForFiles = (files: EnteFile[]) => ({ }); /** - * Helper function to compute hidden collection summaries from their - * dependencies. + * Compute hidden collection summaries from their dependencies. */ export const deriveHiddenCollectionSummaries = ( user: User, @@ -1008,8 +1013,8 @@ const findAllSectionVisibleFiles = ( ); /** - * Helper function to derive the {@link GalleryView} from its dependencies when - * we are switching to (or back to) the "albums" view. + * Compute the {@link GalleryView} from its dependencies when we are switching + * to (or back to) the "albums" view. */ const deriveAlbumsViewAndSelectedID = ( collections: GalleryState["collections"], @@ -1034,8 +1039,8 @@ const deriveAlbumsViewAndSelectedID = ( }; /** - * Helper function to derive the {@link GalleryView} from its dependencies when - * we are switching to (or back to) the "people" view. + * Compute the {@link GalleryView} from its dependencies when we are switching + * to (or back to) the "people" view. */ const derivePeopleView = ( peopleState: GalleryState["peopleState"], @@ -1083,27 +1088,39 @@ const derivePeopleView = ( }; /** - * Helper function to compute the sorted list of files to show when we're - * showing either "albums" or "hidden-albums". + * Return a new state by recomputing the {@link filteredFiles} property if we're + * showing the "albums" view. + * + * Usually, we update state by manually dependency tracking on a fine grained + * basis, but it is cumbersome (and mistake prone) to do that for the list of + * filtered files which depend on a many things. So this is a convenience + * function for recomputing filtered files whenever any bit of the underlying + * state that could affect the "albums" view changes (and we're showing it). */ -export const deriveAlbumishFilteredFiles = (state: GalleryState) => { - const view = state.view; - if (view?.type == "albums") { - return deriveAlbumsFilteredFiles(state, view); - } else if (view?.type == "hidden-albums") { - return deriveHiddenAlbumsFilteredFiles(state, view); +export const refreshingFilteredFilesIfShowingAlbums = (state: GalleryState) => { + if (state.view?.type == "albums") { + const filteredFiles = deriveAlbumsFilteredFiles( + state.files, + state.trashedFiles, + state.archivedCollectionIDs, + state.hiddenFileIDs, + state.tempDeletedFileIDs, + state.tempHiddenFileIDs, + state.view, + ); + return { ...state, filteredFiles }; } else { - // TODO: Setup the types so that we don't come here. - throw new Error("Not implemented"); + return state; } }; /** - * Helper function to compute the sorted list of files to show when we're - * in the "albums" view. + * Compute the sorted list of files to show when we're in the "albums" view and + * the dependencies change. */ -export const deriveAlbumsFilteredFiles = ( +const deriveAlbumsFilteredFiles = ( files: GalleryState["files"], + trashedFiles: GalleryState["trashedFiles"], archivedCollectionIDs: GalleryState["archivedCollectionIDs"], hiddenFileIDs: GalleryState["hiddenFileIDs"], tempDeletedFileIDs: GalleryState["tempDeletedFileIDs"], @@ -1111,6 +1128,15 @@ export const deriveAlbumsFilteredFiles = ( view: Extract, ) => { const activeCollectionSummaryID = view.activeCollectionSummaryID; + + // Trash is dealt with separately. + if (activeCollectionSummaryID === TRASH_SECTION) { + return uniqueFilesByID([ + ...trashedFiles, + ...files.filter((file) => tempDeletedFileIDs.has(file.id)), + ]); + } + const filteredFiles = files.filter((file) => { if (tempDeletedFileIDs.has(file.id)) return false; if (hiddenFileIDs.has(file.id)) return false; @@ -1144,24 +1170,44 @@ export const deriveAlbumsFilteredFiles = ( }; /** - * Helper function to compute the sorted list of files to show when we're - * showing the "Trash". + * Return a new state by recomputing the {@link filteredFiles} property if when + * we're showing the "hidden-albums" view. + * + * See {@link refreshingFilteredFilesIfShowingAlbums} for more details. + */ +export const refreshingFilteredFilesIfShowingHiddenAlbums = ( + state: GalleryState, +) => { + if (state.view?.type == "hidden-albums") { + const filteredFiles = deriveHiddenAlbumsFilteredFiles( + state.hiddenFiles, + state.defaultHiddenCollectionIDs, + state.tempDeletedFileIDs, + state.view, + ); + return { ...state, filteredFiles }; + } else { + return state; + } +}; + +/** + * Convenience method for chaining the refresh functions for "albums" and + * "hidden-albums". This is useful if something that potentially affects both + * scenarios changes. */ -export const deriveTrashFilteredFiles = ({ - files, - trashedFiles, - tempDeletedFileIDs, -}: GalleryState) => - uniqueFilesByID([ - ...trashedFiles, - ...files.filter((file) => tempDeletedFileIDs.has(file.id)), - ]); +export const refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums = ( + state: GalleryState, +) => + refreshingFilteredFilesIfShowingHiddenAlbums( + refreshingFilteredFilesIfShowingAlbums(state), + ); /** - * Helper function to compute the sorted list of files to show when we're - * in the "hidden-albums" view. + * Compute the sorted list of files to show when we're in the "hidden-albums" + * view and the dependencies change. */ -export const deriveHiddenAlbumsFilteredFiles = ( +const deriveHiddenAlbumsFilteredFiles = ( hiddenFiles: GalleryState["hiddenFiles"], defaultHiddenCollectionIDs: GalleryState["defaultHiddenCollectionIDs"], tempDeletedFileIDs: GalleryState["tempDeletedFileIDs"], @@ -1205,10 +1251,10 @@ const sortAndUniqueFilteredFiles = ( }; /** - * Helper function to compute the sorted list of files to show when we're - * in the "people" view. + * Compute the sorted list of files to show when we're in the "people" view and + * the dependencies change. */ -export const derivePeopleFilteredFiles = ( +const derivePeopleFilteredFiles = ( { files }: GalleryState, view: Extract, ) => { From 717cada22dcf46ec3457395da61e9227b33aedc6 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 11:32:35 +0530 Subject: [PATCH 4/6] wip checkpoint --- .../new/photos/components/gallery/reducer.ts | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index f4d32c05ec2..3c03d987444 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -266,6 +266,9 @@ export interface GalleryState { * * That is, {@link isInSearchMode} may be true even when * {@link searchResults} is undefined. + * + * We will be _showing_ search results if both {@link isInSearchMode} is + * `true` and {@link searchResults} is defined. */ isInSearchMode: boolean; /** @@ -313,10 +316,9 @@ export type GalleryAction = } | { type: "showPeople" } | { type: "showPerson"; personID: string } - | { type: "searchResults"; searchResults: EnteFile[] } + | { type: "setSearchResults"; searchResults: EnteFile[] } | { type: "enterSearchMode" } - | { type: "exitSearch" } - | { type: "setFilteredFiles"; filteredFiles: EnteFile[] }; + | { type: "exitSearch" }; const initialGalleryState: GalleryState = { user: undefined, @@ -671,12 +673,14 @@ const galleryReducer: React.Reducer = ( state.tempHiddenFileIDs, state.selectedPersonID, ); + const filteredFiles = derivePeopleFilteredFiles(state.files, view); return { ...state, searchResults: undefined, selectedPersonID: view.activePerson?.id, view, isInSearchMode: false, + filteredFiles, }; } case "showPerson": { @@ -686,20 +690,29 @@ const galleryReducer: React.Reducer = ( state.tempHiddenFileIDs, action.personID, ); + const filteredFiles = derivePeopleFilteredFiles(state.files, view); return { ...state, searchResults: undefined, selectedPersonID: view.activePerson?.id, view, isInSearchMode: false, + filteredFiles, }; } - case "enterSearchMode": - return { ...state, isInSearchMode: true }; - case "searchResults": + case "setSearchResults": return { ...state, searchResults: action.searchResults, + filteredFiles: state.isInSearchMode + ? action.searchResults + : state.filteredFiles, + }; + case "enterSearchMode": + return { + ...state, + isInSearchMode: true, + filteredFiles: state.searchResults ?? state.filteredFiles, }; case "exitSearch": return { @@ -707,8 +720,6 @@ const galleryReducer: React.Reducer = ( isInSearchMode: false, searchResults: undefined, }; - case "setFilteredFiles": - return { ...state, filteredFiles: action.filteredFiles }; } }; @@ -1255,7 +1266,7 @@ const sortAndUniqueFilteredFiles = ( * the dependencies change. */ const derivePeopleFilteredFiles = ( - { files }: GalleryState, + files: GalleryState["files"], view: Extract, ) => { const pfSet = new Set(view.activePerson?.fileIDs ?? []); From 9c8701cd8cd423a4d757efbbb2c9729ed45a57d4 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 11:35:29 +0530 Subject: [PATCH 5/6] Use --- web/apps/photos/src/pages/gallery.tsx | 71 +++---------------- .../new/photos/components/gallery/reducer.ts | 18 +++-- 2 files changed, 19 insertions(+), 70 deletions(-) diff --git a/web/apps/photos/src/pages/gallery.tsx b/web/apps/photos/src/pages/gallery.tsx index 59966ed4f3a..9c288e67c7f 100644 --- a/web/apps/photos/src/pages/gallery.tsx +++ b/web/apps/photos/src/pages/gallery.tsx @@ -20,9 +20,6 @@ import { SearchResultsHeader, } from "@/new/photos/components/gallery"; import { - deriveAlbumishFilteredFiles, - derivePeopleFilteredFiles, - deriveTrashFilteredFiles, uniqueFilesByID, useGalleryReducer, type GalleryBarMode, @@ -32,7 +29,6 @@ import { shouldShowWhatsNew } from "@/new/photos/services/changelog"; import { ALL_SECTION, DUMMY_UNCATEGORIZED_COLLECTION, - TRASH_SECTION, isHiddenCollection, } from "@/new/photos/services/collection"; import { areOnlySystemCollections } from "@/new/photos/services/collection/ui"; @@ -306,15 +302,7 @@ export default function Gallery() { const collections = state.collections; const files = state.files; const hiddenFiles = state.hiddenFiles; - const trashedFiles = state.trashedFiles; - const archivedCollectionIDs = state.archivedCollectionIDs; - const hiddenFileIDs = state.hiddenFileIDs; - const collectionNameMap = state.allCollectionNameByID; - const fileToCollectionsMap = state.fileCollectionIDs; const collectionSummaries = state.collectionSummaries; - const hiddenCollectionSummaries = state.hiddenCollectionSummaries; - const tempDeletedFileIDs = state.tempDeletedFileIDs; - const tempHiddenFileIDs = state.tempHiddenFileIDs; const barMode = state.view?.type ?? "albums"; const activeCollectionID = state.view?.type == "people" @@ -328,7 +316,7 @@ export default function Gallery() { const isInSearchMode = state.isInSearchMode; const filteredFiles = state.filteredFiles; - if (process.env.NEXT_PUBLIC_ENTE_WIP_CL) console.log("render", { state }); + if (process.env.NEXT_PUBLIC_ENTE_WIP_CL) console.log("render", state); const router = useRouter(); @@ -483,51 +471,13 @@ export default function Gallery() { // TODO: Make this a normal useEffect. useMemoSingleThreaded(async () => { - if ( - !files || - !user || - !trashedFiles || - !hiddenFiles || - !archivedCollectionIDs - ) { - dispatch({ - type: "setFilteredFiles", - filteredFiles: [], - }); - return; - } - - let filteredFiles: EnteFile[]; if (selectedSearchOption) { - filteredFiles = await filterSearchableFiles( + const searchResults = await filterSearchableFiles( selectedSearchOption.suggestion, ); - } else if (state.view?.type == "people") { - filteredFiles = derivePeopleFilteredFiles(state, state.view); - } else if (activeCollectionID === TRASH_SECTION) { - filteredFiles = deriveTrashFilteredFiles(state); - } else { - filteredFiles = deriveAlbumishFilteredFiles(state); + dispatch({ type: "setSearchResults", searchResults }); } - - dispatch({ - type: "setFilteredFiles", - filteredFiles, - }); - }, [ - barMode, - files, - trashedFiles, - hiddenFiles, - tempDeletedFileIDs, - tempHiddenFileIDs, - hiddenFileIDs, - selectedSearchOption, - activeCollectionID, - archivedCollectionIDs, - peopleState, - activePersonID, - ]); + }, [selectedSearchOption]); const selectAll = (e: KeyboardEvent) => { // ignore ctrl/cmd + a if the user is typing in a text field @@ -923,8 +873,8 @@ export default function Gallery() { [], ); - if (!user || !filteredFiles) { - // Don't render until we get the logged in user and dispatch "mount". + if (!user) { + // Don't render until we dispatch "mount" with the logged in user. return
; } @@ -1046,7 +996,8 @@ export default function Gallery() { activeCollection, activeCollectionID, setActiveCollectionID: handleSetActiveCollectionID, - hiddenCollectionSummaries, + hiddenCollectionSummaries: + state.hiddenCollectionSummaries, showPeopleSectionButton, people: (state.view.type == "people" @@ -1129,8 +1080,8 @@ export default function Gallery() { activeCollectionID={activeCollectionID} activePersonID={activePerson?.id} enableDownload={true} - fileToCollectionsMap={fileToCollectionsMap} - collectionNameMap={collectionNameMap} + fileToCollectionsMap={state.fileCollectionIDs} + collectionNameMap={state.allCollectionNameByID} showAppDownloadBanner={ files.length < 30 && !isInSearchMode } @@ -1185,7 +1136,7 @@ export default function Gallery() { ({ /** * Compute hidden collection summaries from their dependencies. */ -export const deriveHiddenCollectionSummaries = ( +const deriveHiddenCollectionSummaries = ( user: User, hiddenCollections: Collection[], hiddenFiles: EnteFile[], @@ -1079,11 +1079,11 @@ const derivePeopleView = ( people = filterTemp(people); visiblePeople = filterTemp(visiblePeople); } - const findByID = (ps: Person[]) => ps.find((p) => p.id == selectedPersonID); - let activePerson = findByID(visiblePeople); + const findByIDIn = (ps: Person[]) => ps.find((p) => p.id == selectedPersonID); + let activePerson = findByIDIn(visiblePeople); if (!activePerson) { // This might be one of the normally hidden small clusters. - activePerson = findByID(people); + activePerson = findByIDIn(people); if (activePerson) { // Temporarily add this person's entry to the list of people // surfaced in the people view. @@ -1108,7 +1108,7 @@ const derivePeopleView = ( * function for recomputing filtered files whenever any bit of the underlying * state that could affect the "albums" view changes (and we're showing it). */ -export const refreshingFilteredFilesIfShowingAlbums = (state: GalleryState) => { +const refreshingFilteredFilesIfShowingAlbums = (state: GalleryState) => { if (state.view?.type == "albums") { const filteredFiles = deriveAlbumsFilteredFiles( state.files, @@ -1186,9 +1186,7 @@ const deriveAlbumsFilteredFiles = ( * * See {@link refreshingFilteredFilesIfShowingAlbums} for more details. */ -export const refreshingFilteredFilesIfShowingHiddenAlbums = ( - state: GalleryState, -) => { +const refreshingFilteredFilesIfShowingHiddenAlbums = (state: GalleryState) => { if (state.view?.type == "hidden-albums") { const filteredFiles = deriveHiddenAlbumsFilteredFiles( state.hiddenFiles, @@ -1207,7 +1205,7 @@ export const refreshingFilteredFilesIfShowingHiddenAlbums = ( * "hidden-albums". This is useful if something that potentially affects both * scenarios changes. */ -export const refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums = ( +const refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums = ( state: GalleryState, ) => refreshingFilteredFilesIfShowingHiddenAlbums( From c8b057cf9d8df26d08168b4e3fe2883b92f56a04 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Wed, 23 Oct 2024 12:10:51 +0530 Subject: [PATCH 6/6] Deterministically handle the extra entry --- .../new/photos/components/gallery/reducer.ts | 53 +++++++++++++++---- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/web/packages/new/photos/components/gallery/reducer.ts b/web/packages/new/photos/components/gallery/reducer.ts index d679a26e9ee..4557cf403ec 100644 --- a/web/packages/new/photos/components/gallery/reducer.ts +++ b/web/packages/new/photos/components/gallery/reducer.ts @@ -237,6 +237,14 @@ export interface GalleryState { * possible) on switching back. */ selectedPersonID: string | undefined; + /** + * If present, this person is tacked on the the list of visible people + * temporarily (until the user switches out from the people view). + * + * This is needed to retain a usually non-visible but temporarily selected + * person in the people bar until the user switches to some other view. + */ + extraVisiblePerson: Person | undefined; /** * List of files that match the selected search option. * @@ -341,6 +349,7 @@ const initialGalleryState: GalleryState = { tempHiddenFileIDs: new Set(), selectedCollectionSummaryID: undefined, selectedPersonID: undefined, + extraVisiblePerson: undefined, searchResults: undefined, view: undefined, filteredFiles: [], @@ -609,8 +618,9 @@ const galleryReducer: React.Reducer = ( case "showAll": return refreshingFilteredFilesIfShowingAlbums({ ...state, - searchResults: undefined, selectedCollectionSummaryID: undefined, + extraVisiblePerson: undefined, + searchResults: undefined, view: { type: "albums", activeCollectionSummaryID: ALL_SECTION, @@ -621,8 +631,9 @@ const galleryReducer: React.Reducer = ( case "showHidden": return refreshingFilteredFilesIfShowingHiddenAlbums({ ...state, - searchResults: undefined, selectedCollectionSummaryID: undefined, + extraVisiblePerson: undefined, + searchResults: undefined, view: { type: "hidden-albums", activeCollectionSummaryID: HIDDEN_ITEMS_SECTION, @@ -639,8 +650,9 @@ const galleryReducer: React.Reducer = ( ); return refreshingFilteredFilesIfShowingAlbums({ ...state, - searchResults: undefined, selectedCollectionSummaryID, + extraVisiblePerson: undefined, + searchResults: undefined, view, isInSearchMode: false, }); @@ -648,8 +660,9 @@ const galleryReducer: React.Reducer = ( case "showNormalOrHiddenCollectionSummary": return refreshingFilteredFilesIfShowingAlbumsOrHiddenAlbums({ ...state, - searchResults: undefined, selectedCollectionSummaryID: action.collectionSummaryID, + extraVisiblePerson: undefined, + searchResults: undefined, view: { type: action.collectionSummaryID !== undefined && @@ -667,34 +680,38 @@ const galleryReducer: React.Reducer = ( isInSearchMode: false, }); case "showPeople": { - const view = derivePeopleView( + const { view, extraVisiblePerson } = derivePeopleView( state.peopleState, state.tempDeletedFileIDs, state.tempHiddenFileIDs, state.selectedPersonID, + state.extraVisiblePerson, ); const filteredFiles = derivePeopleFilteredFiles(state.files, view); return { ...state, - searchResults: undefined, selectedPersonID: view.activePerson?.id, + extraVisiblePerson, + searchResults: undefined, view, isInSearchMode: false, filteredFiles, }; } case "showPerson": { - const view = derivePeopleView( + const { view, extraVisiblePerson } = derivePeopleView( state.peopleState, state.tempDeletedFileIDs, state.tempHiddenFileIDs, action.personID, + state.extraVisiblePerson, ); const filteredFiles = derivePeopleFilteredFiles(state.files, view); return { ...state, searchResults: undefined, selectedPersonID: view.activePerson?.id, + extraVisiblePerson, view, isInSearchMode: false, filteredFiles, @@ -1058,7 +1075,11 @@ const derivePeopleView = ( tempDeletedFileIDs: GalleryState["tempDeletedFileIDs"], tempHiddenFileIDs: GalleryState["tempHiddenFileIDs"], selectedPersonID: GalleryState["selectedPersonID"], -): Extract => { + extraVisiblePerson: GalleryState["extraVisiblePerson"], +): { + view: Extract; + extraVisiblePerson: GalleryState["extraVisiblePerson"]; +} => { let people = peopleState?.people ?? []; let visiblePeople = peopleState?.visiblePeople ?? []; if (tempDeletedFileIDs.size + tempHiddenFileIDs.size > 0) { @@ -1079,7 +1100,9 @@ const derivePeopleView = ( people = filterTemp(people); visiblePeople = filterTemp(visiblePeople); } - const findByIDIn = (ps: Person[]) => ps.find((p) => p.id == selectedPersonID); + + const findByIDIn = (ps: Person[]) => + ps.find((p) => p.id == selectedPersonID); let activePerson = findByIDIn(visiblePeople); if (!activePerson) { // This might be one of the normally hidden small clusters. @@ -1087,7 +1110,7 @@ const derivePeopleView = ( if (activePerson) { // Temporarily add this person's entry to the list of people // surfaced in the people view. - visiblePeople.push(activePerson); + extraVisiblePerson = activePerson; } else { // We don't have an "All" pseudo-album in people view, so default to // the first person in the list (if any). @@ -1095,7 +1118,15 @@ const derivePeopleView = ( } } - return { type: "people", visiblePeople, activePerson }; + const view = { + type: "people" as const, + visiblePeople: extraVisiblePerson + ? visiblePeople.concat([extraVisiblePerson]) + : visiblePeople, + activePerson, + }; + + return { view, extraVisiblePerson }; }; /**