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 keyboard interaction (draft / partial implementation) #2070

Closed
wants to merge 34 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
e823d57
Prototype as of 4/3
msmithNI Apr 4, 2024
b974ba5
Merge from main
msmithNI Apr 30, 2024
4dad3d3
Updated keyboard nav code
msmithNI May 2, 2024
07cc96b
Lint/prettier
msmithNI May 2, 2024
f57ac31
Add TODO
msmithNI May 2, 2024
6f2126b
Merge from main
msmithNI May 2, 2024
893db0c
Merge branch 'main' into users/masmith/table-keyboard-interaction-temp
msmithNI May 2, 2024
e185f0d
Fix test, less console.log noise when body focused
msmithNI May 3, 2024
65ab3dd
Remove no-op call
msmithNI May 6, 2024
078fccb
Bugfix for Safari Mac issue, temporary fix for scrolling down via Dow…
msmithNI May 10, 2024
afaa3cb
Remove tabIndexOverride
msmithNI May 10, 2024
247b51a
Undo no-op change
msmithNI May 10, 2024
0ba987e
Feature complete keyboard nav
msmithNI May 17, 2024
0a92870
Cleanup / fix F2 for single-element cells
msmithNI May 17, 2024
7a7ce21
prettier
msmithNI May 17, 2024
e29b789
Fix action menu Shift-Tab issue
msmithNI May 17, 2024
5110178
Prettier
msmithNI May 17, 2024
4248deb
Simplify onTabPressed
msmithNI May 20, 2024
531eac3
Fix anchor view tabbableChildren implementation, get rid of extra vie…
msmithNI May 20, 2024
249c1bf
Cleanup (methods to set focus type, Tab for headers uses similar logi…
msmithNI May 21, 2024
c6244a1
Merge branch 'main' into users/masmith/table-keyboard-interaction-temp
msmithNI May 22, 2024
8f3c5ce
Fix build
msmithNI May 22, 2024
cacf215
Uptake tabindex changes
msmithNI May 22, 2024
adfb7a7
Fix column header tabIndex behavior
msmithNI May 22, 2024
b23ea0d
Fix issues scrolling with a focused cell (+ switch back to observers)
msmithNI May 22, 2024
0a26787
Fix build
msmithNI May 22, 2024
0d5ff72
Refactoring/cleanup
msmithNI May 29, 2024
6dbad6d
Cleanup
msmithNI May 30, 2024
44f0936
Remove (flatten) TableFocusState into kbd nav code
msmithNI May 30, 2024
f6e6a34
Fix some issues with anchor columns / action menus
msmithNI May 30, 2024
e472265
Add anchor to lastName in Storybook (for testing)
msmithNI May 30, 2024
cccd1d2
Fix another 'single interactive element' corner case with Tab
msmithNI May 30, 2024
9a32fb3
Remove 'single interactive element' special case
msmithNI May 30, 2024
c80b0f0
Update Enter (when cell focused) to focus content, but not activate it
msmithNI May 31, 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
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ TableColumnAnchorColumnConfig
public override focusedRecycleCallback(): void {
this.anchor?.blur();
}

public override get tabbableChildren(): HTMLElement[] {
// this.anchor can be initialized even when not active in the template, so make sure it's in our DOM
if (this.anchor?.getRootNode() === this.shadowRoot) {
return [this.anchor];
}
return [];
}
}

