From f1e3093ae31ee6a82b8ff40265bf0211ecaa6c59 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:04:15 +0100 Subject: [PATCH] Merge pull request #35268 from overleaf/dp-frontend-routing [web] store project dashboard navigation state in the URL GitOrigin-RevId: 76d95c5706fc196dac7c362e186c66c5334f35ac --- services/web/app/src/router.mjs | 31 ++- .../context/project-list-context.tsx | 55 ++++- .../project-list/util/navigation-state.ts | 116 +++++++++ .../test/acceptance/src/ProjectCRUDTests.mjs | 20 ++ services/web/test/frontend/bootstrap.js | 10 + .../components/project-list-root.test.tsx | 6 +- .../unit/navigation-state.test.ts | 223 ++++++++++++++++++ 7 files changed, 443 insertions(+), 18 deletions(-) create mode 100644 services/web/frontend/js/features/project-list/util/navigation-state.ts create mode 100644 services/web/test/frontend/features/project-list/unit/navigation-state.test.ts diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 251066a07d..ed73e7bfa4 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -525,15 +525,30 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { ProjectController.projectEntitiesJson ) - webRouter.get( - '/project', - AuthenticationController.requireLogin(), - RateLimiterMiddleware.rateLimit(rateLimiters.openDashboard), - AsyncLocalStorage.middleware, - await Modules.middleware('domainCaptureTestSession'), - PermissionsController.useCapabilities(), - ProjectListController.projectListPage + // All project dashboard navigation states render the same page. The active + // navigation item is derived from the URL on the frontend. + const domainCaptureTestSessionMiddleware = await Modules.middleware( + 'domainCaptureTestSession' ) + for (const projectDashboardRoute of [ + '/project', + '/project/owned', + '/project/shared', + '/project/archived', + '/project/trashed', + '/project/untagged', + '/project/tags/:tag', + ]) { + webRouter.get( + projectDashboardRoute, + AuthenticationController.requireLogin(), + RateLimiterMiddleware.rateLimit(rateLimiters.openDashboard), + AsyncLocalStorage.middleware, + domainCaptureTestSessionMiddleware, + PermissionsController.useCapabilities(), + ProjectListController.projectListPage + ) + } webRouter.post( '/project/new', AuthenticationController.requireLogin(), diff --git a/services/web/frontend/js/features/project-list/context/project-list-context.tsx b/services/web/frontend/js/features/project-list/context/project-list-context.tsx index cb0bce5715..6e46eb3742 100644 --- a/services/web/frontend/js/features/project-list/context/project-list-context.tsx +++ b/services/web/frontend/js/features/project-list/context/project-list-context.tsx @@ -25,11 +25,16 @@ import { Project, Sort, } from '../../../../../types/project/dashboard/api' -import usePersistedState from '../../../shared/hooks/use-persisted-state' import getMeta from '../../../utils/meta' import useAsync from '../../../shared/hooks/use-async' import { getProjects } from '../util/api' import sortProjects from '../util/sort-projects' +import { + getInitialNavigationState, + getNavigationState, + getNavigationUrl, + migrateLegacyNavigationState, +} from '../util/navigation-state' import { isArchivedOrTrashed, isDeletableProject, @@ -135,14 +140,18 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { by: 'lastUpdated', order: 'desc', }) - const [filter, setFilter] = usePersistedState( - 'project-list-filter', - 'all' + const [initialNavigationState] = useState(getInitialNavigationState) + const [filter, setFilter] = useState( + initialNavigationState.type === 'filter' + ? initialNavigationState.filter + : 'all' ) const prevSortRef = useRef(sort) - const [selectedTagId, setSelectedTagId] = usePersistedState< - string | undefined - >('project-list-selected-tag-id', undefined) + const [selectedTagId, setSelectedTagId] = useState( + initialNavigationState.type === 'tag' + ? initialNavigationState.tag + : undefined + ) const [showCustomPicker, setShowCustomPicker] = useState(false) const olTags = getMeta('ol-tags') || [] @@ -255,6 +264,28 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { prevSortRef.current = sort }, [sort]) + useEffect(() => { + const handlePopState = () => { + const navigationState = getNavigationState(window.location.pathname) + if (navigationState.type === 'tag') { + setFilter('all') + setSelectedTagId(navigationState.tag) + } else { + setFilter(navigationState.filter) + setSelectedTagId(undefined) + } + setSelectedProjectIds(new Set()) + } + window.addEventListener('popstate', handlePopState) + return () => window.removeEventListener('popstate', handlePopState) + }, []) + + // Temporary: migrate the legacy local-storage navigation state to the URL on + // first load. Safe to remove around September 2026 (see navigation-state.ts). + useEffect(() => { + migrateLegacyNavigationState() + }, []) + const showAllProjects = useCallback(() => { setLoadMoreCount(0) setHiddenProjectsCount(0) @@ -334,6 +365,11 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { setSelectedTagId(undefined) const selected = false selectOrUnselectAllProjects(selected) + window.history.pushState( + null, + '', + getNavigationUrl({ type: 'filter', filter }) + ) }, [selectOrUnselectAllProjects, setFilter, setSelectedTagId] ) @@ -342,6 +378,11 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { (tagId: string) => { setFilter('all') setSelectedTagId(tagId) + window.history.pushState( + null, + '', + getNavigationUrl({ type: 'tag', tag: tagId }) + ) }, [setSelectedTagId, setFilter] ) diff --git a/services/web/frontend/js/features/project-list/util/navigation-state.ts b/services/web/frontend/js/features/project-list/util/navigation-state.ts new file mode 100644 index 0000000000..0a519a30c5 --- /dev/null +++ b/services/web/frontend/js/features/project-list/util/navigation-state.ts @@ -0,0 +1,116 @@ +import { Filter, UNCATEGORIZED_KEY } from '../context/project-list-context' +import customLocalStorage from '../../../infrastructure/local-storage' + +export const DASHBOARD_PATH = '/project' + +export type NavigationState = + | { type: 'filter'; filter: Filter } + | { type: 'tag'; tag: string } + +function isDashboardBasePath(pathname: string): boolean { + return pathname.replace(/\/$/, '') === DASHBOARD_PATH +} + +// Each non-default filter maps to a URL segment of the same name (e.g. +// `owned` -> `/project/owned`); the `all` filter is the bare dashboard path. +const FILTER_SEGMENTS: Exclude[] = [ + 'owned', + 'shared', + 'archived', + 'trashed', +] + +function isFilterSegment(segment: string): segment is Exclude { + return (FILTER_SEGMENTS as string[]).includes(segment) +} + +export function getNavigationState(pathname: string): NavigationState { + const path = pathname.replace(/\/$/, '') + const rest = path.slice(DASHBOARD_PATH.length).replace(/^\//, '') + + if (rest === 'untagged') { + return { type: 'tag', tag: UNCATEGORIZED_KEY } + } + + const tagMatch = rest.match(/^tags\/(.+)$/) + if (tagMatch) { + try { + return { type: 'tag', tag: decodeURIComponent(tagMatch[1]) } + } catch { + return { type: 'filter', filter: 'all' } + } + } + + if (isFilterSegment(rest)) { + return { type: 'filter', filter: rest } + } + + return { type: 'filter', filter: 'all' } +} + +export function getNavigationUrl(state: NavigationState): string { + if (state.type === 'tag') { + if (state.tag === UNCATEGORIZED_KEY) { + return `${DASHBOARD_PATH}/untagged` + } + return `${DASHBOARD_PATH}/tags/${encodeURIComponent(state.tag)}` + } + if (state.filter !== 'all') { + return `${DASHBOARD_PATH}/${state.filter}` + } + return DASHBOARD_PATH +} + +// --- Temporary migration code --------------------------------------------- +// The navigation state used to be persisted in local storage. To smooth the +// transition to URL-based navigation, when a user lands on the bare `/project` +// path we honour their stored state once and then delete it, so their first +// load after this change keeps them where they left off. +// +// This is one-time-per-user migration code. It can be removed (along with the +// `Legacy` helpers below) once the local-storage keys have aged out of active +// users — around September 2026. Introduced for #35214. +const LEGACY_FILTER_KEY = 'project-list-filter' +const LEGACY_SELECTED_TAG_ID_KEY = 'project-list-selected-tag-id' + +function getLegacyNavigationState(): NavigationState | null { + const selectedTagId = customLocalStorage.getItem(LEGACY_SELECTED_TAG_ID_KEY) + if (typeof selectedTagId === 'string') { + return { type: 'tag', tag: selectedTagId } + } + const filter = customLocalStorage.getItem(LEGACY_FILTER_KEY) + if (typeof filter === 'string' && isFilterSegment(filter)) { + return { type: 'filter', filter } + } + return null +} + +function clearLegacyNavigationState(): void { + customLocalStorage.removeItem(LEGACY_FILTER_KEY) + customLocalStorage.removeItem(LEGACY_SELECTED_TAG_ID_KEY) +} + +// Resolve the initial navigation state, honouring the legacy local-storage +// state once on the bare dashboard path. Read-only: the stored state is not +// cleared here (see migrateLegacyNavigationState). +export function getInitialNavigationState(): NavigationState { + if (isDashboardBasePath(window.location.pathname)) { + const legacyState = getLegacyNavigationState() + if (legacyState) { + return legacyState + } + } + return getNavigationState(window.location.pathname) +} + +// Reflect any legacy state in the URL (via replaceState, so the back button is +// not affected) and then delete it. Call once on mount. +export function migrateLegacyNavigationState(): void { + if (isDashboardBasePath(window.location.pathname)) { + const legacyState = getLegacyNavigationState() + if (legacyState) { + window.history.replaceState(null, '', getNavigationUrl(legacyState)) + } + } + clearLegacyNavigationState() +} diff --git a/services/web/test/acceptance/src/ProjectCRUDTests.mjs b/services/web/test/acceptance/src/ProjectCRUDTests.mjs index 0a95d254b3..5b232eb86e 100644 --- a/services/web/test/acceptance/src/ProjectCRUDTests.mjs +++ b/services/web/test/acceptance/src/ProjectCRUDTests.mjs @@ -94,6 +94,26 @@ describe('Project CRUD', function () { }) }) + describe('dashboard navigation-state routes', function () { + const dashboardRoutes = [ + '/project', + '/project/owned', + '/project/shared', + '/project/archived', + '/project/trashed', + '/project/untagged', + '/project/tags/aaaaaaaaaaaaaaaaaaaaaaaa', + ] + + for (const route of dashboardRoutes) { + it(`should render the project dashboard for ${route}`, async function () { + const { response, body } = await this.user.doRequest('GET', route) + expect(response.statusCode).to.equal(200) + expect(body).to.include('name="ol-prefetchedProjectsBlob"') + }) + } + }) + describe("when project doesn't exist", function () { it('should return 404', async function () { const { response } = await this.user.doRequest( diff --git a/services/web/test/frontend/bootstrap.js b/services/web/test/frontend/bootstrap.js index b8b3f59f31..c21d7b70b4 100644 --- a/services/web/test/frontend/bootstrap.js +++ b/services/web/test/frontend/bootstrap.js @@ -122,3 +122,13 @@ Object.defineProperty(navigator, 'onLine', { configurable: true, get: () => true, }) + +// Reset the URL after each test. Some features (e.g. the project dashboard) +// push navigation state into the URL via history.pushState; because the jsdom +// window is shared across the whole run, a dirty path would otherwise leak into +// the next test's initial state and into the `page` field of analytics events. +exports.mochaHooks = { + afterEach() { + window.history.replaceState(null, '', '/') + }, +} diff --git a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx index 264c94b67d..1f71fd16a1 100644 --- a/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx +++ b/services/web/test/frontend/features/project-list/components/project-list-root.test.tsx @@ -881,7 +881,7 @@ describe('', function () { 'project-list-page-interaction', { action: 'rename', - page: '/', + page: '/project', projectId: copyableProject.id, isSmallDevice: true, } @@ -1022,7 +1022,7 @@ describe('', function () { 'project-list-page-interaction', { action: 'clone', - page: '/', + page: '/project', projectId: archiveableProject.id, isSmallDevice: true, } @@ -1187,7 +1187,7 @@ describe('', function () { 'project-list-page-interaction', { action: 'clone', - page: '/', + page: '/project/shared', projectId: archiveableProject.id, isSmallDevice: true, } diff --git a/services/web/test/frontend/features/project-list/unit/navigation-state.test.ts b/services/web/test/frontend/features/project-list/unit/navigation-state.test.ts new file mode 100644 index 0000000000..02c40bc383 --- /dev/null +++ b/services/web/test/frontend/features/project-list/unit/navigation-state.test.ts @@ -0,0 +1,223 @@ +import { expect } from 'chai' +import { + getInitialNavigationState, + getNavigationState, + getNavigationUrl, + migrateLegacyNavigationState, +} from '../../../../../frontend/js/features/project-list/util/navigation-state' +import { UNCATEGORIZED_KEY } from '../../../../../frontend/js/features/project-list/context/project-list-context' + +const LEGACY_FILTER_KEY = 'project-list-filter' +const LEGACY_SELECTED_TAG_ID_KEY = 'project-list-selected-tag-id' + +describe('navigation-state', function () { + describe('getNavigationState', function () { + it('maps the bare dashboard path to the default filter', function () { + expect(getNavigationState('/project')).to.deep.equal({ + type: 'filter', + filter: 'all', + }) + }) + + it('ignores a trailing slash', function () { + expect(getNavigationState('/project/')).to.deep.equal({ + type: 'filter', + filter: 'all', + }) + }) + + it('maps filter segments to filters', function () { + expect(getNavigationState('/project/owned')).to.deep.equal({ + type: 'filter', + filter: 'owned', + }) + expect(getNavigationState('/project/shared')).to.deep.equal({ + type: 'filter', + filter: 'shared', + }) + expect(getNavigationState('/project/archived')).to.deep.equal({ + type: 'filter', + filter: 'archived', + }) + expect(getNavigationState('/project/trashed')).to.deep.equal({ + type: 'filter', + filter: 'trashed', + }) + }) + + it('maps the untagged path to the uncategorized tag', function () { + expect(getNavigationState('/project/untagged')).to.deep.equal({ + type: 'tag', + tag: UNCATEGORIZED_KEY, + }) + }) + + it('maps a tag path to the tag id', function () { + expect(getNavigationState('/project/tags/abc123')).to.deep.equal({ + type: 'tag', + tag: 'abc123', + }) + }) + + it('decodes the tag id', function () { + expect(getNavigationState('/project/tags/a%2Fb')).to.deep.equal({ + type: 'tag', + tag: 'a/b', + }) + }) + + it('falls back to the default filter for unknown paths', function () { + expect(getNavigationState('/project/nonsense')).to.deep.equal({ + type: 'filter', + filter: 'all', + }) + }) + }) + + describe('getNavigationUrl', function () { + it('maps the default filter to the bare dashboard path', function () { + expect(getNavigationUrl({ type: 'filter', filter: 'all' })).to.equal( + '/project' + ) + }) + + it('maps filters to filter segments', function () { + expect(getNavigationUrl({ type: 'filter', filter: 'owned' })).to.equal( + '/project/owned' + ) + expect(getNavigationUrl({ type: 'filter', filter: 'shared' })).to.equal( + '/project/shared' + ) + expect(getNavigationUrl({ type: 'filter', filter: 'archived' })).to.equal( + '/project/archived' + ) + expect(getNavigationUrl({ type: 'filter', filter: 'trashed' })).to.equal( + '/project/trashed' + ) + }) + + it('maps the uncategorized tag to the untagged path', function () { + expect( + getNavigationUrl({ type: 'tag', tag: UNCATEGORIZED_KEY }) + ).to.equal('/project/untagged') + }) + + it('maps a tag id to a tag path', function () { + expect(getNavigationUrl({ type: 'tag', tag: 'abc123' })).to.equal( + '/project/tags/abc123' + ) + }) + + it('encodes the tag id', function () { + expect(getNavigationUrl({ type: 'tag', tag: 'a/b' })).to.equal( + '/project/tags/a%2Fb' + ) + }) + }) + + describe('legacy local-storage migration', function () { + const setLegacyFilter = (value: string) => + localStorage.setItem(LEGACY_FILTER_KEY, JSON.stringify(value)) + const setLegacyTag = (value: string) => + localStorage.setItem(LEGACY_SELECTED_TAG_ID_KEY, JSON.stringify(value)) + + beforeEach(function () { + localStorage.clear() + window.history.replaceState(null, '', '/project') + }) + + afterEach(function () { + localStorage.clear() + window.history.replaceState(null, '', '/project') + }) + + describe('getInitialNavigationState', function () { + it('uses the URL state when there is no legacy state', function () { + window.history.replaceState(null, '', '/project/owned') + expect(getInitialNavigationState()).to.deep.equal({ + type: 'filter', + filter: 'owned', + }) + }) + + it('restores a legacy filter on the bare dashboard path', function () { + setLegacyFilter('trashed') + expect(getInitialNavigationState()).to.deep.equal({ + type: 'filter', + filter: 'trashed', + }) + }) + + it('restores a legacy selected tag on the bare dashboard path', function () { + setLegacyTag('abc123') + expect(getInitialNavigationState()).to.deep.equal({ + type: 'tag', + tag: 'abc123', + }) + }) + + it('prefers a legacy tag over a legacy filter', function () { + setLegacyFilter('trashed') + setLegacyTag(UNCATEGORIZED_KEY) + expect(getInitialNavigationState()).to.deep.equal({ + type: 'tag', + tag: UNCATEGORIZED_KEY, + }) + }) + + it('ignores legacy state when the URL already has a navigation state', function () { + window.history.replaceState(null, '', '/project/shared') + setLegacyFilter('trashed') + expect(getInitialNavigationState()).to.deep.equal({ + type: 'filter', + filter: 'shared', + }) + }) + + it('ignores an invalid legacy filter', function () { + setLegacyFilter('nonsense') + expect(getInitialNavigationState()).to.deep.equal({ + type: 'filter', + filter: 'all', + }) + }) + + it('does not mutate the URL or clear the legacy state', function () { + setLegacyFilter('trashed') + getInitialNavigationState() + expect(window.location.pathname).to.equal('/project') + expect(localStorage.getItem(LEGACY_FILTER_KEY)).to.not.equal(null) + }) + }) + + describe('migrateLegacyNavigationState', function () { + it('rewrites the URL to the legacy state and clears it', function () { + setLegacyFilter('trashed') + migrateLegacyNavigationState() + expect(window.location.pathname).to.equal('/project/trashed') + expect(localStorage.getItem(LEGACY_FILTER_KEY)).to.equal(null) + expect(localStorage.getItem(LEGACY_SELECTED_TAG_ID_KEY)).to.equal(null) + }) + + it('rewrites the URL to a legacy tag state', function () { + setLegacyTag('abc123') + migrateLegacyNavigationState() + expect(window.location.pathname).to.equal('/project/tags/abc123') + expect(localStorage.getItem(LEGACY_SELECTED_TAG_ID_KEY)).to.equal(null) + }) + + it('clears legacy state without rewriting on a non-bare path', function () { + window.history.replaceState(null, '', '/project/owned') + setLegacyFilter('trashed') + migrateLegacyNavigationState() + expect(window.location.pathname).to.equal('/project/owned') + expect(localStorage.getItem(LEGACY_FILTER_KEY)).to.equal(null) + }) + + it('leaves the URL unchanged when there is no legacy state', function () { + migrateLegacyNavigationState() + expect(window.location.pathname).to.equal('/project') + }) + }) + }) +})