Skip to content

Commit

Permalink
Support tabindex overriding for Button, MenuButton, ToggleButton, Che…
Browse files Browse the repository at this point in the history
…ckbox, and Anchor (#2110)

# Pull Request

## 🤨 Rationale

Fixes #2094

## 👩‍💻 Implementation

For each of `Button`, `MenuButton`, `ToggleButton`, 'Checkbox', and
`Anchor`, added `tabindex` attribute, updated the template to forward
the `tabindex` value, and wrote automated tests. Additionally for
`Button` and `Checkbox`, forked template from FAST and copied over the
FAST tests.

## 🧪 Testing

Ran tests. Manually tested in Storybook in Chrome and Firefox.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
  • Loading branch information
m-akinc and rajsite authored May 21, 2024
1 parent c60497e commit 32fcfaf
Show file tree
Hide file tree
Showing 20 changed files with 1,481 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Support tabindex overriding for Button, MenuButton, ToggleButton, Checkbox, and Anchor",
"packageName": "@ni/nimble-components",
"email": "7282195+m-akinc@users.noreply.github.com",
"dependentChangeType": "patch"
}
9 changes: 9 additions & 0 deletions packages/eslint-config-nimble/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@ module.exports = {
]
}
},
{
files: ['*.foundation.spec.ts'],
rules: {
'no-restricted-imports': [
'error',
{ paths: restrictedImportsPaths() }
]
}
},
{
files: ['styles.ts'],
rules: {
Expand Down
29 changes: 29 additions & 0 deletions packages/nimble-components/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,35 @@ const nimbleButton = Button.compose({
});
```

If delegating focus, you must forward the `tabindex` attribute to any focusable elements in the shadow DOM. While some browsers (e.g. Chrome) will work properly without forwarding, others (e.g. Firefox) won't. Override the `tabIndex` property and mark it as an attribute:

```ts
export class MyComponent {
...
@attr({ attribute: 'tabindex', converter: nullableNumberConverter })
public override tabIndex!: number;
}
```

Then in the template, bind the focusable elements' `tabindex` to the host component's property:

<!-- prettier-ignore -->
```html
html<MyComponent>`
<nimble-button
...
tabindex="${x => x.tabIndex}">
</nimble-button>
// or for an element that isn't focusable by default:
<div
...
tabindex="${x => {
const tabindex = x.tabIndex ?? 0;
return x.disabled ? undefined : `${tabindex}`;
}">
</div>`;
```

### Leverage mixins for shared APIs across components

TypeScript and the FAST library each offer patterns and/or mechanisms to alter the APIs for a component via a mixin.
Expand Down
10 changes: 9 additions & 1 deletion packages/nimble-components/src/anchor/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { attr } from '@microsoft/fast-element';
import { attr, nullableNumberConverter } from '@microsoft/fast-element';
import {
DesignSystem,
Anchor as FoundationAnchor,
Expand Down Expand Up @@ -35,6 +35,14 @@ export class Anchor extends AnchorBase {
@attr
public appearance: AnchorAppearance;

/**
* @public
* @remarks
* HTML Attribute: tabindex
*/
@attr({ attribute: 'tabindex', converter: nullableNumberConverter })
public override tabIndex!: number;

/**
* @public
* @remarks
Expand Down
1 change: 1 addition & 0 deletions packages/nimble-components/src/anchor/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ AnchorOptions
rel="${x => x.rel}"
target="${x => x.target}"
type="${x => x.type}"
tabindex="${x => x.tabIndex}"
aria-atomic="${x => x.ariaAtomic}"
aria-busy="${x => x.ariaBusy}"
aria-controls="${x => x.ariaControls}"
Expand Down
19 changes: 19 additions & 0 deletions packages/nimble-components/src/anchor/tests/anchor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,25 @@ describe('Anchor', () => {
expect(element.control!.getAttribute(name)).toBe('foo');
});
});

it('for attribute tabindex', async () => {
await connect();

element.setAttribute('tabindex', '-1');
await waitForUpdatesAsync();

expect(element.control!.getAttribute('tabindex')).toBe('-1');
});
});

it('should clear tabindex attribute from the internal control when cleared from the host', async () => {
element.setAttribute('tabindex', '-1');
await connect();

element.removeAttribute('tabindex');
await waitForUpdatesAsync();

expect(element.control!.hasAttribute('tabindex')).toBeFalse();
});

describe('contenteditable behavior', () => {
Expand Down
12 changes: 10 additions & 2 deletions packages/nimble-components/src/button/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import { attr } from '@microsoft/fast-element';
import { attr, nullableNumberConverter } from '@microsoft/fast-element';
import {
Button as FoundationButton,
ButtonOptions,
buttonTemplate as template,
DesignSystem
} from '@microsoft/fast-foundation';
import type {
ButtonPattern,
ButtonAppearanceVariantPattern
} from '../patterns/button/types';
import { styles } from './styles';
import { template } from './template';
import { ButtonAppearance, ButtonAppearanceVariant } from './types';

declare global {
Expand Down Expand Up @@ -47,6 +47,14 @@ export class Button
*/
@attr({ attribute: 'content-hidden', mode: 'boolean' })
public contentHidden = false;

/**
* @public
* @remarks
* HTML Attribute: tabindex
*/
@attr({ attribute: 'tabindex', converter: nullableNumberConverter })
public override tabIndex!: number;
}

/**
Expand Down
59 changes: 59 additions & 0 deletions packages/nimble-components/src/button/template.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { html, ref, slotted } from '@microsoft/fast-element';
import type { ViewTemplate } from '@microsoft/fast-element';
import {
ButtonOptions,
endSlotTemplate,
FoundationElementTemplate,
startSlotTemplate
} from '@microsoft/fast-foundation';
import type { Button } from '.';

export const template: FoundationElementTemplate<
ViewTemplate<Button>,
ButtonOptions
> = (context, definition) => html`
<button
class="control"
part="control"
?autofocus="${x => x.autofocus}"
?disabled="${x => x.disabled}"
form="${x => x.formId}"
formaction="${x => x.formaction}"
formenctype="${x => x.formenctype}"
formmethod="${x => x.formmethod}"
formnovalidate="${x => x.formnovalidate}"
formtarget="${x => x.formtarget}"
name="${x => x.name}"
type="${x => x.type}"
value="${x => x.value}"
tabindex="${x => x.tabIndex}"
aria-atomic="${x => x.ariaAtomic}"
aria-busy="${x => x.ariaBusy}"
aria-controls="${x => x.ariaControls}"
aria-current="${x => x.ariaCurrent}"
aria-describedby="${x => x.ariaDescribedby}"
aria-details="${x => x.ariaDetails}"
aria-disabled="${x => x.ariaDisabled}"
aria-errormessage="${x => x.ariaErrormessage}"
aria-expanded="${x => x.ariaExpanded}"
aria-flowto="${x => x.ariaFlowto}"
aria-haspopup="${x => x.ariaHaspopup}"
aria-hidden="${x => x.ariaHidden}"
aria-invalid="${x => x.ariaInvalid}"
aria-keyshortcuts="${x => x.ariaKeyshortcuts}"
aria-label="${x => x.ariaLabel}"
aria-labelledby="${x => x.ariaLabelledby}"
aria-live="${x => x.ariaLive}"
aria-owns="${x => x.ariaOwns}"
aria-pressed="${x => x.ariaPressed}"
aria-relevant="${x => x.ariaRelevant}"
aria-roledescription="${x => x.ariaRoledescription}"
${ref('control')}
>
${startSlotTemplate(context, definition)}
<span class="content" part="content">
<slot ${slotted('defaultSlottedContent')}></slot>
</span>
${endSlotTemplate(context, definition)}
</button>
`;
Loading

0 comments on commit 32fcfaf

Please sign in to comment.