Skip to content

Conversation

rubencarvalho
Copy link
Contributor

@rubencarvalho rubencarvalho commented Oct 14, 2025

Description

Updated the type property in OverlayTrigger class to accept OverlayTypes instead of the more restrictive OverlayTriggerInteractions. This change expands the allowed values from 'modal' to all overlay types ('auto', 'hint', 'manual', 'modal', 'page') for the click content. Also updated the documentation to reflect this.

What I changed:

  • OverlayTrigger.ts: I updated the type property from OverlayTriggerInteractions to TriggerInteractions
  • overlay-types.ts: I added OVERLAY_TYPES const array for runtime use and to have a single source of truth, also to use in the...
  • ...tests!: Enhanced test coverage using imported OVERLAY_TYPES with proper TypeScript casting

Motivation and context

The previous definition was incorrect.

Related issue(s)

  • SWC-1282

Author's checklist

  • I have read the CONTRIBUTING and PULL_REQUESTS documents.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
  • I have added automated tests to cover my changes.
  • I have included a well-written changeset if my change needs to be published.
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • TypeScript compilation passes with expanded type values
  1. Create overlay-trigger with each type: 'auto', 'hint', 'manual', 'modal', 'page'
  2. Verify TypeScript compilation succeeds without type errors
  3. Expect no TypeScript warnings or errors in IDE

@changeset-bot
Copy link

changeset-bot bot commented Oct 14, 2025

🦋 Changeset detected

Latest commit: 85aaf9a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 84 packages
Name Type
@spectrum-web-components/overlay Patch
@spectrum-web-components/combobox Patch
@spectrum-web-components/contextual-help Patch
@spectrum-web-components/menu Patch
@spectrum-web-components/picker Patch
@spectrum-web-components/popover Patch
@spectrum-web-components/tooltip Patch
@spectrum-web-components/story-decorator Patch
@spectrum-web-components/bundle Patch
@spectrum-web-components/truncated Patch
@spectrum-web-components/breadcrumbs Patch
@spectrum-web-components/custom-vars-viewer Patch
example-project-rollup Patch
example-project-webpack Patch
@spectrum-web-components/action-menu Patch
@spectrum-web-components/action-bar Patch
@spectrum-web-components/card Patch
documentation Patch
@spectrum-web-components/vrt-compare Patch
@spectrum-web-components/eslint-plugin Patch
@spectrum-web-components/accordion Patch
@spectrum-web-components/action-button Patch
@spectrum-web-components/action-group Patch
@spectrum-web-components/alert-banner Patch
@spectrum-web-components/alert-dialog Patch
@spectrum-web-components/asset Patch
@spectrum-web-components/avatar Patch
@spectrum-web-components/badge Patch
@spectrum-web-components/button-group Patch
@spectrum-web-components/button Patch
@spectrum-web-components/checkbox Patch
@spectrum-web-components/clear-button Patch
@spectrum-web-components/close-button Patch
@spectrum-web-components/coachmark Patch
@spectrum-web-components/color-area Patch
@spectrum-web-components/color-field Patch
@spectrum-web-components/color-handle Patch
@spectrum-web-components/color-loupe Patch
@spectrum-web-components/color-slider Patch
@spectrum-web-components/color-wheel Patch
@spectrum-web-components/dialog Patch
@spectrum-web-components/divider Patch
@spectrum-web-components/dropzone Patch
@spectrum-web-components/field-group Patch
@spectrum-web-components/field-label Patch
@spectrum-web-components/help-text Patch
@spectrum-web-components/icon Patch
@spectrum-web-components/icons-ui Patch
@spectrum-web-components/icons-workflow Patch
@spectrum-web-components/icons Patch
@spectrum-web-components/iconset Patch
@spectrum-web-components/illustrated-message Patch
@spectrum-web-components/infield-button Patch
@spectrum-web-components/link Patch
@spectrum-web-components/meter Patch
@spectrum-web-components/modal Patch
@spectrum-web-components/number-field Patch
@spectrum-web-components/picker-button Patch
@spectrum-web-components/progress-bar Patch
@spectrum-web-components/progress-circle Patch
@spectrum-web-components/radio Patch
@spectrum-web-components/search Patch
@spectrum-web-components/sidenav Patch
@spectrum-web-components/slider Patch
@spectrum-web-components/split-view Patch
@spectrum-web-components/status-light Patch
@spectrum-web-components/swatch Patch
@spectrum-web-components/switch Patch
@spectrum-web-components/table Patch
@spectrum-web-components/tabs Patch
@spectrum-web-components/tags Patch
@spectrum-web-components/textfield Patch
@spectrum-web-components/thumbnail Patch
@spectrum-web-components/toast Patch
@spectrum-web-components/top-nav Patch
@spectrum-web-components/tray Patch
@spectrum-web-components/underlay Patch
@spectrum-web-components/base Patch
@spectrum-web-components/grid Patch
@spectrum-web-components/opacity-checkerboard Patch
@spectrum-web-components/reactive-controllers Patch
@spectrum-web-components/shared Patch
@spectrum-web-components/styles Patch
@spectrum-web-components/theme Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2025

