Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(list): improve select performance for big lists #378

Merged
merged 3 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/components/sectionList/SectionListRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@ export type SectionListRowProps<Model extends BaseListModel> = {
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<Model extends BaseListModel>({
export const SectionListRow = React.memo(function SectionListRow<
Model extends BaseListModel
>({
active,
selectedColumns,
modelData,
Expand Down Expand Up @@ -56,10 +61,14 @@ export function SectionListRow<Model extends BaseListModel>({
key={selectedColumn.path}
onClick={() => onClick?.(modelData)}
>
{renderColumnValue(selectedColumn)}
{renderColumnValue(selectedColumn, modelData)}
</DataTableCell>
))}
<DataTableCell>{renderActions(modelData.id)}</DataTableCell>
<DataTableCell>{renderActions(modelData)}</DataTableCell>
</DataTableRow>
)
}
})

export const MemoedSectionListRow = React.memo(
SectionListRow
) as typeof SectionListRow
122 changes: 64 additions & 58 deletions src/components/sectionList/SectionListWrapper.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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<BaseListModel> | undefined
Expand All @@ -31,40 +33,11 @@ export const SectionListWrapper = ({
}: SectionListWrapperProps) => {
const { columns: headerColumns } = useModelListView()
const schema = useSchemaFromHandle()
const [selectedModels, setSelectedModels] = useState<Set<string>>(new Set())
const { selectedModels, checkAllSelected, add, remove, toggle, clearAll } =
useSelectedModels()
const [detailsId, setDetailsId] = useState<string | undefined>()
const [sharingDialogId, setSharingDialogId] = useState<string | undefined>()

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)
Expand All @@ -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 (
<ModelValue path={path} schema={schema} sectionModel={model} />
)
},
[schema]
)

const renderActions = useCallback(
(model: BaseListModel) => (
<DefaultListActions
model={model}
onShowDetailsClick={handleDetailsClick}
onOpenSharingClick={setSharingDialogId}
/>
),
[handleDetailsClick, setSharingDialogId]
)

const isAllSelected = data ? checkAllSelected(data) : false

return (
<div>
<SectionListTitle />
Expand All @@ -87,49 +112,30 @@ export const SectionListWrapper = ({
{selectedModels.size > 0 ? (
<SectionListHeaderBulk
selectedModels={selectedModels}
onDeselectAll={() => setSelectedModels(new Set())}
onDeselectAll={clearAll}
/>
) : (
<SectionListHeader />
)}
<SectionList
headerColumns={headerColumns}
onSelectAll={handleSelectAll}
allSelected={allSelected}
allSelected={isAllSelected}
>
<SectionListMessage />
{data?.map((model) => (
<SectionListRow
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 (
<ModelValue
path={path}
schema={schema}
sectionModel={model}
/>
)
}}
renderActions={() => (
<DefaultListActions
model={model}
onShowDetailsClick={handleShowDetails}
onOpenSharingClick={setSharingDialogId}
/>
)}
renderColumnValue={renderColumnValue}
renderActions={renderActions}
/>
))}

<SectionListPagination pager={pager} />
</SectionList>
{detailsId && (
Expand Down
4 changes: 2 additions & 2 deletions src/components/sectionList/listActions/DefaultListActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type ModelWithAccess = Pick<BaseIdentifiableObject, 'id' | 'access'>

type DefaultListActionProps = {
model: ModelWithAccess
onShowDetailsClick: (id: string) => void
onShowDetailsClick: (model: ModelWithAccess) => void
onOpenSharingClick: (id: string) => void
}

Expand All @@ -23,7 +23,7 @@ export const DefaultListActions = ({
<ActionMore
modelId={model.id}
editAccess={editAccess}
onShowDetailsClick={() => onShowDetailsClick(model.id)}
onShowDetailsClick={() => onShowDetailsClick(model)}
onOpenSharingClick={() => onOpenSharingClick(model.id)}
/>
</ListActions>
Expand Down
74 changes: 74 additions & 0 deletions src/components/sectionList/useSelectedModels.ts
Original file line number Diff line number Diff line change
@@ -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<Set<string>>(
() => 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,
}
}
4 changes: 2 additions & 2 deletions src/lib/models/access.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { Access, BaseIdentifiableObject } from '../../types/generated'

export const editAccess = (access: Access) => access.write
export const hasWriteAccess = (access: Partial<Access>) => !!access.write

export const canEditModel = (model: Pick<BaseIdentifiableObject, 'access'>) =>
editAccess(model.access)
hasWriteAccess(model.access)

export type ParsedAccessPart = {
read: boolean
Expand Down
21 changes: 16 additions & 5 deletions src/pages/dataElements/List.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/types/generated/models.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading