From 0a98f3c88430d2a59a64c1575f16b4bfa01d8c76 Mon Sep 17 00:00:00 2001 From: Birk Johansson Date: Wed, 13 Mar 2024 21:59:30 +0100 Subject: [PATCH] fix(list): improve select performance for big lists (#378) * fix(list): improve select performance for big lists * fix: fix test * refactor: move data out of useSelectedModels --- src/components/sectionList/SectionListRow.tsx | 21 ++- .../sectionList/SectionListWrapper.tsx | 122 +++++++++--------- .../listActions/DefaultListActions.tsx | 4 +- .../sectionList/useSelectedModels.ts | 74 +++++++++++ src/lib/models/access.ts | 4 +- src/pages/dataElements/List.spec.tsx | 21 ++- src/types/generated/models.ts | 2 +- 7 files changed, 174 insertions(+), 74 deletions(-) create mode 100644 src/components/sectionList/useSelectedModels.ts diff --git a/src/components/sectionList/SectionListRow.tsx b/src/components/sectionList/SectionListRow.tsx index 2febc669..4867dbd5 100644 --- a/src/components/sectionList/SectionListRow.tsx +++ b/src/components/sectionList/SectionListRow.tsx @@ -13,13 +13,18 @@ export type SectionListRowProps = { selectedColumns: SelectedColumns onSelect: (modelId: string, checked: boolean) => void selected: boolean - renderActions: (modelId: string) => React.ReactNode - renderColumnValue: (column: SelectedColumn) => React.ReactNode + renderActions: (model: Model) => React.ReactNode + renderColumnValue: ( + column: SelectedColumn, + modelData: Model + ) => React.ReactNode onClick?: (modelData: Model) => void active?: boolean } -export function SectionListRow({ +export const SectionListRow = React.memo(function SectionListRow< + Model extends BaseListModel +>({ active, selectedColumns, modelData, @@ -56,10 +61,14 @@ export function SectionListRow({ key={selectedColumn.path} onClick={() => onClick?.(modelData)} > - {renderColumnValue(selectedColumn)} + {renderColumnValue(selectedColumn, modelData)} ))} - {renderActions(modelData.id)} + {renderActions(modelData)} ) -} +}) + +export const MemoedSectionListRow = React.memo( + SectionListRow +) as typeof SectionListRow diff --git a/src/components/sectionList/SectionListWrapper.tsx b/src/components/sectionList/SectionListWrapper.tsx index a0b49124..56101986 100644 --- a/src/components/sectionList/SectionListWrapper.tsx +++ b/src/components/sectionList/SectionListWrapper.tsx @@ -1,7 +1,7 @@ import { FetchError } from '@dhis2/app-runtime' import { SharingDialog } from '@dhis2/ui' -import React, { useMemo, useState } from 'react' -import { BaseListModel, useSchemaFromHandle } from '../../lib' +import React, { useCallback, useState } from 'react' +import { BaseListModel, canEditModel, useSchemaFromHandle } from '../../lib' import { Pager, ModelCollection } from '../../types/models' import { SectionListHeaderBulk } from './bulk' import { DetailsPanel, DefaultDetailsPanelContent } from './detailsPanel' @@ -17,6 +17,8 @@ import { SectionListEmpty, SectionListError } from './SectionListMessages' import { SectionListPagination } from './SectionListPagination' import { SectionListRow } from './SectionListRow' import { SectionListTitle } from './SectionListTitle' +import { SelectedColumn } from './types' +import { useSelectedModels } from './useSelectedModels' type SectionListWrapperProps = { data: ModelCollection | undefined @@ -31,40 +33,11 @@ export const SectionListWrapper = ({ }: SectionListWrapperProps) => { const { columns: headerColumns } = useModelListView() const schema = useSchemaFromHandle() - const [selectedModels, setSelectedModels] = useState>(new Set()) + const { selectedModels, checkAllSelected, add, remove, toggle, clearAll } = + useSelectedModels() const [detailsId, setDetailsId] = useState() const [sharingDialogId, setSharingDialogId] = useState() - const handleSelect = (id: string, checked: boolean) => { - if (checked) { - setSelectedModels(new Set(selectedModels).add(id)) - } else { - selectedModels.delete(id) - setSelectedModels(new Set(selectedModels)) - } - } - - const handleSelectAll = (checked: boolean) => { - if (checked) { - setSelectedModels( - new Set( - data?.map((model) => { - return model.id - }) - ) - ) - } else { - setSelectedModels(new Set()) - } - } - - const handleShowDetails = (id: string) => - setDetailsId((prevDetailsId) => (prevDetailsId === id ? undefined : id)) - - const allSelected = useMemo(() => { - return data?.length !== 0 && data?.length === selectedModels.size - }, [data, selectedModels.size]) - const SectionListMessage = () => { if (error) { console.log(error.details || error) @@ -79,6 +52,58 @@ export const SectionListWrapper = ({ return null } + const handleSelectAll = useCallback( + (checked: boolean) => { + if (!data) { + return + } + if (checked) { + const editableIds = data + .filter((model) => canEditModel(model)) + .map((model) => model.id) + add(editableIds) + } else { + remove( + data.filter((model) => model.id).map((model) => model.id) + ) + } + }, + [data, add, remove] + ) + + const handleDetailsClick = useCallback( + ({ id }: BaseListModel) => { + setDetailsId((prevDetailsId) => + prevDetailsId === id ? undefined : id + ) + }, + [setDetailsId] + ) + + /* Note that SectionListRow is memoed, to prevent re-rendering + every item when interacting with a row */ + const renderColumnValue = useCallback( + ({ path }: SelectedColumn, model: BaseListModel) => { + return ( + + ) + }, + [schema] + ) + + const renderActions = useCallback( + (model: BaseListModel) => ( + + ), + [handleDetailsClick, setSharingDialogId] + ) + + const isAllSelected = data ? checkAllSelected(data) : false + return (
@@ -87,7 +112,7 @@ export const SectionListWrapper = ({ {selectedModels.size > 0 ? ( setSelectedModels(new Set())} + onDeselectAll={clearAll} /> ) : ( @@ -95,7 +120,7 @@ export const SectionListWrapper = ({ {data?.map((model) => ( @@ -103,33 +128,14 @@ export const SectionListWrapper = ({ key={model.id} modelData={model} selectedColumns={headerColumns} - onSelect={handleSelect} - onClick={({ id }) => { - setDetailsId((prevDetailsId) => - prevDetailsId === id ? undefined : id - ) - }} + onSelect={toggle} + onClick={handleDetailsClick} selected={selectedModels.has(model.id)} active={model.id === detailsId} - renderColumnValue={({ path }) => { - return ( - - ) - }} - renderActions={() => ( - - )} + renderColumnValue={renderColumnValue} + renderActions={renderActions} /> ))} - {detailsId && ( diff --git a/src/components/sectionList/listActions/DefaultListActions.tsx b/src/components/sectionList/listActions/DefaultListActions.tsx index 60d2715e..a78054f9 100644 --- a/src/components/sectionList/listActions/DefaultListActions.tsx +++ b/src/components/sectionList/listActions/DefaultListActions.tsx @@ -7,7 +7,7 @@ type ModelWithAccess = Pick type DefaultListActionProps = { model: ModelWithAccess - onShowDetailsClick: (id: string) => void + onShowDetailsClick: (model: ModelWithAccess) => void onOpenSharingClick: (id: string) => void } @@ -23,7 +23,7 @@ export const DefaultListActions = ({ onShowDetailsClick(model.id)} + onShowDetailsClick={() => onShowDetailsClick(model)} onOpenSharingClick={() => onOpenSharingClick(model.id)} /> diff --git a/src/components/sectionList/useSelectedModels.ts b/src/components/sectionList/useSelectedModels.ts new file mode 100644 index 00000000..8fbe8269 --- /dev/null +++ b/src/components/sectionList/useSelectedModels.ts @@ -0,0 +1,74 @@ +// export interface useSelecto +import { memoize } from 'lodash' +import { useState, useMemo, useCallback } from 'react' +import { BaseListModel, canEditModel } from '../../lib' + +export type UseSelectedModelsOptions = { intialSelected?: string[] } + +export function useSelectedModels(options?: UseSelectedModelsOptions) { + const [selectedModels, setSelectedModels] = useState>( + () => new Set(options?.intialSelected || []) + ) + + const add = useCallback( + (ids: string | string[]) => { + const addIds = Array.isArray(ids) ? ids : [ids] + setSelectedModels((prevSelected) => { + const newSelected = new Set(prevSelected) + addIds.forEach((id) => newSelected.add(id)) + return newSelected + }) + }, + [setSelectedModels] + ) + + const remove = useCallback( + (ids: string | string[]) => { + const removeIds = Array.isArray(ids) ? ids : [ids] + + setSelectedModels((prevSelected) => { + const newSelected = new Set(prevSelected) + removeIds.forEach((id) => newSelected.delete(id)) + return newSelected + }) + }, + [setSelectedModels] + ) + const toggle = useCallback( + (ids: string | string[], checked: boolean) => { + if (checked) { + add(ids) + } else { + remove(ids) + } + }, + [add, remove] + ) + + const clearAll = useCallback( + () => setSelectedModels(new Set()), + [setSelectedModels] + ) + + const checkAllSelected = useMemo( + () => + memoize((data: BaseListModel[]) => { + if (!data) { + return false + } + return data + .filter((m) => canEditModel(m)) + .every((model) => selectedModels.has(model.id)) + }), + [selectedModels] + ) + + return { + selectedModels, + checkAllSelected, + add, + remove, + toggle, + clearAll, + } +} diff --git a/src/lib/models/access.ts b/src/lib/models/access.ts index 85e122e0..b3d11d5b 100644 --- a/src/lib/models/access.ts +++ b/src/lib/models/access.ts @@ -1,9 +1,9 @@ import type { Access, BaseIdentifiableObject } from '../../types/generated' -export const editAccess = (access: Access) => access.write +export const hasWriteAccess = (access: Partial) => !!access.write export const canEditModel = (model: Pick) => - editAccess(model.access) + hasWriteAccess(model.access) export type ParsedAccessPart = { read: boolean diff --git a/src/pages/dataElements/List.spec.tsx b/src/pages/dataElements/List.spec.tsx index 126ef634..2d6f35ef 100644 --- a/src/pages/dataElements/List.spec.tsx +++ b/src/pages/dataElements/List.spec.tsx @@ -8,7 +8,7 @@ import { import userEvent from '@testing-library/user-event' import React from 'react' import dataElementSchemaMock from '../../__mocks__/schema/dataElementsSchema.json' -import { SECTIONS_MAP } from '../../lib' +import { SECTIONS_MAP, canEditModel } from '../../lib' import { useSchemaStore } from '../../lib/schemas/schemaStore' import { ModelSchemas } from '../../lib/useLoadApp' import TestComponentWithRouter, { @@ -291,13 +291,24 @@ describe('Data Elements List', () => { const allCheckBoxes = queryAllByTestId('section-list-row-checkbox') + // should only select checkboxes that are editable + const numberOfEditableItems = dataElementsMock.dataElements.reduce( + (acc, de) => (de && canEditModel(de) ? acc + 1 : acc), + 0 + ) expect(allCheckBoxes.length).toEqual(dataElementsMock.pager.pageSize) - // the UI library doesn't seem to apply checked on the input, which would have been a better test, so checking "checked" class on the svg icon + let numberOfSelectedBoxes = 0 allCheckBoxes.forEach((checkbox) => { - expect(checkbox.getElementsByTagName('svg')[0]).toHaveClass( - 'checked' - ) + // the UI library doesn't seem to apply checked on the input, which would have been a better test, so checking "checked" class on the svg icon + if ( + checkbox + .getElementsByTagName('svg')[0] + .classList.contains('checked') + ) { + numberOfSelectedBoxes++ + } }) + expect(numberOfSelectedBoxes).toEqual(numberOfEditableItems) }) // empty list diff --git a/src/types/generated/models.ts b/src/types/generated/models.ts index 10496b6d..ed712852 100644 --- a/src/types/generated/models.ts +++ b/src/types/generated/models.ts @@ -1,7 +1,7 @@ /* GENERATED BY https://github.com/Birkbjo/dhis2-open-api-ts */ export type Access = { - data: AccessData + data?: AccessData delete: boolean externalize: boolean manage: boolean