From 344a054ba2eee76246d2b73698c63ad03a011408 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Thu, 12 Jan 2023 23:06:58 +0100 Subject: [PATCH] [SM-74] TableDataSource for sorting (#4079) * Initial draft of a table data source * Improve table data source * Migrate projects table for demo * Update existing tables * Fix access selector * remove sortDirection from custom fn * a11y improvements * update icons; make button full width * update storybook docs * apply code review changes * fix: add table body to projects list * Fix error on create secret. Fix project list setting projects on getter. Copy table data on set. Fix documentation * Change signature to protected, rename method to not start with underscore * add hover and focus effects Co-authored-by: William Martin --- .../access-selector.component.html | 4 +- .../manage/events.component.html | 4 +- .../dialogs/bulk-status-dialog.component.html | 4 +- .../projects-list.component.html | 12 +- .../projects-list/projects-list.component.ts | 5 + .../dialog/secret-dialog.component.html | 4 +- .../access/access-list.component.html | 4 +- .../service-account-dialog.component.html | 4 +- .../service-accounts-list.component.html | 4 +- .../shared/secrets-list.component.html | 4 +- .../src/stories/button-docs.stories.mdx | 2 +- .../src/stories/table-docs.stories.mdx | 104 +++++++++++ libs/components/src/table/index.ts | 1 + .../src/table/sortable.component.ts | 125 +++++++++++++ .../components/src/table/table-data-source.ts | 164 ++++++++++++++++++ .../components/src/table/table.component.html | 4 +- libs/components/src/table/table.component.ts | 47 ++++- libs/components/src/table/table.module.ts | 13 +- libs/components/src/table/table.stories.ts | 81 ++++++++- 19 files changed, 557 insertions(+), 33 deletions(-) create mode 100644 libs/components/src/stories/table-docs.stories.mdx create mode 100644 libs/components/src/table/sortable.component.ts create mode 100644 libs/components/src/table/table-data-source.ts diff --git a/apps/web/src/app/organizations/components/access-selector/access-selector.component.html b/apps/web/src/app/organizations/components/access-selector/access-selector.component.html index cc4d1d7fa78..6e5a299b57e 100644 --- a/apps/web/src/app/organizations/components/access-selector/access-selector.component.html +++ b/apps/web/src/app/organizations/components/access-selector/access-selector.component.html @@ -40,7 +40,7 @@ - + {{ emptySelectionText }} - + diff --git a/apps/web/src/app/organizations/manage/events.component.html b/apps/web/src/app/organizations/manage/events.component.html index e2c03f0dde5..f936cc7a863 100644 --- a/apps/web/src/app/organizations/manage/events.component.html +++ b/apps/web/src/app/organizations/manage/events.component.html @@ -75,7 +75,7 @@ {{ "event" | i18n }} - + {{ e.date | date: "medium" }} @@ -86,7 +86,7 @@ - + - + @@ -25,8 +25,8 @@ {{ "all" | i18n }} - {{ "name" | i18n }} - {{ "lastEdited" | i18n }} + {{ "name" | i18n }} + {{ "lastEdited" | i18n }} - + diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/access/access-list.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/access/access-list.component.html index dd53ccd08b4..3cceeb1d0e6 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/access/access-list.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/access/access-list.component.html @@ -39,7 +39,7 @@ - + - + diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.html index 9c41d839990..fa329edbe32 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.html @@ -29,13 +29,13 @@ {{ "permissions" | i18n }} - + example example - +
diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts-list.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts-list.component.html index 7a12d8bc517..5212e9d205e 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts-list.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-accounts-list.component.html @@ -39,7 +39,7 @@ - + - + diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html index 46a05021456..aed896b3ff9 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html @@ -39,7 +39,7 @@ - + - + diff --git a/libs/components/src/stories/button-docs.stories.mdx b/libs/components/src/stories/button-docs.stories.mdx index 0c40e780775..cdcdb87f9db 100644 --- a/libs/components/src/stories/button-docs.stories.mdx +++ b/libs/components/src/stories/button-docs.stories.mdx @@ -1,4 +1,4 @@ -import { Meta, Story, Source } from "@storybook/addon-docs"; +import { Meta, Story } from "@storybook/addon-docs"; diff --git a/libs/components/src/stories/table-docs.stories.mdx b/libs/components/src/stories/table-docs.stories.mdx new file mode 100644 index 00000000000..139574a18fe --- /dev/null +++ b/libs/components/src/stories/table-docs.stories.mdx @@ -0,0 +1,104 @@ +import { Meta, Story, Source } from "@storybook/addon-docs"; + + + +# Table + +## Overview + +All tables should have a visible horizontal header and label for each column. + + + +The below code is the absolute minimum required to create a table. However we stronly advice you to +use the `dataSource` input to provide a data source for your table. This allows you to easily sort +data. + +```html + + + + Header 1 + Header 2 + Header 3 + + + + + Cell 1 + Cell 2 + Cell 3 + + + +``` + +## Data Source + +Bitwarden provides a data source for tables that can be used in place of a traditional data array. +The `TableDataSource` implements sorting and will in the future also support filtering. This allows +the `bitTable` component to focus on rendering while offloading the data management to the data +source. + +```ts +// External data source +const data: T[]; + +const dataSource = new TableDataSource(); +dataSource.data = data; +``` + +We use the `dataSource` as an input to the `bit-table` component, and access the rows to render +within the `ng-template`which provides access to the rows using `let-rows$`. + + + +### Sortable + +We provide a simple component for displaying sortable column headers. The `bitSortable` component +wires up to the `TableDataSource` and will automatically sort the data when clicked and display +an indicator for which column is currently sorted. The dafault sorting can be specified by setting +the `default`. + +```html +Id +Name +``` + +It's also possible to define a custom sorting function by setting the `fn` input. + +```ts +const sortFn = (a: T, b: T) => (a.id > b.id ? 1 : -1); +``` + +### Virtual Scrolling + +It's heavily adviced to use virtual scrolling if you expect the table to have any significant amount +of data. This is easily done by wrapping the table in the `cdk-virtual-scroll-viewport` component, +specify a `itemSize`, set `scrollWindow` to `true` and replace `*ngFor` with `*cdkVirtualFor`. + +```html + + + + + Id + Name + Other + + + + + {{ r.id }} + {{ r.name }} + {{ r.other }} + + + + +``` + +## Accessibility + +- Always incude a row or column header with your table; this allows screen readers to better contextualize the data +- Avoid spanning data across cells diff --git a/libs/components/src/table/index.ts b/libs/components/src/table/index.ts index 02fed986ac7..1981f1a6613 100644 --- a/libs/components/src/table/index.ts +++ b/libs/components/src/table/index.ts @@ -1 +1,2 @@ export * from "./table.module"; +export * from "./table-data-source"; diff --git a/libs/components/src/table/sortable.component.ts b/libs/components/src/table/sortable.component.ts new file mode 100644 index 00000000000..9f53f956da3 --- /dev/null +++ b/libs/components/src/table/sortable.component.ts @@ -0,0 +1,125 @@ +import { coerceBooleanProperty } from "@angular/cdk/coercion"; +import { Component, HostBinding, Input, OnInit } from "@angular/core"; + +import type { SortFn } from "./table-data-source"; +import { TableComponent } from "./table.component"; + +@Component({ + selector: "th[bitSortable]", + template: ` + + `, +}) +export class SortableComponent implements OnInit { + /** + * Mark the column as sortable and specify the key to sort by + */ + @Input() bitSortable: string; + + private _default: boolean; + /** + * Mark the column as the default sort column + */ + @Input() set default(value: boolean | "") { + this._default = coerceBooleanProperty(value); + } + + /** + * Custom sorting function + * + * @example + * fn = (a, b) => a.name.localeCompare(b.name) + */ + @Input() fn: SortFn; + + constructor(private table: TableComponent) {} + + ngOnInit(): void { + if (this._default && !this.isActive) { + this.setActive(); + } + } + + @HostBinding("attr.aria-sort") get ariaSort() { + if (!this.isActive) { + return undefined; + } + return this.sort.direction === "asc" ? "ascending" : "descending"; + } + + protected setActive() { + if (this.table.dataSource) { + const direction = this.isActive && this.direction === "asc" ? "desc" : "asc"; + this.table.dataSource.sort = { column: this.bitSortable, direction: direction, fn: this.fn }; + } + } + + private get sort() { + return this.table.dataSource?.sort; + } + + get isActive() { + return this.sort?.column === this.bitSortable; + } + + get direction() { + return this.sort?.direction; + } + + get icon() { + if (!this.isActive) { + return "bwi-chevron-up tw-opacity-0 group-hover:tw-opacity-100 group-focus-visible:tw-opacity-100"; + } + return this.direction === "asc" ? "bwi-chevron-up" : "bwi-angle-down"; + } + + get classList() { + return [ + // Offset to border and padding + "-tw-m-1.5", + + // Below is copied from BitIconButtonComponent + "tw-font-semibold", + "tw-border", + "tw-border-solid", + "tw-rounded", + "tw-transition", + "hover:tw-no-underline", + "focus:tw-outline-none", + + "tw-bg-transparent", + "!tw-text-muted", + "tw-border-transparent", + "hover:tw-bg-transparent-hover", + "hover:tw-border-primary-700", + "focus-visible:before:tw-ring-primary-700", + "disabled:tw-opacity-60", + "disabled:hover:tw-border-transparent", + "disabled:hover:tw-bg-transparent", + + // Workaround for box-shadow with transparent offset issue: + // https://github.com/tailwindlabs/tailwindcss/issues/3595 + // Remove `before:` and use regular `tw-ring` when browser no longer has bug, or better: + // switch to `outline` with `outline-offset` when Safari supports border radius on outline. + // Using `box-shadow` to create outlines is a hack and as such `outline` should be preferred. + "tw-relative", + "before:tw-content-['']", + "before:tw-block", + "before:tw-absolute", + "before:-tw-inset-[3px]", + "before:tw-rounded-md", + "before:tw-transition", + "before:tw-ring", + "before:tw-ring-transparent", + "focus-visible:tw-z-10", + ]; + } +} diff --git a/libs/components/src/table/table-data-source.ts b/libs/components/src/table/table-data-source.ts new file mode 100644 index 00000000000..d5f4473a8f7 --- /dev/null +++ b/libs/components/src/table/table-data-source.ts @@ -0,0 +1,164 @@ +import { _isNumberValue } from "@angular/cdk/coercion"; +import { DataSource } from "@angular/cdk/collections"; +import { BehaviorSubject, combineLatest, map, Observable, Subscription } from "rxjs"; + +export type SortDirection = "asc" | "desc"; +export type SortFn = (a: any, b: any) => number; +export type Sort = { + column?: string; + direction: SortDirection; + fn?: SortFn; +}; + +// Loosely based on CDK TableDataSource +// https://github.com/angular/components/blob/main/src/material/table/table-data-source.ts +export class TableDataSource extends DataSource { + private readonly _data: BehaviorSubject; + private readonly _sort: BehaviorSubject; + private readonly _renderData = new BehaviorSubject([]); + + private _renderChangesSubscription: Subscription | null = null; + + constructor() { + super(); + this._data = new BehaviorSubject([]); + this._sort = new BehaviorSubject({ direction: "asc" }); + } + + get data() { + return this._data.value; + } + + set data(data: T[]) { + this._data.next(data ? [...data] : []); + } + + set sort(sort: Sort) { + this._sort.next(sort); + } + + get sort() { + return this._sort.value; + } + + connect(): Observable { + if (!this._renderChangesSubscription) { + this.updateChangeSubscription(); + } + + return this._renderData; + } + + disconnect(): void { + this._renderChangesSubscription?.unsubscribe(); + this._renderChangesSubscription = null; + } + + private updateChangeSubscription() { + const orderedData = combineLatest([this._data, this._sort]).pipe( + map(([data]) => this.orderData(data)) + ); + + this._renderChangesSubscription?.unsubscribe(); + this._renderChangesSubscription = orderedData.subscribe((data) => this._renderData.next(data)); + } + + private orderData(data: T[]): T[] { + if (!this.sort) { + return data; + } + + return this.sortData(data, this.sort); + } + + /** + * Copied from https://github.com/angular/components/blob/main/src/material/table/table-data-source.ts + * License: MIT + * Copyright (c) 2022 Google LLC. + * + * Data accessor function that is used for accessing data properties for sorting through + * the default sortData function. + * This default function assumes that the sort header IDs (which defaults to the column name) + * matches the data's properties (e.g. column Xyz represents data['Xyz']). + * May be set to a custom function for different behavior. + * @param data Data object that is being accessed. + * @param sortHeaderId The name of the column that represents the data. + */ + protected sortingDataAccessor(data: T, sortHeaderId: string): string | number { + const value = (data as unknown as Record)[sortHeaderId]; + + if (_isNumberValue(value)) { + const numberValue = Number(value); + + return numberValue < Number.MAX_SAFE_INTEGER ? numberValue : value; + } + + return value; + } + + /** + * Copied from https://github.com/angular/components/blob/main/src/material/table/table-data-source.ts + * License: MIT + * Copyright (c) 2022 Google LLC. + * + * Gets a sorted copy of the data array based on the state of the MatSort. Called + * after changes are made to the filtered data or when sort changes are emitted from MatSort. + * By default, the function retrieves the active sort and its direction and compares data + * by retrieving data using the sortingDataAccessor. May be overridden for a custom implementation + * of data ordering. + * @param data The array of data that should be sorted. + * @param sort The connected MatSort that holds the current sort state. + */ + protected sortData(data: T[], sort: Sort): T[] { + const column = sort.column; + const direction = sort.direction; + if (!column) { + return data; + } + + return data.sort((a, b) => { + // If a custom sort function is provided, use it instead of the default. + if (sort.fn) { + return sort.fn(a, b) * (direction === "asc" ? 1 : -1); + } + + let valueA = this.sortingDataAccessor(a, column); + let valueB = this.sortingDataAccessor(b, column); + + // If there are data in the column that can be converted to a number, + // it must be ensured that the rest of the data + // is of the same type so as not to order incorrectly. + const valueAType = typeof valueA; + const valueBType = typeof valueB; + + if (valueAType !== valueBType) { + if (valueAType === "number") { + valueA += ""; + } + if (valueBType === "number") { + valueB += ""; + } + } + + // If both valueA and valueB exist (truthy), then compare the two. Otherwise, check if + // one value exists while the other doesn't. In this case, existing value should come last. + // This avoids inconsistent results when comparing values to undefined/null. + // If neither value exists, return 0 (equal). + let comparatorResult = 0; + if (valueA != null && valueB != null) { + // Check if one value is greater than the other; if equal, comparatorResult should remain 0. + if (valueA > valueB) { + comparatorResult = 1; + } else if (valueA < valueB) { + comparatorResult = -1; + } + } else if (valueA != null) { + comparatorResult = 1; + } else if (valueB != null) { + comparatorResult = -1; + } + + return comparatorResult * (direction === "asc" ? 1 : -1); + }); + } +} diff --git a/libs/components/src/table/table.component.html b/libs/components/src/table/table.component.html index 11108d8ec1a..57e3853f976 100644 --- a/libs/components/src/table/table.component.html +++ b/libs/components/src/table/table.component.html @@ -5,6 +5,8 @@ - + diff --git a/libs/components/src/table/table.component.ts b/libs/components/src/table/table.component.ts index 8099c35b929..40a2da70927 100644 --- a/libs/components/src/table/table.component.ts +++ b/libs/components/src/table/table.component.ts @@ -1,7 +1,50 @@ -import { Component } from "@angular/core"; +import { isDataSource } from "@angular/cdk/collections"; +import { + AfterContentChecked, + Component, + ContentChild, + Directive, + Input, + OnDestroy, + TemplateRef, +} from "@angular/core"; +import { Observable } from "rxjs"; + +import { TableDataSource } from "./table-data-source"; + +@Directive({ + selector: "ng-template[body]", +}) +export class TableBodyDirective { + // eslint-disable-next-line @typescript-eslint/explicit-member-accessibility + constructor(public readonly template: TemplateRef) {} +} @Component({ selector: "bit-table", templateUrl: "./table.component.html", }) -export class TableComponent {} +export class TableComponent implements OnDestroy, AfterContentChecked { + @Input() dataSource: TableDataSource; + + @ContentChild(TableBodyDirective) templateVariable: TableBodyDirective; + + protected rows: Observable; + + private _initialized = false; + + ngAfterContentChecked(): void { + if (!this._initialized && isDataSource(this.dataSource)) { + this._initialized = true; + + const dataStream = this.dataSource.connect(); + this.rows = dataStream; + } + } + + ngOnDestroy(): void { + if (isDataSource(this.dataSource)) { + this.dataSource.disconnect(); + } + } +} diff --git a/libs/components/src/table/table.module.ts b/libs/components/src/table/table.module.ts index 2ca88a349f0..753e4362e6f 100644 --- a/libs/components/src/table/table.module.ts +++ b/libs/components/src/table/table.module.ts @@ -3,11 +3,18 @@ import { NgModule } from "@angular/core"; import { CellDirective } from "./cell.directive"; import { RowDirective } from "./row.directive"; -import { TableComponent } from "./table.component"; +import { SortableComponent } from "./sortable.component"; +import { TableBodyDirective, TableComponent } from "./table.component"; @NgModule({ imports: [CommonModule], - declarations: [TableComponent, CellDirective, RowDirective], - exports: [TableComponent, CellDirective, RowDirective], + declarations: [ + TableComponent, + CellDirective, + RowDirective, + SortableComponent, + TableBodyDirective, + ], + exports: [TableComponent, CellDirective, RowDirective, SortableComponent, TableBodyDirective], }) export class TableModule {} diff --git a/libs/components/src/table/table.stories.ts b/libs/components/src/table/table.stories.ts index 961636dd9cc..60e80117bef 100644 --- a/libs/components/src/table/table.stories.ts +++ b/libs/components/src/table/table.stories.ts @@ -1,12 +1,14 @@ +import { ScrollingModule } from "@angular/cdk/scrolling"; import { Meta, moduleMetadata, Story } from "@storybook/angular"; +import { TableDataSource } from "./table-data-source"; import { TableModule } from "./table.module"; export default { title: "Component Library/Table", decorators: [ moduleMetadata({ - imports: [TableModule], + imports: [TableModule, ScrollingModule], }), ], argTypes: { @@ -34,7 +36,7 @@ const Template: Story = (args) => ({ Header 3 - + Cell 1 Cell 2
Multiline Cell @@ -50,9 +52,8 @@ const Template: Story = (args) => ({ Cell 8 Cell 9 - + - `, }); @@ -60,3 +61,75 @@ export const Default = Template.bind({}); Default.args = { alignRowContent: "baseline", }; + +const data = new TableDataSource<{ id: number; name: string; other: string }>(); + +data.data = [...Array(5).keys()].map((i) => ({ + id: i, + name: `name-${i}`, + other: `other-${i}`, +})); + +const DataSourceTemplate: Story = (args) => ({ + props: { + dataSource: data, + sortFn: (a: any, b: any) => a.id - b.id, + }, + template: ` + + + + Id + Name + Other + + + + + {{ r.id }} + {{ r.name }} + {{ r.other }} + + + + `, +}); + +export const DataSource = DataSourceTemplate.bind({}); + +const data2 = new TableDataSource<{ id: number; name: string; other: string }>(); + +data2.data = [...Array(100).keys()].map((i) => ({ + id: i, + name: `name-${i}`, + other: `other-${i}`, +})); + +const ScrollableTemplate: Story = (args) => ({ + props: { + dataSource: data2, + sortFn: (a: any, b: any) => a.id - b.id, + }, + template: ` + + + + + Id + Name + Other + + + + + {{ r.id }} + {{ r.name }} + {{ r.other }} + + + + + `, +}); + +export const Scrollable = ScrollableTemplate.bind({});