📚 Branch Preview

🔍 Visual Regression Test Results

When 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: pr-5806

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 14, 2025

Tachometer results

Chrome

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 811 kB 403.97ms - 407.45ms - faster ✔
2% - 3%
8.82ms - 13.30ms
branch 799 kB 415.36ms - 418.18ms slower ❌
2% - 3%
8.82ms - 13.30ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 815 kB 25.29ms - 26.50ms - faster ✔
1% - 6%
0.20ms - 1.74ms
branch 803 kB 26.39ms - 27.33ms slower ❌
1% - 7%
0.20ms - 1.74ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 804 kB 338.37ms - 344.14ms - faster ✔
2% - 4%
5.92ms - 12.43ms
branch 792 kB 348.92ms - 351.94ms slower ❌
2% - 4%
5.92ms - 12.43ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 43.67ms - 45.20ms - faster ✔
1% - 5%
0.36ms - 2.51ms
branch 593 kB 45.11ms - 46.62ms slower ❌
1% - 6%
0.36ms - 2.51ms
-
Firefox

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 811 kB 689.23ms - 695.65ms - faster ✔
1% - 2%
4.15ms - 13.81ms
branch 799 kB 697.81ms - 705.03ms slower ❌
1% - 2%
4.15ms - 13.81ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 815 kB 48.21ms - 50.51ms - unsure 🔍
-5% - +1%
-2.32ms - +0.40ms
branch 803 kB 49.59ms - 51.05ms unsure 🔍
-1% - +5%
-0.40ms - +2.32ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 804 kB 543.11ms - 551.93ms - faster ✔
0% - 3%
1.73ms - 15.15ms
branch 792 kB 550.89ms - 561.03ms slower ❌
0% - 3%
1.73ms - 15.15ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 94.30ms - 97.94ms - unsure 🔍
-2% - +3%
-1.56ms - +3.08ms
branch 593 kB 93.92ms - 96.80ms unsure 🔍
-3% - +2%
-3.08ms - +1.56ms
-

@coveralls
Copy link
Collaborator

coveralls commented Oct 14, 2025

Pull Request Test Coverage Report for Build 18740821736

Details

  • 13 of 13 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.966%

Totals Coverage Status
Change from base Build 18730825611: 0.0%
Covered Lines: 34259
Relevant Lines: 34788

💛 - Coveralls

@rubencarvalho rubencarvalho marked this pull request as ready for review October 14, 2025 12:36
@rubencarvalho rubencarvalho requested a review from a team as a code owner October 14, 2025 12:36
this.innerTrigger = this.testDiv.querySelector(
'#inner-trigger'
) as OverlayTrigger;
)! as OverlayTrigger;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By casting these, it will offer us correct Intellisense.

'inline' | 'modal' | 'replace'
>;
await elementUpdated(this.outerTrigger);
const outerTrigger = this.outerTrigger as OverlayTrigger;
Copy link
Contributor Author

@rubencarvalho rubencarvalho Oct 15, 2025

Choose a reason for hiding this comment

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

Casting this means that if we ever pass a type that’s not valid, the next line will throw TS error and test will fail.

Copy link
Contributor

@TarunAdobe TarunAdobe left a comment

Choose a reason for hiding this comment

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

LGTM!

@rubencarvalho rubencarvalho added Component: Overlay Status: Ready for review PR ready for review or re-review. labels Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Overlay Status: Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants