feat(tooltip): tests and code conformance#6344
Conversation
5t3ph
added
Component:Tooltip
Status:Ready for review
|
📚 Branch Preview Links🔍 First Generation Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
caseyisonit
added
Status:Ready for merge
rubencarvalho
left a comment
rubencarvalho
left a comment
There was a problem hiding this comment.
Left some out of scope comments, but I stumbled upon them when checking the tests 😛
| if (!trigger) { | ||
| return; | ||
| } | ||
| const target = (trigger.shadowRoot?.querySelector('button') ?? |
There was a problem hiding this comment.
I've been thinking about this (also because of the popover), we had these "focusable-selectors" in Gen1 (1st-gen/tools/shared/src/focusable-selectors.ts), I'm wondering if we'll ever want to extend this.
| * When set, wires `ariaLabelledByElements` instead of `ariaDescribedByElements` on the trigger's | ||
| * inner interactive element. For icon-only triggers where the tooltip text is the sole accessible name. | ||
| * | ||
| * Additive/deferred: active in the additive phase. |
There was a problem hiding this comment.
what's exactly the plan for this property? This is going to surface on the API table but it does not seem like it has effect.
| this.open = isOpen; | ||
| } | ||
| // When no CSS transition is active, dispatch after-* immediately since transitionend will not fire. | ||
| if (getComputedStyle(this).transitionDuration === '0s') { |
There was a problem hiding this comment.
I think this will not work for all cases... Check here: https://codepen.io/rubencarvalho/pen/yyVpGez
when multiple transition-property values are declared, we get a comma-separated duration list ("0s, 0s, …"), which would never === '0s'
| } as Meta; | ||
|
|
||
| // Type alias for elements with ariaDescribedByElements (available in Chrome 135+, Firefox 136+, Safari 16.4+). | ||
| type AriaRelatable = Element & { ariaDescribedByElements: Element[] | null }; |
There was a problem hiding this comment.
nit: could we tighten these casts? maybe it even makes sense to augment the globals in in 2nd-gen/packages/core/global.d.ts if we need.
Something along the lines of:
/**
* ARIA element-reflection properties (the IDREF-free associations). Not yet in
* the TypeScript DOM lib. Available in Chrome 135+, Firefox 136+, Safari 16.4+.
*/
interface ARIAMixin {
ariaDescribedByElements: Element[] | null;
ariaLabelledByElements: Element[] | null;
}
I think ARIAMixin is the interface that Element/HTMLElement/HTMLButtonElement all implement, so this lands the property on every element type without more casting 😄
| const swcButton = canvasElement.querySelector( | ||
| '#tt-swc-trigger' | ||
| ) as HTMLElement; | ||
| await (swcButton as HTMLElement & { updateComplete: Promise<boolean> }) |
There was a problem hiding this comment.
We can cast this as an actuall Button clas which will bring updateComplete out of the box 😄
There was a problem hiding this comment.
Is this supposed to be included? I've been getting this file regenerating and getting stale every time I run Storybook locally, prob worth looking into this.
4 participants
Description
Adds Phase 6 (migration testing) and the post-Phase-6 code conformance pass for
swc-tooltip.Testing (SWC-2024)
Adds two test files:
tooltip.test.ts— Storybook play-function tests using Vitest and@storybook/test. Covers defaults, property/attribute mutation, open/close state, lifecycle events (swc-open,swc-close,swc-after-open,swc-after-close), ARIAariaDescribedByElementswiring for native triggers,swc-buttonshadow triggers,triggerElementoverrides, standalone (no trigger), and manual mode. Also covers Escape key dismissal, all variants, all placements, and the dev-mode warning for an unresolvedforattribute.tooltip.a11y.spec.ts— Playwright ARIA snapshot tests. Validates that a closed tooltip is hidden from the accessibility tree, that an open tooltip exposesrole="tooltip", that it is removed from the tree when closed, that Escape closes it, and that variant and placement trigger buttons are reachable.Conformance pass (SWC-2025)
Tooltip.ts— Added@firesJSDoc tags for all four lifecycle events.tooltip.css— Added explanatory comment for!importanton::slotted()resets.tooltip.stories.ts— Section separator renamed from "METADATA SETUP" to "METADATA"tooltip.test.ts— Restructured from ad-hoc section headers to the 5 prescribed standard sections; step labels updated to start with verbs;EscapeClosesTestmoved to the Variants / States section; section renamed from "Debug warnings" to "Dev mode warnings".Motivation and context
Phase 6 of the 2nd-gen tooltip migration. Tests validate the full public API surface defined in Phase 3 and the accessibility wiring implemented in Phase 4. The conformance pass ensures all files align with project style guides before Phase 7 (documentation) begins.
Related issue(s)
Manual review test cases
Play-function tests pass in the Storybook test runner
yarn test:storybook(or equivalent) scoped to the tooltip componentTooltip/Testsshow green in the test results panelPlaywright a11y tests pass
yarn test:a11yscoped to tooltiptooltip.a11y.spec.tspassConformance-only changes produce no behavioral regressions
Tooltip→Overviewin StorybookTooltip→VariantsandTooltip→Placements; confirm all variants and placements render correctly