-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add click-to-sort column headers to all entity list table views #6934
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
base: develop
Are you sure you want to change the base?
Changes from all commits
d68d042
f061d84
8f638cf
cb728f4
dd6a4a3
a19e937
302ecb3
f203559
bffc293
89287e8
10a8ef2
c71771a
278c185
4eab9d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,11 +2,14 @@ import React, { useMemo } from "react"; | |
| import { Table, Form } from "react-bootstrap"; | ||
| import { CheckBoxSelect } from "../Shared/Select"; | ||
| import cx from "classnames"; | ||
| import { SortDirectionEnum } from "src/core/generated-graphql"; | ||
| import { SortByIcon } from "./ListFilter"; | ||
|
|
||
| export interface IColumn { | ||
| label: string; | ||
| value: string; | ||
| mandatory?: boolean; | ||
| sortable?: boolean; | ||
| } | ||
|
|
||
| export const ColumnSelector: React.FC<{ | ||
|
|
@@ -47,6 +50,9 @@ interface IListTableProps<T> { | |
| selectedIds: Set<string>; | ||
| onSelectChange: (id: string, selected: boolean, shiftKey: boolean) => void; | ||
| renderCell: (column: IColumn, item: T, index: number) => React.ReactNode; | ||
| onSort?: (value: string) => void; | ||
| sortBy?: string; | ||
| sortDirection?: SortDirectionEnum; | ||
| } | ||
|
|
||
| export const ListTable = <T extends { id: string }>( | ||
|
|
@@ -61,6 +67,9 @@ export const ListTable = <T extends { id: string }>( | |
| selectedIds, | ||
| onSelectChange, | ||
| renderCell, | ||
| onSort, | ||
| sortBy, | ||
| sortDirection, | ||
| } = props; | ||
|
|
||
| const visibleColumns = useMemo(() => { | ||
|
|
@@ -102,12 +111,33 @@ export const ListTable = <T extends { id: string }>( | |
| }; | ||
|
|
||
| const columnHeaders = useMemo(() => { | ||
| return visibleColumns.map((column) => ( | ||
| <th key={column.value} className={`${column.value}-head`}> | ||
| {column.label} | ||
| </th> | ||
| )); | ||
| }, [visibleColumns]); | ||
| return visibleColumns.map((column) => { | ||
| const isSortable = column.sortable !== false && onSort; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do sortable columns outnumber non-sortable columns? If not, then perhaps we should consider defaulting to
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Answering my own question, it appears that there's more sortable than not. I don't love defaulting to false with a name like |
||
| const isActive = sortBy === column.value; | ||
|
|
||
| return ( | ||
| <th | ||
| key={column.value} | ||
| className={`${column.value}-head${isSortable ? " sortable" : ""}`} | ||
| onClick={isSortable ? () => onSort(column.value) : undefined} | ||
| style={isSortable ? { cursor: "pointer" } : undefined} | ||
| > | ||
| <div | ||
| style={{ | ||
| display: "flex", | ||
| alignItems: "center", | ||
| justifyContent: "space-between", | ||
| }} | ||
| > | ||
| {column.label} | ||
| {isActive && sortDirection && ( | ||
| <SortByIcon sortDirection={sortDirection} /> | ||
| )} | ||
| </div> | ||
| </th> | ||
| ); | ||
| }); | ||
| }, [visibleColumns, onSort, sortBy, sortDirection]); | ||
|
|
||
| return ( | ||
| <div className={cx("table-list", className)}> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't drop favorite sort option and birthdate case because they are needed for Favourite/Age column clicks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You didn't include these changes in the original description.