const anchorCellView = TableColumnAnchorCellView.compose({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export const template = html<TableColumnAnchorCellView>`
<${anchorTag}
${ref('anchor')}
${overflow('hasOverflow')}
tabindex="-1"
href="${x => x.cellRecord?.href}"
hreflang="${x => x.columnConfig?.hreflang}"
ping="${x => x.columnConfig?.ping}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ export abstract class TableCellView<
@observable
public recordId?: string;

/**
* Gets the child elements in this cell view that should be able to be reached via Tab/ Shift-Tab,
* if any.
*/
public get tabbableChildren(): HTMLElement[] {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a table column type will contain focusable/tabbable elements, our guidance would be:

  • That column's cell view overrides this property, and returns the focusable element(s) in the array
  • The focusable elements should be tabindex=-1 by default

KeyboardNavigationManager uses this when handling Tab/Shift-Tab for a given row, and to reflect the correct focus state if one of these elements is focused (TableFocusType.cellContent).

Currently KeyboardNavigationManager updates tabindex of these elements directly (except for the tabIndexOverride cases described previously). For arbitrary cell content, this might not be sufficient, since the element that should be tabindex=0 could be an element descendant in a shadow root or something similar. So we may need to add an additional API/method to handle focusing/unfocusing these children.

return [];
}

private delegatedEvents: readonly string[] = [];

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ export const ExampleSortType = {
secondColumnDescendingFirstColumnAscending:
'SecondColumnDescendingFirstColumnAscending',
firstColumnAscendingSecondColumnDisabled:
'FirstColumnAscendingSecondColumnDisabled'
'FirstColumnAscendingSecondColumnDisabled',
allColumnsDisabled: 'AllColumnsDisabled'
} as const;
export type ExampleSortType =
(typeof ExampleSortType)[keyof typeof ExampleSortType];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {
} from '../../../table-column/base/types';
import { styles } from './styles';
import { template } from './template';
import type { TableCellView } from '../../../table-column/base/cell-view';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -52,6 +53,11 @@ export class TableCell<

public readonly actionMenuButton?: MenuButton;

/** @internal */
public get cellView(): TableCellView<TCellRecord> {
return this.shadowRoot?.firstElementChild as TableCellView<TCellRecord>;
}

public onActionMenuBeforeToggle(
event: CustomEvent<MenuButtonToggleEventDetail>
): void {
Expand Down
11 changes: 11 additions & 0 deletions packages/nimble-components/src/table/components/cell/styles.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { css } from '@microsoft/fast-element';
import { display } from '../../../utilities/style/display';
import {
borderHoverColor,
controlHeight,
controlSlimHeight,
mediumPadding
} from '../../../theme-provider/design-tokens';
import { focusVisible } from '../../../utilities/style/focus';

export const styles = css`
${display('flex')}
Expand All @@ -22,6 +24,11 @@ export const styles = css`
--ni-private-table-cell-action-menu-display: block;
}

:host(${focusVisible}) {
outline: 2px solid ${borderHoverColor};
outline-offset: -2px;
}

.cell-view {
overflow: hidden;
}
Expand All @@ -34,4 +41,8 @@ export const styles = css`
height: ${controlSlimHeight};
align-self: center;
}

.action-menu.focused {
display: block;
atmgrifter00 marked this conversation as resolved.
Show resolved Hide resolved
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const template = html<TableCell>`
<${menuButtonTag} ${ref('actionMenuButton')}
content-hidden
appearance="${ButtonAppearance.ghost}"
tabindex="-1"
@beforetoggle="${(x, c) => x.onActionMenuBeforeToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>)}"
@toggle="${(x, c) => x.onActionMenuToggle(c.event as CustomEvent<MenuButtonToggleEventDetail>)}"
@click="${(_, c) => c.event.stopPropagation()}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import { attr, observable } from '@microsoft/fast-element';
import {
Checkbox,
DesignSystem,
FoundationElement
} from '@microsoft/fast-foundation';
import type { TableColumn } from '../../../table-column/base';
import { styles } from './styles';
import { template } from './template';
import {
TableRowFocusableElements,
TableRowSelectionState,
TableRowSelectionToggleEventDetail
} from '../../types';
import type { Checkbox } from '../../../checkbox';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -29,6 +30,9 @@ export class TableGroupRow extends FoundationElement {
@observable
public nestingLevel = 0;

@observable
public dataIndex?: number;

@observable
public immediateChildCount?: number;

Expand Down Expand Up @@ -83,7 +87,7 @@ export class TableGroupRow extends FoundationElement {
}

/** @internal */
public onSelectionChange(event: CustomEvent): void {
public onSelectionCheckboxChange(event: CustomEvent): void {
if (this.ignoreSelectionChangeEvents) {
return;
}
Expand All @@ -100,6 +104,11 @@ export class TableGroupRow extends FoundationElement {
this.$emit('group-selection-toggle', detail);
}

/** @internal */
public getFocusableElements(): TableRowFocusableElements {
return { selectionCheckbox: this.selectionCheckbox, cells: [] };
}

private selectionStateChanged(): void {
this.setSelectionCheckboxState();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { White } from '@ni/nimble-tokens/dist/styledictionary/js/tokens';
import { display } from '../../../utilities/style/display';
import {
applicationBackgroundColor,
borderHoverColor,
borderWidth,
controlHeight,
fillHoverColor,
Expand All @@ -14,6 +15,7 @@ import { hexToRgbaCssColor } from '../../../utilities/style/colors';
import { themeBehavior } from '../../../utilities/style/theme';
import { userSelectNone } from '../../../utilities/style/user-select';
import { styles as expandCollapseStyles } from '../../../patterns/expand-collapse/styles';
import { focusVisible } from '../../../utilities/style/focus';

export const styles = css`
${display('grid')}
Expand Down Expand Up @@ -55,6 +57,11 @@ export const styles = css`
background-color: ${fillHoverColor};
}

:host(${focusVisible}) {
outline: 2px solid ${borderHoverColor};
outline-offset: -2px;
}

.expand-collapse-button {
margin-left: calc(
${mediumPadding} + ${standardPadding} * 2 *
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const template = html<TableGroupRow>`
<${checkboxTag}
${ref('selectionCheckbox')}
class="selection-checkbox"
@change="${(x, c) => x.onSelectionChange(c.event as CustomEvent)}"
@change="${(x, c) => x.onSelectionCheckboxChange(c.event as CustomEvent)}"
@click="${(_, c) => c.event.stopPropagation()}"
title="${x => tableGroupSelectAllLabel.getValueFor(x)}"
aria-label="${x => tableGroupSelectAllLabel.getValueFor(x)}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { css } from '@microsoft/fast-element';
import { display } from '../../../utilities/style/display';
import {
borderHoverColor,
controlHeight,
iconColor,
mediumPadding,
tableHeaderFont,
tableHeaderFontColor
} from '../../../theme-provider/design-tokens';
import { focusVisible } from '../../../utilities/style/focus';

export const styles = css`
${display('flex')}
Expand All @@ -23,6 +25,11 @@ export const styles = css`
cursor: default;
}

:host(${focusVisible}) {
outline: 2px solid ${borderHoverColor};
outline-offset: -2px;
}

.sort-indicator,
.grouped-indicator {
flex: 0 0 auto;
Expand Down
47 changes: 39 additions & 8 deletions packages/nimble-components/src/table/components/row/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
volatile
} from '@microsoft/fast-element';
import {
Checkbox,
DesignSystem,
FoundationElement
} from '@microsoft/fast-foundation';
Expand All @@ -18,15 +17,17 @@ import type {
TableFieldName,
TableRecord,
TableRowExpansionToggleEventDetail,
TableRowFocusableElements,
TableRowSelectionToggleEventDetail
} from '../../types';
import type { TableColumn } from '../../../table-column/base';
import type { MenuButtonToggleEventDetail } from '../../../menu-button/types';
import { TableCell } from '../cell';
import { TableCell, tableCellTag } from '../cell';
import {
ColumnInternals,
isColumnInternalsProperty
} from '../../../table-column/base/models/column-internals';
import type { Checkbox } from '../../../checkbox';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -77,6 +78,9 @@ export class TableRow<
@observable
public nestingLevel = 0;

@observable
public dataIndex?: number;

@attr({ attribute: 'is-parent-row', mode: 'boolean' })
public isParentRow = false;

Expand Down Expand Up @@ -109,6 +113,10 @@ export class TableRow<
@observable
public readonly selectionCheckbox?: Checkbox;

/** @internal */
@observable
public readonly expandCollapseButton?: HTMLElement;

/** @internal */
public readonly cellContainer!: HTMLSpanElement;

Expand All @@ -133,6 +141,11 @@ export class TableRow<
return this.isParentRow && this.nestingLevel > 0;
}

