adobe
fix(overlay): default listenerHost to target in overlay-trigger-directive
Description
Guards OverlayTriggerDirective's reconnected() implementation so it only calls init() after an overlay instance exists. This prevents type errors during the reconnected flow when listenerHost would otherwise be unset because the overlay is not yet ready.
Motivation and context
When an overlay trigger directive’s reconnected hook runs with no listenerHost set in its overridden update(), we see type/runtime errors. Aligning with the parent directive and setting this.listenerHost = this.target prevents these errors and keeps typing consistent with expectations when slottable-request-directive executes. The parent directive already uses this pattern; this brings overlay-trigger-directive in line.
Related issue(s)
- fixes #5723
Manual Testing Steps
- [ ] Open this story
- [ ] Toggle overlay render button and notice there is no console dev errors/warnings
Author's checklist
- [x] I have read the CONTRIBUTING and PULL_REQUESTS documents.
- [x] I have reviewed at the Accessibility Practices for this feature, see: Aria Practices
- [x] I have added automated tests to cover my changes.
- [x] 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, ormajorfeatures - [ ] 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
🦋 Changeset detected
Latest commit: e83e1df905678dcceea9a7ec6de572ffbd35f80f
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 78 packages
| Name | Type |
|---|---|
| @spectrum-web-components/overlay | Patch |
| @spectrum-web-components/action-menu | 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/bundle | Patch |
| @spectrum-web-components/truncated | Patch |
| @spectrum-web-components/breadcrumbs | Patch |
| @spectrum-web-components/action-bar | Patch |
| @spectrum-web-components/card | Patch |
| @spectrum-web-components/coachmark | 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/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 |
| @spectrum-web-components/eslint-plugin | 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
![changeset-bot[bot] avatar](https://avatars.githubusercontent.com/in/28616?v=4 "Published by a pub.dev verified publisher"){kind=small}
📚 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:
- Spectrum | Light | Medium | LTR
- Spectrum | Dark | Large | RTL
- Express | Light | Medium | LTR
- Express | Dark | Large | RTL
- Spectrum-two | Light | Medium | LTR
- Spectrum-two | Dark | Large | RTL
- High Contrast Mode | Medium | LTR
Deployed to Azure Blob Storage: pr-5873
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[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@TarunAdobe can you include the manual testing steps in the PR description?
When testing overlay from this link: https://swcpreviews.z13.web.core.windows.net/pr-5873/docs/storybook/?path=/story/overlay-directive--default
The overlay does not open and im receiving this in the console:
📚 Branch Preview Links
🔍 First Generation 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:
- Spectrum | Light | Medium | LTR
- Spectrum | Dark | Large | RTL
- Express | Light | Medium | LTR
- Express | Dark | Large | RTL
- Spectrum-two | Light | Medium | LTR
- Spectrum-two | Dark | Large | RTL
- High Contrast Mode | Medium | LTR
Deployed to Azure Blob Storage: pr-5873
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.
When testing overlay from this link: https://swcpreviews.z13.web.core.windows.net/pr-5873/docs/storybook/?path=/story/overlay-directive--default
The overlay does not open and im receiving this in the console:
WE TOTALLY DIDN'T KNOW BT THIS IS A PROBLEM IN MAIN TOO!!
@caseyisonit the error you saw in the story would be fixed in a separate pr #5896
@TarunAdobe can you include the manual testing steps in the PR description?
I updated the Managed overlay trigger story implementation so that it now caches the overlay trigger instead of replacing it and thus you can verify this fix from that story (check description again).
@lehelen19 Could you help validate this in Hz before we move forward with the rollout? It would also be helpful if you could share your specific use case. We can then add it to our Storybook dev instance to reproduce the scenario and run additional tests on our side. This issue appears to be highly use-case specific, so any additional context will help us ensure full coverage.
The bug was triggering errors logged in Splunk, so I don't have a specific use case to share
The bug was triggering errors logged in Splunk, so I don't have a specific use case to share
Thanks for the context. @lehelen19 I want to do a soft test here before we roll out this fix. Can you confirm if this fix works for you or not?
Thanks for the context. @lehelen19 I want to do a soft test here before we roll out this fix. Can you confirm if this fix works for you or not?
I don't have a specific test case for this issue. You might be able to remove a component with the overlay directive and then reconnect it to see if you are encountering the error in the console?
We should be able to see the related Splunk error disappear once we get the fix in main