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

refactor!: enabled delegatesFocus #6056

Merged
merged 15 commits into from
Dec 15, 2022
Merged
Show file tree
Hide file tree
Changes from 11 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
3 changes: 1 addition & 2 deletions src/components/action-bar/action-bar.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,7 @@ describe("calcite-action-bar", () => {
</calcite-action-bar>
`,
{
focusId: "expand-toggle",
focusTargetSelector: "calcite-action-bar"
focusTargetSelector: "calcite-action"
}
));

Expand Down
15 changes: 5 additions & 10 deletions src/components/action-bar/action-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import { Position, Scale, Layout } from "../interfaces";
import { ExpandToggle, toggleChildActionText } from "../functional/ExpandToggle";
import { CSS, SLOTS } from "./resources";
import { getSlotted, focusElement } from "../../utils/dom";
import { getSlotted } from "../../utils/dom";
import {
geActionDimensions,
getOverflowCount,
Expand Down Expand Up @@ -53,7 +53,9 @@ import {
@Component({
tag: "calcite-action-bar",
styleUrl: "action-bar.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class ActionBar
Expand Down Expand Up @@ -227,18 +229,11 @@ export class ActionBar

/**
* Sets focus on the component.
*
* @param focusId
*/
@Method()
async setFocus(focusId?: "expand-toggle"): Promise<void> {
async setFocus(): Promise<void> {
await componentLoaded(this);

if (focusId === "expand-toggle") {
await focusElement(this.expandToggleEl);
return;
}

this.el?.focus();
}

Expand Down
4 changes: 3 additions & 1 deletion src/components/action-group/action-group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import { Messages } from "./assets/action-group/t9n";
@Component({
tag: "calcite-action-group",
styleUrl: "action-group.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class ActionGroup implements ConditionalSlotComponent, LocalizedComponent, T9nComponent {
Expand Down
3 changes: 1 addition & 2 deletions src/components/action-pad/action-pad.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ describe("calcite-action-pad", () => {
</calcite-action-pad>
`,
{
focusId: "expand-toggle",
focusTargetSelector: "calcite-action-pad"
focusTargetSelector: "calcite-action"
}
));

