Skip to content

Commit

Permalink
fix(list): improve select performance for big lists (#378)
Browse files Browse the repository at this point in the history
* fix(list): improve select performance for big lists

* fix: fix test

* refactor: move data out of useSelectedModels
  • Loading branch information
Birkbjo authored Mar 13, 2024
1 parent 90b02a4 commit 0a98f3c
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 74 deletions.
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

0 comments on commit 0a98f3c

Please sign in to comment.