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

Table - Sort by selected #2387

Merged
merged 61 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
86ac456
trying new approach
zamoore Sep 4, 2024
9d9e291
sorting works with minimized changeset
zamoore Sep 4, 2024
a59220d
can only sort by selected on enabled tables
zamoore Sep 4, 2024
c903e1c
fixed styling
zamoore Sep 4, 2024
88c0101
adding showcase demo
zamoore Sep 5, 2024
579f6e3
custom sort by selected works well
zamoore Sep 5, 2024
e4e5452
updating docs
zamoore Sep 5, 2024
7d59fcc
addressing ts error
zamoore Sep 5, 2024
dea66cf
adding new tests
zamoore Sep 5, 2024
7616d69
adding test to table for sorting by selected
zamoore Sep 5, 2024
9c5e12a
adding index test for sorting by selected
zamoore Sep 5, 2024
e698453
finished new index test
zamoore Sep 5, 2024
b5a00a3
added updated docs for the how-to-use file
zamoore Sep 6, 2024
a41254b
cleaning up for docs
zamoore Sep 6, 2024
e21168c
fixing linting errors
zamoore Sep 6, 2024
3cbaf38
fixing failing tests
zamoore Sep 6, 2024
df62e7f
simplifying api
zamoore Sep 6, 2024
5730a3d
removed second example
zamoore Sep 10, 2024
14fb835
refining implementation
zamoore Sep 10, 2024
78e77bf
responding to pr feedback
zamoore Sep 10, 2024
d784d95
responding to pr feedback
zamoore Sep 10, 2024
6686ab4
preserving some deleted doc comments
zamoore Sep 10, 2024
324189e
removed dynamic classname
zamoore Sep 10, 2024
76bd11b
remove doc updates
zamoore Sep 11, 2024
677ebed
adding showcase example
zamoore Sep 11, 2024
fd524df
added test for sorting by selected item key without a model
zamoore Sep 11, 2024
d550b89
added sortable examples to ThSelectable
zamoore Sep 11, 2024
2e9438f
addressing pr feedback
zamoore Sep 11, 2024
593139e
addressing linting issue
zamoore Sep 11, 2024
6a605f0
fixing test
zamoore Sep 12, 2024
436fc5a
fixed failing test
zamoore Sep 12, 2024
94a4fbe
responding to pr feedback. added changeset
zamoore Sep 12, 2024
51cbd11
fixing bad example
zamoore Sep 12, 2024
b484faf
adding derived state to determine sort button visibility
zamoore Sep 12, 2024
a68dcd8
addressing extraneous aria-sort
zamoore Sep 12, 2024
5d0fee1
fixing aria issues
zamoore Sep 12, 2024
92b2ae5
responding to PR feedback
zamoore Sep 12, 2024
6225c48
simplified the api a bit
zamoore Sep 12, 2024
c839677
changed argument name for clarity
zamoore Sep 12, 2024
7c49ebe
fixing test
zamoore Sep 12, 2024
32af8fc
further simplified
zamoore Sep 12, 2024
2b11d1e
finished simplifying
zamoore Sep 12, 2024
c28671a
removed debugging code
zamoore Sep 12, 2024
26fd8d9
linting error
zamoore Sep 12, 2024
5211a48
fixed indeterminate demo
zamoore Sep 12, 2024
464f9ca
reorganized some demo code and renamed some variables/functions for c…
didoo Sep 13, 2024
69d0896
removed demo that is not meaningful
didoo Sep 13, 2024
f06e29b
removed drilled down arguments related to sorting, for static/yielded…
didoo Sep 13, 2024
c889cbe
fixed tests for table index
didoo Sep 13, 2024
27f1914
responding to pr feedback
zamoore Sep 13, 2024
85f6d50
responding to pr feedback
zamoore Sep 13, 2024
24bc1f3
fixing build error
zamoore Sep 13, 2024
8afce3e
added more detail to changeset
zamoore Sep 13, 2024
37357d8
restored drilled down arguments related to sorting, for static/yielde…
didoo Sep 16, 2024
ee8b4da
renamed variable
didoo Sep 16, 2024
ad26142
added new example to showcase for “sort by selected” with yielded `<t…
didoo Sep 16, 2024
7842bb8
added back missing data row in showcase
didoo Sep 16, 2024
0af97b5
Update .changeset/large-spiders-protect.md
zamoore Sep 16, 2024
fb08566
responding to pr feedback
zamoore Sep 16, 2024
7bdc841
Update packages/components/src/components/hds/table/index.ts
zamoore Sep 17, 2024
1d4ac7c
responding to pr feedback
zamoore Sep 17, 2024
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
15 changes: 15 additions & 0 deletions .changeset/large-spiders-protect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@hashicorp/design-system-components": minor
---

`Hds::Table`
zamoore marked this conversation as resolved.
Show resolved Hide resolved
- Added `@selectableColumnKey` argument which enables sorting by row selection state and specifies the corresponding selection state key.

`Hds::Table::Tr`
- Added `@selectableColumnKey` argument which enables sorting by row selection state and specifies the corresponding selection state key.
- Added `@sortBySelectedOrder` argument which determines the state of the sort button in the selected item column.
- Added `@onClickSortBySelected` argument which is the callback for the sort button in the selected item column.

`Hds::Table::ThSelectable`
- Added `@onClickSortBySelected` argument which is the callback for the sort button in the selected item column.
- Added `@sortBySelectedOrder` argument which determines the state of the sort button in the selected item column.
Comment on lines +8 to +15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: some of these are for internal use; I don't think we plan on documenting them all (but I might be wrong) so I suggest we add to the changelog only the arguments we will expose in the component API

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was what I had originally, but @didoo suggested that I document all of the additions to subcomponents as well. Curious to hear his thoughts

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of changelog, we all agree the changes should be mentioned.
For the website, personally I would document them anyway (and in case add a note clarifying that it's meant for internal use). That said, I totally get that the Table API documentation is already heavy so if you decide differently I would not oppose (but consider that, at times, we use the component API documentation too, to understand what a certain argument does).

4 changes: 4 additions & 0 deletions packages/components/src/components/hds/table/index.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
{{#if @columns}}
<Hds::Table::Tr
@selectionScope="col"
@onClickSortBySelected={{if @selectableColumnKey (fn this.setSortBy @selectableColumnKey)}}
@sortBySelectedOrder={{if (eq this.sortBy @selectableColumnKey) this.sortOrder}}
@isSelectable={{@isSelectable}}
@onSelectionChange={{this.onSelectionAllChange}}
@didInsert={{this.didInsertSelectAllCheckbox}}
Expand Down Expand Up @@ -52,6 +54,8 @@
didInsert=this.didInsertSelectAllCheckbox
willDestroy=this.willDestroySelectAllCheckbox
selectionAriaLabelSuffix="all rows"
onClickSortBySelected=(if @selectableColumnKey (fn this.setSortBy @selectableColumnKey))
sortBySelectedOrder=(if (eq this.sortBy @selectableColumnKey) this.sortOrder)
)
Th=(component "hds/table/th")
ThSort=(component "hds/table/th-sort")
Expand Down
4 changes: 3 additions & 1 deletion packages/components/src/components/hds/table/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
HdsTableSortingFunction,
HdsTableThSortOrder,
HdsTableVerticalAlignment,
HdsTableModel,
} from './types';
import type { HdsFormCheckboxBaseSignature } from '../form/checkbox/base';
import type { HdsTableTdArgs } from './td.ts';
Expand Down Expand Up @@ -50,11 +51,12 @@ export interface HdsTableArgs {
isFixedLayout?: boolean;
isSelectable?: boolean;
isStriped?: boolean;
model: Array<Record<string, unknown>>;
model?: HdsTableModel;
onSelectionChange?: (selection: HdsTableOnSelectionChangeArgs) => void;
onSort?: (sortBy: string, sortOrder: HdsTableThSortOrder) => void;
selectionAriaLabelSuffix?: string;
sortBy?: string;
selectableColumnKey?: string;
sortedMessageText?: string;
sortOrder?: HdsTableThSortOrder;
valign?: HdsTableVerticalAlignment;
Expand Down
30 changes: 3 additions & 27 deletions packages/components/src/components/hds/table/th-button-sort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,10 @@ export interface HdsTableThButtonSortArgs {
const NOOP = () => {};

export default class HdsTableThButtonSort extends Component<HdsTableThButtonSortArgs> {
/**
* Generates a unique ID for the (hidden) "label prefix/suffix" <span> elements
*
* @param prefixLabelId/suffixLabelId
*/
// Generates a unique ID for the (hidden) "label prefix/suffix" <span> elements
prefixLabelId = 'prefix-' + guidFor(this);
suffixLabelId = 'suffix-' + guidFor(this);

/**
* @param icon
* @type {HdsTableThSortOrderIcons}
* @private
* @default swap-vertical
* @description Determines which icon to use based on the sort order defined
*/
get icon(): HdsTableThSortOrderIcons {
switch (this.args.sortOrder) {
case HdsTableThSortOrderValues.Asc:
Expand All @@ -53,22 +42,14 @@ export default class HdsTableThButtonSort extends Component<HdsTableThButtonSort
}
}

/**
* @param sortOrderLabel
* @default 'ascending'
* @description Determines the label (suffix) to use in the `aria-labelledby` attribute of the button, used to indicate what will happen if the user clicks on the button
*/
// Determines the label (suffix) to use in the `aria-labelledby` attribute of the button,
// used to indicate what will happen if the user clicks on the button
get sortOrderLabel(): HdsTableThSortOrderLabels {
return this.args.sortOrder === HdsTableThSortOrderValues.Asc
? HdsTableThSortOrderLabelValues.Desc
: HdsTableThSortOrderLabelValues.Asc;
}

/**
* @param onClick
* @type {function}
* @default () => {}
*/
get onClick(): () => void {
const { onClick } = this.args;

Expand All @@ -79,11 +60,6 @@ export default class HdsTableThButtonSort extends Component<HdsTableThButtonSort
}
}

/**
* Get the class names to apply to the component.
* @method classNames
* @return {string} The "class" attribute to apply to the component.
*/
get classNames(): string {
const classes = ['hds-table__th-button', 'hds-table__th-button--sort'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@ export interface HdsTableThButtonTooltipArgs {
}

export default class HdsTableThButtonTooltip extends Component<HdsTableThButtonTooltipArgs> {
/**
* Generates a unique ID for the (hidden) "label prefix" <span> element
zamoore marked this conversation as resolved.
Show resolved Hide resolved
*
* @param prefixLabelId
*/
// Generates a unique ID for the (hidden) "label prefix" <span> element
prefixLabelId = guidFor(this);

get tooltip(): string {
Expand All @@ -31,11 +27,6 @@ export default class HdsTableThButtonTooltip extends Component<HdsTableThButtonT
return this.args.tooltip;
}

/**
* Get the class names to apply to the component.
* @method classNames
* @return {string} The "class" attribute to apply to the component.
*/
get classNames(): string {
const classes = ['hds-table__th-button', 'hds-table__th-button--tooltip'];

Expand Down
35 changes: 25 additions & 10 deletions packages/components/src/components/hds/table/th-selectable.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,29 @@
SPDX-License-Identifier: MPL-2.0
}}

<Hds::Table::Th class="hds-table__th--is-selectable" @scope={{@selectionScope}} ...attributes>
<Hds::Form::Checkbox::Base
id={{this.checkboxId}}
class="hds-table__checkbox"
checked={{@isSelected}}
aria-label={{this.ariaLabel}}
{{did-insert this.didInsert}}
{{will-destroy this.willDestroyNode}}
{{on "change" this.onSelectionChange}}
/>
<Hds::Table::Th
class="hds-table__th--is-selectable"
aria-sort={{if this.isSortable this.ariaSort}}
@scope={{@selectionScope}}
...attributes
>
<div class="hds-table__th-content">
zamoore marked this conversation as resolved.
Show resolved Hide resolved
<Hds::Form::Checkbox::Base
id={{this.checkboxId}}
class="hds-table__checkbox"
checked={{@isSelected}}
aria-label={{this.ariaLabel}}
{{did-insert this.didInsert}}
{{will-destroy this.willDestroyNode}}
{{on "change" this.onSelectionChange}}
/>
{{#if this.isSortable}}
<span id={{this.labelId}} class="sr-only">selection state</span>
zamoore marked this conversation as resolved.
Show resolved Hide resolved
<Hds::Table::ThButtonSort
@sortOrder={{@sortBySelectedOrder}}
@onClick={{@onClickSortBySelected}}
@labelId={{this.labelId}}
zamoore marked this conversation as resolved.
Show resolved Hide resolved
/>
{{/if}}
</div>
</Hds::Table::Th>
37 changes: 31 additions & 6 deletions packages/components/src/components/hds/table/th-selectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@ import Component from '@glimmer/component';
import { action } from '@ember/object';
import { guidFor } from '@ember/object/internals';
import { tracked } from '@glimmer/tracking';
import {
HdsTableThSortOrderValues,
HdsTableThSortOrderLabelValues,
} from './types.ts';
import type { HdsFormCheckboxBaseSignature } from '../form/checkbox/base';
import type { HdsTableScope } from './types';
import type {
HdsTableScope,
HdsTableThSortOrder,
HdsTableThSortOrderLabels,
} from './types';
import type { HdsTableThArgs } from './th';

export interface HdsTableThSelectableArgs {
Expand All @@ -18,13 +26,15 @@ export interface HdsTableThSelectableArgs {
selectionKey?: string
) => void;
isSelected?: boolean;
onClickSortBySelected?: () => void;
onSelectionChange: (
target: HdsFormCheckboxBaseSignature['Element'],
selectionKey: string | undefined
) => void;
selectionAriaLabelSuffix?: string;
selectionKey?: string;
selectionScope: HdsTableScope;
sortBySelectedOrder?: HdsTableThSortOrder;
willDestroy: (selectionKey?: string) => void;
};
Element: HdsTableThArgs['Element'];
Expand All @@ -33,11 +43,14 @@ export interface HdsTableThSelectableArgs {
export default class HdsTableThSelectable extends Component<HdsTableThSelectableArgs> {
@tracked isSelected = this.args.isSelected;

/**
* Generate a unique ID for the Checkbox
* @return {string}
*/
checkboxId = 'checkbox-' + guidFor(this);
guid = guidFor(this);

checkboxId = `checkbox-${this.guid}`;
labelId = `label-${this.guid}`;

get isSortable(): boolean {
return this.args.onClickSortBySelected !== undefined;
}

get ariaLabel(): string {
const { selectionAriaLabelSuffix } = this.args;
Expand All @@ -49,6 +62,18 @@ export default class HdsTableThSelectable extends Component<HdsTableThSelectable
}
}

get ariaSort(): HdsTableThSortOrderLabels | undefined {
switch (this.args.sortBySelectedOrder) {
case HdsTableThSortOrderValues.Asc:
return HdsTableThSortOrderLabelValues.Asc;
case HdsTableThSortOrderValues.Desc:
return HdsTableThSortOrderLabelValues.Desc;
default:
// none is the default per the spec.
return HdsTableThSortOrderLabelValues.None;
}
}

@action
didInsert(checkbox: HdsFormCheckboxBaseSignature['Element']): void {
const { didInsert } = this.args;
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/components/hds/table/th-sort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
HdsTableThSortOrder,
HdsTableThSortOrderLabels,
} from './types.ts';
import type { HdsTableThButtonSortArgs } from './th-button-sort';

export const ALIGNMENTS: string[] = Object.values(
HdsTableHorizontalAlignmentValues
Expand All @@ -26,7 +27,7 @@ export const DEFAULT_ALIGN = HdsTableHorizontalAlignmentValues.Left;
export interface HdsTableThSortArgs {
Args: {
align?: HdsTableHorizontalAlignment;
onClickSort?: () => void;
onClickSort?: HdsTableThButtonSortArgs['Args']['onClick'];
zamoore marked this conversation as resolved.
Show resolved Hide resolved
sortOrder?: HdsTableThSortOrder;
tooltip?: string;
width?: string;
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/components/hds/table/tr.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@
@selectionScope={{@selectionScope}}
@selectionKey={{this.selectionKey}}
@selectionAriaLabelSuffix={{@selectionAriaLabelSuffix}}
@sortBySelectedOrder={{@sortBySelectedOrder}}
@didInsert={{@didInsert}}
@willDestroy={{@willDestroy}}
@onClickSortBySelected={{@onClickSortBySelected}}
@onSelectionChange={{@onSelectionChange}}
/>
{{/if}}
Expand Down
13 changes: 6 additions & 7 deletions packages/components/src/components/hds/table/tr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@
import Component from '@glimmer/component';
import { assert } from '@ember/debug';
import { HdsTableScopeValues } from './types.ts';
import type { HdsTableScope } from './types.ts';
import type { HdsTableScope, HdsTableThSortOrder } from './types.ts';
import type { HdsFormCheckboxBaseSignature } from '../form/checkbox/base';
import type { HdsTableArgs } from './index.ts';
import type { HdsTableThSelectableArgs } from './th-selectable.ts';

export interface BaseHdsTableTrArgs {
Args: {
selectableColumnKey?: HdsTableArgs['Args']['selectableColumnKey'];
isSelectable?: boolean;
isSelected?: false;
selectionAriaLabelSuffix?: string;
selectionKey?: string;
selectionScope: HdsTableScope;
sortBySelectedOrder?: HdsTableThSortOrder;
didInsert: (
checkbox: HdsFormCheckboxBaseSignature['Element'],
selectionKey?: string
Expand All @@ -25,6 +29,7 @@ export interface BaseHdsTableTrArgs {
selectionKey?: string
) => void;
willDestroy: () => void;
onClickSortBySelected?: HdsTableThSelectableArgs['Args']['onClickSortBySelected'];
};
Blocks: {
default: [];
Expand All @@ -43,13 +48,7 @@ export interface SelectableHdsTableTrArgs extends BaseHdsTableTrArgs {

// Union type to combine both possible states
export type HdsTableTrArgs = BaseHdsTableTrArgs | SelectableHdsTableTrArgs;

export default class HdsTableTr extends Component<HdsTableTrArgs> {
/**
* @param selectionKey
* @type {string}
* @default undefined
*/
get selectionKey(): string | undefined {
if (this.args.isSelectable && this.args.selectionScope === 'row') {
assert(
Expand Down
2 changes: 2 additions & 0 deletions packages/components/src/components/hds/table/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,5 @@ export interface HdsTableOnSelectionChangeArgs {
isSelected?: boolean;
}[];
}

export type HdsTableModel = Array<Record<string, unknown>>;
Loading