Skip to content

Commit

Permalink
update dialog trap focus to default to false (#6016)
Browse files Browse the repository at this point in the history 
* change trapFocus on dialog to noFocusTrap to better support boolean attributes

* Change files
  • Loading branch information
@chrisdholt
chrisdholt authored and EisenbergEffect committed May 31, 2022
1 parent fc641ba commit cfe670a
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 29 deletions.
7 changes: 7 additions & 0 deletions change/@microsoft-fast-foundation-f2ecedda-fb31-42c8-9919-9b0f157cce36.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "change trapFocus on dialog to noFocusTrap to better support boolean attributes",
"packageName": "@microsoft/fast-foundation",
"email": "[email protected]",
"dependentChangeType": "patch"
}
4 changes: 1 addition & 3 deletions packages/web-components/fast-foundation/docs/api-report.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1085,10 +1085,8 @@ export class Dialog extends FoundationElement {
hidden: boolean;
hide(): void;
modal: boolean;
noFocusTrap: boolean;
show(): void;
trapFocus: boolean;
// (undocumented)
protected trapFocusChanged: () => void;
}

// @public
Expand Down
25 changes: 12 additions & 13 deletions packages/web-components/fast-foundation/src/dialog/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,17 @@ export const myDialog = Dialog.compose({

#### Fields

| Name | Privacy | Type | Default | Description | Inherited From |
| ------------------ | --------- | ------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- |
| `modal` | public | `boolean` | `false` | Indicates the element is modal. When modal, user mouse interaction will be limited to the contents of the element by a modal overlay. Clicks on the overlay will cause the dialog to emit a "dismiss" event. | |
| `hidden` | public | `boolean` | `false` | The hidden state of the element. | |
| `trapFocus` | public | `boolean` | `true` | Indicates that the dialog should trap focus. | |
| `trapFocusChanged` | protected | | | | |
| `ariaDescribedby` | public | `string` | | The id of the element describing the dialog. | |
| `ariaLabelledby` | public | `string` | | The id of the element labeling the dialog. | |
| `ariaLabel` | public | `string` | | The label surfaced to assistive technologies. | |
| `$presentation` | public | `ComponentPresentation or null` | | A property which resolves the ComponentPresentation instance for the current component. | FoundationElement |
| `template` | public | `ElementViewTemplate or void or null` | | Sets the template of the element instance. When undefined, the element will attempt to resolve the template from the associated presentation or custom element definition. | FoundationElement |
| `styles` | public | `ElementStyles or void or null` | | Sets the default styles for the element instance. When undefined, the element will attempt to resolve default styles from the associated presentation or custom element definition. | FoundationElement |
| Name | Privacy | Type | Default | Description | Inherited From |
| ----------------- | ------- | ------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- |
| `modal` | public | `boolean` | `false` | Indicates the element is modal. When modal, user mouse interaction will be limited to the contents of the element by a modal overlay. Clicks on the overlay will cause the dialog to emit a "dismiss" event. | |
| `hidden` | public | `boolean` | `false` | The hidden state of the element. | |
| `noFocusTrap` | public | `boolean` | `false` | Indicates that the dialog should not trap focus. | |
| `ariaDescribedby` | public | `string` | | The id of the element describing the dialog. | |
| `ariaLabelledby` | public | `string` | | The id of the element labeling the dialog. | |
| `ariaLabel` | public | `string` | | The label surfaced to assistive technologies. | |
| `$presentation` | public | `ComponentPresentation or null` | | A property which resolves the ComponentPresentation instance for the current component. | FoundationElement |
| `template` | public | `ElementViewTemplate or void or null` | | Sets the template of the element instance. When undefined, the element will attempt to resolve the template from the associated presentation or custom element definition. | FoundationElement |
| `styles` | public | `ElementStyles or void or null` | | Sets the default styles for the element instance. When undefined, the element will attempt to resolve default styles from the associated presentation or custom element definition. | FoundationElement |

#### Methods

Expand All @@ -98,7 +97,7 @@ export const myDialog = Dialog.compose({
| ------------------ | --------------- | -------------- |
| | modal | |
| | hidden | |
| `trap-focus` | trapFocus | |
| `no-focus-trap` | noFocusTrap | |
| `aria-describedby` | ariaDescribedby | |
| `aria-labelledby` | ariaLabelledby | |
| `aria-label` | ariaLabel | |
Expand Down
12 changes: 6 additions & 6 deletions packages/web-components/fast-foundation/src/dialog/dialog.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,27 +128,27 @@ describe("Dialog", () => {
await disconnect();
});

it("should add an attribute of `trap-focus` when trapFocus is true", async () => {
it("should add an attribute of `no-focus-trap` when noFocusTrap is true", async () => {
const { element, connect, disconnect } = await setup();

element.trapFocus = true;
element.noFocusTrap = true;

await connect();
await Updates.next();

expect(element.hasAttribute("trap-focus")).to.equal(true);
expect(element.hasAttribute("no-focus-trap")).to.equal(true);

await disconnect();
});

it("should add a default attribute of `trap-focus` when trapFocus not defined", async () => {
it("should NOT add a default attribute of `no-focus-trap` when noFocusTrap not defined", async () => {
const { element, connect, disconnect } = await setup();

await connect();
await Updates.next();

expect(element.trapFocus).to.equal(true);
expect(element.hasAttribute("trap-focus")).to.equal(true);
expect(element.noFocusTrap).to.equal(false);
expect(element.hasAttribute("no-focus-trap")).to.equal(false);

await disconnect();
});
Expand Down
14 changes: 7 additions & 7 deletions packages/web-components/fast-foundation/src/dialog/dialog.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ export class Dialog extends FoundationElement {
public hidden: boolean = false;

/**
* Indicates that the dialog should trap focus.
* Indicates that the dialog should not trap focus.
*
* @public
* @defaultValue - true
* @remarks
* HTML Attribute: trap-focus
* HTML Attribute: no-focus-trap
*/
@attr({ attribute: "trap-focus", mode: "boolean" })
public trapFocus: boolean = true;
protected trapFocusChanged = (): void => {
@attr({ attribute: "no-focus-trap", mode: "boolean" })
public noFocusTrap: boolean = false;
private noFocusTrapChanged = (): void => {
if ((this as FoundationElement).$fastController.isConnected) {
this.updateTrapFocus();
}
Expand Down Expand Up @@ -190,7 +190,7 @@ export class Dialog extends FoundationElement {
};

private handleTabKeyDown = (e: KeyboardEvent): void => {
if (!this.trapFocus || this.hidden) {
if (this.noFocusTrap || this.hidden) {
return;
}

Expand Down Expand Up @@ -250,7 +250,7 @@ export class Dialog extends FoundationElement {
* we should we be active trapping focus
*/
private shouldTrapFocus = (): boolean => {
return this.trapFocus && !this.hidden;
return !this.noFocusTrap && !this.hidden;
};

/**
Expand Down

0 comments on commit cfe670a

Please sign in to comment.