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

Support tabindex overriding for Button, MenuButton, ToggleButton, Checkbox, and Anchor #2110

Merged
merged 19 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
@@ -0,0 +1,7 @@
{
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
"type": "patch",
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
"comment": "Support tabindex overriding for Button, MenuButton, ToggleButton, Checkbox, and Anchor",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
9 changes: 9 additions & 0 deletions packages/eslint-config-nimble/typescript.js
rajsite marked this conversation as resolved.
Show resolved Hide resolved
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
22 changes: 22 additions & 0 deletions packages/nimble-components/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,28 @@ 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:

```html
html<MyComponent
>`
<nimble-button ... tabindex="${x => x.tabIndex}"></nimble-button>
// or for an element that isn't focusable by default:
<div ... tabindex="${x => (x.disabled ? null : x.tabIndex ?? 0)}"></div>
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
`;</MyComponent
>
```

### 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;

m-akinc marked this conversation as resolved.
Show resolved Hide resolved
/**
* @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 () => {
await connect();

element.setAttribute('tabindex', '-1');
element.removeAttribute('tabindex');
m-akinc marked this conversation as resolved.
Show resolved Hide resolved
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
Loading