From 8438de1167ab77f8e9addbb51de98ff0fcd58243 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Fri, 16 Sep 2022 08:35:17 -0500 Subject: [PATCH] Merge pull request #9589 from overleaf/jel-dash-clone-sort [web] Sort cloned project on dash and maintain sort across filters GitOrigin-RevId: 011bbada85384aa608777c3bf6c680b794f04d70 --- .../components/table/project-list-table.tsx | 39 +---------------- .../context/project-list-context.tsx | 29 +++++++++---- .../{sort-comparators.ts => sort-projects.ts} | 30 ++++++++++++- .../components/sidebar/tags-list.test.tsx | 42 ++++--------------- ...parators.test.ts => sort-projects.test.ts} | 2 +- 5 files changed, 59 insertions(+), 83 deletions(-) rename services/web/frontend/js/features/project-list/util/{sort-comparators.ts => sort-projects.ts} (64%) rename services/web/test/frontend/features/project-list/unit/{sort-comparators.test.ts => sort-projects.test.ts} (99%) diff --git a/services/web/frontend/js/features/project-list/components/table/project-list-table.tsx b/services/web/frontend/js/features/project-list/components/table/project-list-table.tsx index 3633d08e9f..37824c33f4 100644 --- a/services/web/frontend/js/features/project-list/components/table/project-list-table.tsx +++ b/services/web/frontend/js/features/project-list/components/table/project-list-table.tsx @@ -1,12 +1,7 @@ -import { useEffect } from 'react' import { useTranslation } from 'react-i18next' import Icon from '../../../../shared/components/icon' import ProjectListTableRow from './project-list-table-row' import { useProjectListContext } from '../../context/project-list-context' -import { - ownerNameComparator, - defaultComparator, -} from '../../util/sort-comparators' import { Project, Sort } from '../../../../../../types/project/dashboard/api' import { SortingOrder } from '../../../../../../types/sorting-order' @@ -42,41 +37,9 @@ const toggleSort = (order: SortingOrder): SortingOrder => { return order === 'asc' ? 'desc' : 'asc' } -const order = (order: SortingOrder, projects: Project[]) => { - return order === 'asc' ? [...projects] : projects.reverse() -} - function ProjectListTable() { const { t } = useTranslation() - const { visibleProjects, setVisibleProjects, sort, setSort } = - useProjectListContext() - - useEffect(() => { - if (sort.by === 'title') { - setVisibleProjects(prevProjects => { - const sorted = [...prevProjects].sort((...args) => { - return defaultComparator(...args, 'name') - }) - return order(sort.order, sorted) - }) - } - - if (sort.by === 'lastUpdated') { - setVisibleProjects(prevProjects => { - const sorted = [...prevProjects].sort((...args) => { - return defaultComparator(...args, 'lastUpdated') - }) - return order(sort.order, sorted) - }) - } - - if (sort.by === 'owner') { - setVisibleProjects(prevProjects => { - const sorted = [...prevProjects].sort(ownerNameComparator) - return order(sort.order, sorted) - }) - } - }, [sort.by, sort.order, setVisibleProjects]) + const { visibleProjects, sort, setSort } = useProjectListContext() const handleSortClick = (by: Sort['by']) => { setSort(prev => ({ 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 82e9f3ce03..9537d5fa41 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 @@ -6,6 +6,7 @@ import { useContext, useEffect, useMemo, + useRef, useState, } from 'react' import { Tag } from '../../../../../app/src/Features/Tags/types' @@ -18,6 +19,7 @@ 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' export type Filter = 'all' | 'owned' | 'shared' | 'archived' | 'trashed' type FilterMap = { @@ -52,7 +54,6 @@ export const UNCATEGORIZED_KEY = 'uncategorized' type ProjectListContextValue = { addClonedProjectToViewData: (project: Project) => void visibleProjects: Project[] - setVisibleProjects: React.Dispatch> totalProjectsCount: number error: Error | null isLoading: ReturnType['isLoading'] @@ -94,6 +95,7 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { 'project-list-filter', 'all' ) + const prevSortRef = useRef(sort) const [selectedTagId, setSelectedTagId] = usePersistedState< string | undefined >('project-list-selected-tag-id', undefined) @@ -154,6 +156,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { } else { filteredProjects = _.filter(filteredProjects, filters[filter]) } + + if (prevSortRef.current !== sort) { + filteredProjects = sortProjects(filteredProjects, sort) + const loadedProjectsSorted = sortProjects(loadedProjects, sort) + setLoadedProjects(loadedProjectsSorted) + } + setVisibleProjects(filteredProjects) }, [ loadedProjects, @@ -163,8 +172,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { selectedTagId, setSelectedTagId, searchText, + sort, ]) + useEffect(() => { + prevSortRef.current = sort + }, [sort]) + const untaggedProjectsCount = useMemo(() => { const taggedProjectIds = _.uniq(_.flatten(tags.map(tag => tag.project_ids))) return loadedProjects.filter( @@ -227,12 +241,13 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { project.source = 'owner' project.trashed = false project.archived = false - const projects = [...loadedProjects] - projects.push(project) - setLoadedProjects(projects) - // to do: sort projects after loaded projects updated, otherwise, it's at bottom of list + loadedProjects.push(project) + const loadedProjectsSorted = sortProjects(loadedProjects, sort) + const visibleProjectsSorted = sortProjects(visibleProjects, sort) + setVisibleProjects(visibleProjectsSorted) + setLoadedProjects(loadedProjectsSorted) }, - [loadedProjects, setLoadedProjects] + [loadedProjects, visibleProjects, sort] ) const updateProjectViewData = useCallback( @@ -273,7 +288,6 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { selectTag, setSearchText, setSort, - setVisibleProjects, sort, tags, totalProjectsCount, @@ -296,7 +310,6 @@ export function ProjectListProvider({ children }: ProjectListProviderProps) { selectTag, setSearchText, setSort, - setVisibleProjects, sort, tags, totalProjectsCount, diff --git a/services/web/frontend/js/features/project-list/util/sort-comparators.ts b/services/web/frontend/js/features/project-list/util/sort-projects.ts similarity index 64% rename from services/web/frontend/js/features/project-list/util/sort-comparators.ts rename to services/web/frontend/js/features/project-list/util/sort-projects.ts index 250f4c624d..cf97504a3b 100644 --- a/services/web/frontend/js/features/project-list/util/sort-comparators.ts +++ b/services/web/frontend/js/features/project-list/util/sort-projects.ts @@ -1,7 +1,12 @@ +import { Project, Sort } from '../../../../../types/project/dashboard/api' +import { SortingOrder } from '../../../../../types/sorting-order' import { getOwnerName } from './project' -import { Project } from '../../../../../types/project/dashboard/api' import { Compare } from '../../../../../types/array/sort' +const order = (order: SortingOrder, projects: Project[]) => { + return order === 'asc' ? [...projects] : projects.reverse() +} + export const ownerNameComparator = (v1: Project, v2: Project) => { const ownerNameV1 = getOwnerName(v1) const ownerNameV2 = getOwnerName(v2) @@ -63,3 +68,26 @@ export const defaultComparator = ( return Compare.SORT_KEEP_ORDER } + +export default function sortProjects(projects: Project[], sort: Sort) { + let sorted = [...projects] + if (sort.by === 'title') { + sorted = sorted.sort((...args) => { + return defaultComparator(...args, 'name') + }) + } + + if (sort.by === 'lastUpdated') { + sorted = sorted.sort((...args) => { + return defaultComparator(...args, 'lastUpdated') + }) + } + + if (sort.by === 'owner') { + sorted = sorted.sort((...args) => { + return ownerNameComparator(...args) + }) + } + + return order(sort.order, sorted) +} diff --git a/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx b/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx index 7364e849ef..658a4df2b7 100644 --- a/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx +++ b/services/web/test/frontend/features/project-list/components/sidebar/tags-list.test.tsx @@ -1,14 +1,9 @@ -import { - render, - fireEvent, - screen, - waitFor, - within, -} from '@testing-library/react' +import { fireEvent, screen, waitFor, within } from '@testing-library/react' import { assert, expect } from 'chai' import fetchMock from 'fetch-mock' import TagsList from '../../../../../../frontend/js/features/project-list/components/sidebar/tags-list' -import { ProjectListProvider } from '../../../../../../frontend/js/features/project-list/context/project-list-context' +import { projectsData } from '../../fixtures/projects-data' +import { renderWithProjectListContext } from '../../helpers/render-with-context' describe('', function () { beforeEach(async function () { @@ -16,34 +11,15 @@ describe('', function () { { _id: 'abc123def456', name: 'Tag 1', - project_ids: ['456fea789bcd'], + project_ids: [projectsData[0].id], }, { _id: 'bcd234efg567', name: 'Another tag', - project_ids: ['456fea789bcd', '567efa890bcd'], + project_ids: [projectsData[0].id, projectsData[1].id], }, ]) - fetchMock.post('/api/project', { - projects: [ - { - id: '456fea789bcd', - archived: false, - trashed: false, - }, - { - id: '567efa890bcd', - archived: false, - trashed: false, - }, - { - id: '999fff999fff', - archived: false, - trashed: false, - }, - ], - }) fetchMock.post('/tag', { _id: 'eee888eee888', name: 'New Tag', @@ -52,11 +28,7 @@ describe('', function () { fetchMock.post('express:/tag/:tagId/rename', 200) fetchMock.delete('express:/tag/:tagId', 200) - render(, { - wrapper: ({ children }) => ( - {children} - ), - }) + renderWithProjectListContext() await waitFor(() => expect(fetchMock.called('/api/project'))) }) @@ -79,7 +51,7 @@ describe('', function () { name: 'Another tag (2)', }) screen.getByRole('button', { - name: 'Uncategorized (1)', + name: 'Uncategorized (3)', }) }) diff --git a/services/web/test/frontend/features/project-list/unit/sort-comparators.test.ts b/services/web/test/frontend/features/project-list/unit/sort-projects.test.ts similarity index 99% rename from services/web/test/frontend/features/project-list/unit/sort-comparators.test.ts rename to services/web/test/frontend/features/project-list/unit/sort-projects.test.ts index 794483b2ca..9e34481d91 100644 --- a/services/web/test/frontend/features/project-list/unit/sort-comparators.test.ts +++ b/services/web/test/frontend/features/project-list/unit/sort-projects.test.ts @@ -2,7 +2,7 @@ import { expect } from 'chai' import { ownerNameComparator, defaultComparator, -} from '../../../../../frontend/js/features/project-list/util/sort-comparators' +} from '../../../../../frontend/js/features/project-list/util/sort-projects' import { Project } from '../../../../../types/project/dashboard/api' const now = new Date()