Expand Down
15 changes: 5 additions & 10 deletions src/components/action-pad/action-pad.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from "@stencil/core";
import { Layout, Position, Scale } from "../interfaces";
import { ExpandToggle, toggleChildActionText } from "../functional/ExpandToggle";
import { focusElement, getSlotted } from "../../utils/dom";
import { getSlotted } from "../../utils/dom";
import { CSS, SLOTS } from "./resources";
import {
ConditionalSlotComponent,
Expand Down Expand Up @@ -43,7 +43,9 @@ import {
@Component({
tag: "calcite-action-pad",
styleUrl: "action-pad.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class ActionPad
Expand Down Expand Up @@ -169,18 +171,11 @@ export class ActionPad

/**
* Sets focus on the component.
*
* @param focusId
*/
@Method()
async setFocus(focusId?: "expand-toggle"): Promise<void> {
async setFocus(): Promise<void> {
await componentLoaded(this);

if (focusId === "expand-toggle") {
await focusElement(this.expandToggleEl);
return;
}

this.el?.focus();
}

Expand Down
9 changes: 5 additions & 4 deletions src/components/color-picker/color-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
HSV_LIMITS,
RGB_LIMITS
} from "./resources";
import { Direction, focusElement, getElementDir, isPrimaryPointerButton } from "../../utils/dom";
import { Direction, getElementDir, isPrimaryPointerButton } from "../../utils/dom";
import { colorEqual, CSSColorMode, Format, normalizeHex, parseMode, SupportedMode } from "./utils";
import { throttle } from "lodash-es";

Expand Down Expand Up @@ -54,7 +54,9 @@ const defaultFormat = "auto";
@Component({
tag: "calcite-color-picker",
styleUrl: "color-picker.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class ColorPicker
Expand Down Expand Up @@ -641,8 +643,7 @@ export class ColorPicker
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);

return focusElement(this.colorFieldScopeNode);
this.el.focus();
}

//--------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion src/components/date-picker/date-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ import {
assetsDirs: ["assets"],
tag: "calcite-date-picker",
styleUrl: "date-picker.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class DatePicker implements LocalizedComponent, T9nComponent {
//--------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion src/components/dropdown-group/dropdown-group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import { CSS } from "./resources";
@Component({
tag: "calcite-dropdown-group",
styleUrl: "dropdown-group.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class DropdownGroup {
//--------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion src/components/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ import { isActivationKey } from "../../utils/key";
@Component({
tag: "calcite-dropdown",
styleUrl: "dropdown.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class Dropdown implements InteractiveComponent, OpenCloseComponent, FloatingUIComponent {
//--------------------------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions src/components/filter/filter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
import { debounce } from "lodash-es";
import { CSS, DEBOUNCE_TIMEOUT, ICONS } from "./resources";
import { Scale } from "../interfaces";
import { focusElement } from "../../utils/dom";
import { InteractiveComponent, updateHostInteraction } from "../../utils/interactive";
import { filter } from "../../utils/filter";
import { connectLocalized, disconnectLocalized, LocalizedComponent } from "../../utils/locale";
Expand All @@ -36,7 +35,9 @@ import {
@Component({
tag: "calcite-filter",
styleUrl: "filter.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class Filter
Expand Down Expand Up @@ -184,7 +185,7 @@ export class Filter
async setFocus(): Promise<void> {
await componentLoaded(this);

focusElement(this.textInput);
this.el?.focus();
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this no longer focus the textInput?

Copy link
Member Author

@benelan benelan Dec 15, 2022

Choose a reason for hiding this comment

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

It does focus the Input because it's the first focusable element. I double checked to be sure, putting this in the demos for this branch correctly focuses the Input:

  <calcite-filter></calcite-filter>
  <script>
    customElements.whenDefined("calcite-filter").then(() => 
        document.querySelector("calcite-filter").setFocus());
  </script>

If we call focus() on a component with delegatesFocus enabled does it automatically focus the first element?

It depends on whether everything is loaded. For example this works, but not without the timeout:

    customElements
      .whenDefined("calcite-filter")
      .then(() => setTimeout(() => document.querySelector("calcite-filter").focus(), 1000));

We still need the async setFocus() methods.

}

// --------------------------------------------------------------------------
Expand Down
10 changes: 4 additions & 6 deletions src/components/inline-editable/inline-editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ import {
*/
@Component({
tag: "calcite-inline-editable",
shadow: true,
shadow: {
delegatesFocus: true
},
styleUrl: "inline-editable.scss",
assetsDirs: ["assets"]
})
Expand Down Expand Up @@ -318,11 +320,7 @@ export class InlineEditable
async setFocus(): Promise<void> {
await componentLoaded(this);

if (this.editingEnabled) {
this.inputElement?.setFocus();
} else {
this.enableEditingButton?.setFocus();
}
this.el?.focus();
}

//--------------------------------------------------------------------------
Expand Down
7 changes: 4 additions & 3 deletions src/components/input-date-picker/input-date-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ import {
@Component({
tag: "calcite-input-date-picker",
styleUrl: "input-date-picker.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class InputDatePicker
implements
Expand Down Expand Up @@ -386,8 +388,7 @@ export class InputDatePicker
@Method()
async setFocus(): Promise<void> {
await componentLoaded(this);

this.startInput?.setFocus();
this.el.focus();
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/components/input-time-picker/input-time-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ import {
@Component({
tag: "calcite-input-time-picker",
styleUrl: "input-time-picker.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class InputTimePicker
implements
Expand Down Expand Up @@ -419,7 +421,7 @@ export class InputTimePicker
async setFocus(): Promise<void> {
await componentLoaded(this);

this.calciteInputEl?.setFocus();
this.el.focus();
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/components/pagination/pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ export interface PaginationDetail {
@Component({
tag: "calcite-pagination",
styleUrl: "pagination.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class Pagination implements LocalizedComponent, LocalizedComponent, T9nComponent {
Expand Down
4 changes: 3 additions & 1 deletion src/components/radio-button-group/radio-button-group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ import { Layout, Scale } from "../interfaces";
@Component({
tag: "calcite-radio-button-group",
styleUrl: "radio-button-group.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class RadioButtonGroup {
//--------------------------------------------------------------------------
Expand Down
4 changes: 3 additions & 1 deletion src/components/slider/slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ function isRange(value: number | number[]): value is number[] {
@Component({
tag: "calcite-slider",
styleUrl: "slider.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class Slider
implements
Expand Down
4 changes: 3 additions & 1 deletion src/components/split-button/split-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import { InteractiveComponent, updateHostInteraction } from "../../utils/interac
@Component({
tag: "calcite-split-button",
styleUrl: "split-button.scss",
shadow: true
shadow: {
delegatesFocus: true
}
})
export class SplitButton implements InteractiveComponent {
@Element() el: HTMLCalciteSplitButtonElement;
Expand Down
4 changes: 3 additions & 1 deletion src/components/time-picker/time-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ function capitalize(str: string): string {
@Component({
tag: "calcite-time-picker",
styleUrl: "time-picker.scss",
shadow: true,
shadow: {
delegatesFocus: true
},
assetsDirs: ["assets"]
})
export class TimePicker
Expand Down