@volatile
public get isInHierarchy(): boolean {
return this.isParentRow || this.nestingLevel > 0;
}

// Programmatically updating the selection state of a checkbox fires the 'change' event.
// Therefore, selection change events that occur due to programmatically updating
// the selection checkbox 'checked' value should be ingored.
Expand All @@ -149,17 +162,22 @@ export class TableRow<
}

/** @internal */
public onSelectionChange(event: CustomEvent): void {
public onSelectionCheckboxChange(event: CustomEvent): void {
if (this.ignoreSelectionChangeEvents) {
return;
}

const checkbox = event.target as Checkbox;
const checked = checkbox.checked;
this.selected = checked;
this.onSelectionChange(!checked, checked);
}

/** @internal */
public onSelectionChange(oldState: boolean, newState: boolean): void {
this.selected = newState;
const detail: TableRowSelectionToggleEventDetail = {
oldState: !checked,
newState: checked
oldState,
newState
};
this.$emit('row-selection-toggle', detail);
}
Expand Down Expand Up @@ -213,14 +231,27 @@ export class TableRow<
}
}

public onRowExpandToggle(event: Event): void {
/** @internal */
public getFocusableElements(): TableRowFocusableElements {
const result: TableRowFocusableElements = { cells: [] };
result.selectionCheckbox = this.selectionCheckbox;
this.shadowRoot!.querySelectorAll(tableCellTag).forEach(cell => {
result.cells.push({
actionMenuButton: cell.actionMenuButton,
cell
});
});
return result;
}

public onRowExpandToggle(event?: Event): void {
const expandEventDetail: TableRowExpansionToggleEventDetail = {
oldState: this.expanded,
newState: !this.expanded,
recordId: this.recordId!
};
this.$emit('row-expand-toggle', expandEventDetail);
event.stopImmediatePropagation();
event?.stopImmediatePropagation();
// To avoid a visual glitch with improper expand/collapse icons performing an
// animation (due to visual re-use apparently), we apply a class to the
// contained expand-collapse button temporarily. We use the 'transitionend' event
Expand Down
Loading
Loading