From 59006b389d146c0c22b61066357bfa11aa4c6986 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Mon, 11 Dec 2023 18:07:37 -0800 Subject: [PATCH 1/3] refactor: use Stencil watchers instead of global attributes util --- .../calcite-components/conventions/README.md | 40 +------ .../src/components/button/button.tsx | 38 ++++--- .../src/components/menu/menu.tsx | 33 +++--- .../src/utils/globalAttributes.spec.ts | 97 ---------------- .../src/utils/globalAttributes.ts | 104 ------------------ 5 files changed, 41 insertions(+), 271 deletions(-) delete mode 100644 packages/calcite-components/src/utils/globalAttributes.spec.ts delete mode 100644 packages/calcite-components/src/utils/globalAttributes.ts diff --git a/packages/calcite-components/conventions/README.md b/packages/calcite-components/conventions/README.md index 425ed2ef917..eaf4fab13f9 100644 --- a/packages/calcite-components/conventions/README.md +++ b/packages/calcite-components/conventions/README.md @@ -366,45 +366,7 @@ There are utilities for common workflows in [`src/utils`](../src/utils). ### Global attributes -The [`globalAttributes`](../src/utils/globalAttributes.ts) util was specifically made to access the `lang` global attribute when set on a Calcite component. However, it can be extended to allow additional [global attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes#list_of_global_attributes) by adding to the [`allowedGlobalAttributes`](https://github.com/Esri/calcite-design-system/blob/a33aa0df0c5bf103f91187826e6b12b8ff266d90/src/utils/globalAttributes.ts#L4-L5) array. The util is used in [`calcite-pagination`](../src/components/pagination/pagination.tsx), which you can use as a reference. - -#### Usage steps - -1. Import the interface and watch/unwatch methods - - ```js - import { GlobalAttrComponent, watchGlobalAttributes, unwatchGlobalAttributes } from "../../utils/globalAttributes"; - ``` - -2. Implement the interface - - ```js - export class ComponentName implements GlobalAttrComponent { - ``` - -3. Add `globalAttributes` state - - ```js - @State() globalAttributes = {}; - ``` - -4. Add connect/disconnect callbacks - - ```js - connectedCallback(): void { - watchGlobalAttributes(this, ["lang"]); - } - - disconnectedCallback(): void { - unwatchGlobalAttributes(this); - } - ``` - -5. Use the state to access `lang` (or another global attribute that may be allowed in the future). - - ```js - const lang = this.globalAttributes["lang"] || document.documentElement.lang || "en"; - ``` +Watching global attributes on components is now possible with Stencil v4. Please refer to the [documentation page](https://stenciljs.com/docs/reactive-data#watching-native-html-attributes) for more information. ### BigDecimal diff --git a/packages/calcite-components/src/components/button/button.tsx b/packages/calcite-components/src/components/button/button.tsx index 8af673deaf5..f9de70225ec 100644 --- a/packages/calcite-components/src/components/button/button.tsx +++ b/packages/calcite-components/src/components/button/button.tsx @@ -1,4 +1,15 @@ -import { Build, Component, Element, h, Method, Prop, State, VNode, Watch } from "@stencil/core"; +import { + Build, + Component, + Element, + forceUpdate, + h, + Method, + Prop, + State, + VNode, + Watch, +} from "@stencil/core"; import { findAssociatedForm, FormOwner, resetForm, submitForm } from "../../utils/form"; import { connectInteractive, @@ -27,11 +38,6 @@ import { Appearance, FlipContext, Kind, Scale, Width } from "../interfaces"; import { ButtonMessages } from "./assets/button/t9n"; import { ButtonAlignment } from "./interfaces"; import { CSS } from "./resources"; -import { - GlobalAttrComponent, - unwatchGlobalAttributes, - watchGlobalAttributes, -} from "../../utils/globalAttributes"; import { toAriaBoolean } from "../../utils/dom"; /** Passing a 'href' will render an anchor link, instead of a button. Role will be set to link, or button, depending on this. */ @@ -46,7 +52,6 @@ import { toAriaBoolean } from "../../utils/dom"; }) export class Button implements - GlobalAttrComponent, LabelableComponent, InteractiveComponent, FormOwner, @@ -56,6 +61,17 @@ export class Button { //-------------------------------------------------------------------------- // + // Global attributes + // + //-------------------------------------------------------------------------- + + @Watch("aria-expanded") + handleGlobalAttributesChanged(): void { + forceUpdate(this); + } + + // -------------------------------------------------------------------------- + // // Properties // //-------------------------------------------------------------------------- @@ -183,7 +199,6 @@ export class Button connectInteractive(this); connectLocalized(this); connectMessages(this); - watchGlobalAttributes(this, ["aria-expanded"]); this.hasLoader = this.loading; this.setupTextContentObserver(); connectLabel(this); @@ -198,7 +213,6 @@ export class Button disconnectMessages(this); this.resizeObserver?.disconnect(); this.formEl = null; - unwatchGlobalAttributes(this); } async componentWillLoad(): Promise { @@ -260,6 +274,7 @@ export class Button return ( @@ -357,10 +371,6 @@ export class Button resizeObserver = createObserver("resize", () => this.setTooltipText()); - @State() globalAttributes = { - ariaExpanded: undefined, - }; - //-------------------------------------------------------------------------- // // Private Methods diff --git a/packages/calcite-components/src/components/menu/menu.tsx b/packages/calcite-components/src/components/menu/menu.tsx index 42d212c7440..7ef01878dcf 100644 --- a/packages/calcite-components/src/components/menu/menu.tsx +++ b/packages/calcite-components/src/components/menu/menu.tsx @@ -9,6 +9,7 @@ import { Watch, Method, VNode, + forceUpdate, } from "@stencil/core"; import { focusElement, focusElementInGroup, slotChangeGetAssignedElements } from "../../utils/dom"; import { @@ -26,11 +27,6 @@ import { updateMessages, } from "../../utils/t9n"; import { MenuMessages } from "./assets/menu/t9n"; -import { - GlobalAttrComponent, - unwatchGlobalAttributes, - watchGlobalAttributes, -} from "../../utils/globalAttributes"; type Layout = "horizontal" | "vertical"; @@ -42,12 +38,21 @@ type Layout = "horizontal" | "vertical"; }, assetsDirs: ["assets"], }) -export class CalciteMenu - implements GlobalAttrComponent, LocalizedComponent, T9nComponent, LoadableComponent -{ +export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableComponent { + //-------------------------------------------------------------------------- + // + // Global attributes + // + //-------------------------------------------------------------------------- + + @Watch("role") + handleGlobalAttributesChanged(): void { + forceUpdate(this); + } + //-------------------------------------------------------------------------- // - // Public Properties + // Properties // //-------------------------------------------------------------------------- @@ -102,10 +107,6 @@ export class CalciteMenu updateMessages(this, this.effectiveLocale); } - @State() globalAttributes = { - role: "menubar", - }; - menuItems: HTMLCalciteMenuItemElement[] = []; //-------------------------------------------------------------------------- @@ -117,7 +118,6 @@ export class CalciteMenu connectedCallback(): void { connectLocalized(this); connectMessages(this); - watchGlobalAttributes(this, ["role"]); } async componentWillLoad(): Promise { @@ -132,7 +132,6 @@ export class CalciteMenu disconnectedCallback(): void { disconnectLocalized(this); disconnectMessages(this); - unwatchGlobalAttributes(this); } //-------------------------------------------------------------------------- @@ -220,7 +219,7 @@ export class CalciteMenu setMenuItemLayout(items: HTMLCalciteMenuItemElement[], layout: Layout): void { items.forEach((item) => { item.layout = layout; - if (this.globalAttributes.role === "menubar") { + if (this.el.getAttribute("role") === "menubar") { item.isTopLevelItem = true; item.topLevelMenuLayout = this.layout; } @@ -236,7 +235,7 @@ export class CalciteMenu render(): VNode { return ( -
    +
    diff --git a/packages/calcite-components/src/utils/globalAttributes.spec.ts b/packages/calcite-components/src/utils/globalAttributes.spec.ts deleted file mode 100644 index 56c5b87b23e..00000000000 --- a/packages/calcite-components/src/utils/globalAttributes.spec.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { Build } from "@stencil/core"; -import { JSDOM } from "jsdom"; -import { waitForAnimationFrame } from "../tests/utils"; -import { GlobalAttrComponent, unwatchGlobalAttributes, watchGlobalAttributes } from "./globalAttributes"; - -describe("globalAttributes", () => { - const originalIsBrowser = Build.isBrowser; - - beforeAll(() => (Build.isBrowser = true)); - afterAll(() => (Build.isBrowser = originalIsBrowser)); - - it("sets component's global attributes when watching", async () => { - // we clobber Stencil's custom Mock document implementation - const { window: win } = new JSDOM(); - - // make window references use JSDOM (which is a subset, hence the type cast) - window = win as any as Window & typeof globalThis; - const fakeComponent = win.document.createElement("fake-component"); - win.document.body.append(fakeComponent); - - const globalComponent: GlobalAttrComponent = { - el: fakeComponent, - globalAttributes: {}, - }; - - watchGlobalAttributes(globalComponent, ["lang"]); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({}); - - fakeComponent.setAttribute("lang", "en"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "en" }); - - fakeComponent.setAttribute("title", "untracked global attr should not be set"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "en" }); - - fakeComponent.setAttribute("lang", "es"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "es" }); - - unwatchGlobalAttributes(globalComponent); - - fakeComponent.setAttribute("lang", "fr"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "es" }); - }); - - it("applies globalAttribute defaults (if specified) on initial update", async () => { - // we clobber Stencil's custom Mock document implementation - const { window: win } = new JSDOM(); - - // make window references use JSDOM (which is a subset, hence the type cast) - window = win as any as Window & typeof globalThis; - const fakeComponent = win.document.createElement("fake-component"); - win.document.body.append(fakeComponent); - - const globalComponent: GlobalAttrComponent = { - el: fakeComponent, - globalAttributes: { - lang: "test", - }, - }; - - watchGlobalAttributes(globalComponent, ["lang"]); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "test" }); - - fakeComponent.setAttribute("lang", "en"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "en" }); - - fakeComponent.setAttribute("title", "untracked global attr should not be set"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "en" }); - - fakeComponent.setAttribute("lang", "es"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "es" }); - - unwatchGlobalAttributes(globalComponent); - - fakeComponent.setAttribute("lang", "fr"); - await waitForAnimationFrame(); - - expect(globalComponent.globalAttributes).toEqual({ lang: "es" }); - }); -}); diff --git a/packages/calcite-components/src/utils/globalAttributes.ts b/packages/calcite-components/src/utils/globalAttributes.ts deleted file mode 100644 index 05f7880eec4..00000000000 --- a/packages/calcite-components/src/utils/globalAttributes.ts +++ /dev/null @@ -1,104 +0,0 @@ -import { createObserver } from "./observers"; - -type AttributeObject = { [k: string]: any }; -type AllowedGlobalAttribute = "lang" | "role" | "aria-expanded"; -const allowedGlobalAttributes = ["lang", "role", "aria-expanded"]; - -const elementToComponentAndObserverOptionsMap = new Map< - HTMLElement, - [GlobalAttrComponent, { attributeFilter: AllowedGlobalAttribute[] }] ->(); - -let mutationObserver: MutationObserver; - -function updateGlobalAttributes( - component: GlobalAttrComponent, - attributeFilter: AllowedGlobalAttribute[], - reuseAttributes = false -): void { - const { el } = component; - const updatedAttributes: AttributeObject = reuseAttributes ? component.globalAttributes : {}; - - attributeFilter - .filter((attr) => !!allowedGlobalAttributes.includes(attr) && !!el.hasAttribute(attr)) - .forEach((attr) => { - const value = el.getAttribute(attr); - - if (value !== null) { - updatedAttributes[attr] = value; - } - }); - - component.globalAttributes = updatedAttributes; -} - -function processMutations(mutations: MutationRecord[]): void { - mutations.forEach(({ target }) => { - const [component, options] = elementToComponentAndObserverOptionsMap.get(target as HTMLElement); - updateGlobalAttributes(component, options.attributeFilter); - }); -} - -/** - * Watches global attributes of a component. - * - * Derived from: https://gist.github.com/willmartian/b4dd6b57d71dd0438fb9e7c6f4048578 - */ -export interface GlobalAttrComponent { - /** - * The host element. - */ - readonly el: HTMLElement; - - /** - * The object that stores global attributes to apply internally during rendering. - * - * This prop should use the `@State` decorator. - */ - globalAttributes: AttributeObject; -} - -/** - * Helper to set up listening for changes to global attributes. - * - * render(): VNode { - * return ( - *
      - * ); - * } - * - * @param component - * @param attributeFilter - */ -export function watchGlobalAttributes(component: GlobalAttrComponent, attributeFilter: AllowedGlobalAttribute[]): void { - const { el } = component; - const observerOptions = { attributeFilter }; - - elementToComponentAndObserverOptionsMap.set(el, [component, observerOptions]); - - updateGlobalAttributes(component, attributeFilter, true); - - if (!mutationObserver) { - mutationObserver = createObserver("mutation", processMutations); - } - - mutationObserver.observe(el, observerOptions); -} - -/** - * Helper remove listening for changes to inherited attributes. - * - * @param component - */ -export function unwatchGlobalAttributes(component: GlobalAttrComponent): void { - elementToComponentAndObserverOptionsMap.delete(component.el); - - // we explicitly process queued mutations and disconnect and reconnect - // the observer until MutationObserver gets an `unobserve` method - // see https://github.com/whatwg/dom/issues/126 - processMutations(mutationObserver.takeRecords()); - mutationObserver.disconnect(); - for (const [element, [, observerOptions]] of elementToComponentAndObserverOptionsMap.entries()) { - mutationObserver.observe(element, observerOptions); - } -} From eb60a70681fa091d33878a993b0c64b1d169636d Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 12 Dec 2023 10:20:03 -0800 Subject: [PATCH 2/3] fix menu test --- .../calcite-components/src/components/menu/menu.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/calcite-components/src/components/menu/menu.tsx b/packages/calcite-components/src/components/menu/menu.tsx index 7ef01878dcf..9203724de39 100644 --- a/packages/calcite-components/src/components/menu/menu.tsx +++ b/packages/calcite-components/src/components/menu/menu.tsx @@ -9,7 +9,6 @@ import { Watch, Method, VNode, - forceUpdate, } from "@stencil/core"; import { focusElement, focusElementInGroup, slotChangeGetAssignedElements } from "../../utils/dom"; import { @@ -46,8 +45,8 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo //-------------------------------------------------------------------------- @Watch("role") - handleGlobalAttributesChanged(): void { - forceUpdate(this); + handleGlobalAttributesChanged(value: string): void { + this.menuParentRole = value; } //-------------------------------------------------------------------------- @@ -98,6 +97,8 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo @Element() el: HTMLCalciteMenuElement; + @State() private menuParentRole = "menubar"; + @State() defaultMessages: MenuMessages; @State() effectiveLocale = ""; @@ -219,7 +220,7 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo setMenuItemLayout(items: HTMLCalciteMenuItemElement[], layout: Layout): void { items.forEach((item) => { item.layout = layout; - if (this.el.getAttribute("role") === "menubar") { + if (this.menuParentRole === "menubar") { item.isTopLevelItem = true; item.topLevelMenuLayout = this.layout; } @@ -235,7 +236,7 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo render(): VNode { return ( -
        +
        From 130e9f00563903fb8f3fc43d1196f13e4200c276 Mon Sep 17 00:00:00 2001 From: JC Franco Date: Tue, 12 Dec 2023 16:34:25 -0800 Subject: [PATCH 3/3] fix menu layout regression --- .../src/components/menu/menu.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/calcite-components/src/components/menu/menu.tsx b/packages/calcite-components/src/components/menu/menu.tsx index 9203724de39..9136fc47e7c 100644 --- a/packages/calcite-components/src/components/menu/menu.tsx +++ b/packages/calcite-components/src/components/menu/menu.tsx @@ -9,6 +9,7 @@ import { Watch, Method, VNode, + forceUpdate, } from "@stencil/core"; import { focusElement, focusElementInGroup, slotChangeGetAssignedElements } from "../../utils/dom"; import { @@ -45,8 +46,9 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo //-------------------------------------------------------------------------- @Watch("role") - handleGlobalAttributesChanged(value: string): void { - this.menuParentRole = value; + handleGlobalAttributesChanged(): void { + forceUpdate(this); + this.setMenuItemLayout(this.menuItems, this.layout); } //-------------------------------------------------------------------------- @@ -97,8 +99,6 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo @Element() el: HTMLCalciteMenuElement; - @State() private menuParentRole = "menubar"; - @State() defaultMessages: MenuMessages; @State() effectiveLocale = ""; @@ -220,13 +220,17 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo setMenuItemLayout(items: HTMLCalciteMenuItemElement[], layout: Layout): void { items.forEach((item) => { item.layout = layout; - if (this.menuParentRole === "menubar") { + if (this.getEffectiveRole() === "menubar") { item.isTopLevelItem = true; item.topLevelMenuLayout = this.layout; } }); } + private getEffectiveRole(): string { + return this.el.getAttribute("role") || "menubar"; + } + // -------------------------------------------------------------------------- // // Render Methods @@ -236,7 +240,7 @@ export class CalciteMenu implements LocalizedComponent, T9nComponent, LoadableCo render(): VNode { return ( -
          +