Skip to content

Commit

Permalink
feat: improve focus behavior in components (#7277)
Browse files Browse the repository at this point in the history
**Related Issue:** None

## Summary

- Updates all `setFocus()` methods to use `componentFocusable()` instead
of `componentLoaded()`.
- `TimePicker` component also updates in the `focusPart` private method
to use `componentFocusable()`.
- `TextArea` uses `componentLoaded()` outside of the `setFocus()`
method. This was in the `selectText()` and `resizeObserver`. I left
those them alone since they are not doing any focusing specifically.
- Added `await page.waitForChanges();` to tests that required it.
  - action-menu
  - split-button
  - commonTests helper (removed setTimeout of 0 as well)
  • Loading branch information
driskull authored Jul 6, 2023
1 parent b08c58b commit ad9fbca
Show file tree
Hide file tree
Showing 60 changed files with 134 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
} from "../../utils/conditionalSlot";
import { getSlotted, slotChangeGetAssignedElements } from "../../utils/dom";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -243,7 +243,7 @@ export class ActionBar
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.el?.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from "../../utils/conditionalSlot";
import { getSlotted } from "../../utils/dom";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -123,7 +123,7 @@ export class ActionGroup
/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
this.el.focus();
}
// --------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ describe("calcite-action-menu", () => {
expect(await actionMenu.getProperty("open")).toBe(false);

await actionMenu.callMethod("setFocus");
await page.waitForChanges();

await page.keyboard.press("Enter");

await page.waitForChanges();

expect(await actionMenu.getProperty("open")).toBe(true);
Expand Down Expand Up @@ -358,9 +358,9 @@ describe("calcite-action-menu", () => {
expect(await actionMenu.getProperty("open")).toBe(false);

await actionMenu.callMethod("setFocus");
await page.waitForChanges();

await page.keyboard.press("ArrowDown");

await page.waitForChanges();

expect(await actionMenu.getProperty("open")).toBe(true);
Expand Down Expand Up @@ -392,9 +392,9 @@ describe("calcite-action-menu", () => {
expect(await actionMenu.getProperty("open")).toBe(false);

await actionMenu.callMethod("setFocus");
await page.waitForChanges();

await page.keyboard.press("ArrowDown");

await page.waitForChanges();

const clickSpy = await actions[0].spyOnEvent("click");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { EffectivePlacement, LogicalPlacement, OverlayPositioning } from "../../
import { guid } from "../../utils/guid";
import { isActivationKey } from "../../utils/key";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -185,7 +185,7 @@ export class ActionMenu implements LoadableComponent {
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

focusElement(this.menuButtonEl);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "../../utils/conditionalSlot";
import { getSlotted, slotChangeGetAssignedElements } from "../../utils/dom";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -186,7 +186,7 @@ export class ActionPad
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.el?.focus();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/alert/alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from "../../utils/dom";
import { MenuPlacement } from "../../utils/floating-ui";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -380,7 +380,7 @@ export class Alert implements OpenCloseComponent, LoadableComponent, T9nComponen
/** Sets focus on the component's "close" button (the first focusable item). */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

const alertLinkEl: HTMLCalciteLinkElement = getSlotted(this.el, { selector: "calcite-link" });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { BlockSectionMessages } from "./assets/block-section/t9n";
import { BlockSectionToggleDisplay } from "./interfaces";
import { CSS, ICONS } from "./resources";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -105,7 +105,7 @@ export class BlockSection implements LocalizedComponent, T9nComponent, LoadableC
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
focusFirstTabbable(this.el);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/block/block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { Status } from "../interfaces";
import { BlockMessages } from "./assets/block/t9n";
import { CSS, ICONS, SLOTS } from "./resources";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -145,7 +145,7 @@ export class Block
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
focusFirstTabbable(this.el);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/button/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "../../utils/interactive";
import { connectLabel, disconnectLabel, getLabelText, LabelableComponent } from "../../utils/label";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -293,7 +293,7 @@ export class Button
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.childEl?.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import { isActivationKey } from "../../utils/key";
import { connectLabel, disconnectLabel, getLabelText, LabelableComponent } from "../../utils/label";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -140,7 +140,7 @@ export class Checkbox
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.toggleEl?.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import {
} from "../../utils/interactive";
import { createObserver } from "../../utils/observers";
import { Scale, SelectionMode } from "../interfaces";
import { componentLoaded, setComponentLoaded, setUpLoadableComponent } from "../../utils/loadable";
import {
componentFocusable,
setComponentLoaded,
setUpLoadableComponent
} from "../../utils/loadable";
/**
* @slot - A slot for adding one or more `calcite-chip`s.
*/
Expand Down Expand Up @@ -178,7 +182,7 @@ export class ChipGroup implements InteractiveComponent {
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
if (!this.disabled) {
(this.selectedItems[0] || this.items[0])?.setFocus();
}
Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/chip/chip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
disconnectConditionalSlotComponent
} from "../../utils/conditionalSlot";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -271,7 +271,7 @@ export class Chip
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
if (!this.disabled && this.interactive) {
this.containerEl?.focus();
} else if (!this.disabled && this.closable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { Channels, RGB } from "../color-picker/interfaces";
import Color from "color";
import { focusElement } from "../../utils/dom";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -365,7 +365,7 @@ export class ColorPickerHexInput implements LoadableComponent {
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

focusElement(this.hexInputNode);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import {
} from "../../utils/interactive";
import { isActivationKey } from "../../utils/key";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -652,7 +652,7 @@ export class ColorPicker
/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
this.el.focus();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
} from "../../utils/interactive";
import { connectLabel, disconnectLabel, getLabelText, LabelableComponent } from "../../utils/label";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -347,7 +347,7 @@ export class Combobox
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.textInput?.focus();
this.activeChipIndex = -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
setEndOfDay
} from "../../utils/date";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -193,7 +193,7 @@ export class DatePicker implements LocalizedComponent, LoadableComponent, T9nCom
/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
this.el.focus();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { RequestedItem } from "../dropdown-group/interfaces";
import { FlipContext, Scale, SelectionMode } from "../interfaces";
import { CSS } from "./resources";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -102,7 +102,7 @@ export class DropdownItem implements LoadableComponent {
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.el?.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
} from "../../utils/interactive";
import { isActivationKey } from "../../utils/key";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -200,7 +200,7 @@ export class Dropdown
/** Sets focus on the component's first focusable element. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);
this.el.focus();
}

Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/fab/fab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
updateHostInteraction
} from "../../utils/interactive";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -123,7 +123,7 @@ export class Fab implements InteractiveComponent, LoadableComponent {
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

focusElement(this.buttonEl);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/calcite-components/src/components/filter/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
updateHostInteraction
} from "../../utils/interactive";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -208,7 +208,7 @@ export class Filter
/** Sets focus on the component. */
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

this.el?.focus();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
updateHostInteraction
} from "../../utils/interactive";
import {
componentLoaded,
componentFocusable,
LoadableComponent,
setComponentLoaded,
setUpLoadableComponent
Expand Down Expand Up @@ -213,7 +213,7 @@ export class FlowItem
*/
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);
await componentFocusable(this);

const { backButtonEl, containerEl } = this;

Expand Down
Loading

0 comments on commit ad9fbca

Please sign in to comment.