fix(OverlayTrigger): conditionally attach slotchange listener

[jcmitch]

Conditionally attaching slot change listener in OverlayTrigger to avoid issues where it is possible to get caught in an endless render loop.

Description

Conditionally attach slot change listeners only if the slot element is going to be populated.

Related issue(s)

[Bug]: OverlayTrigger can get stuck in a render loop causing page crash

Motivation and context

Resolves an issue related to endless render loop.

How has this been tested?

Tested on desktop in a standalone simple app using SWC OverlayTrigger.

• [X] Did it pass in Desktop?
• [ ] Did it pass in Mobile?
• [ ] Did it pass in iPad?

Screenshots (if appropriate)

N/A

Types of changes

• [X] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)
• [ ] Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

Checklist

• [X] I have signed the Adobe Open Source CLA.
• [X] My code follows the code style of this project.
• [X] If my change required a change to the documentation, I have updated the documentation in this pull request.
• [X] I have read the CONTRIBUTING document.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.
• [X] I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

Best practices

This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against main.

[github-actions[bot]]

Branch preview

• Documentation Site
• Storybook

Review the following VRT differences

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

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]]

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.99	0.99	0.99
Accessibility	1	1	1
Best Practices	1	1	1
SEO	1	0.92	0.92
PWA	1	1	1

What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category	Latest	Main	Branch
Total	250.547 kB	236.502 kB 🏆	236.865 kB
Scripts	60.232 kB	54.06 kB 🏆	54.34 kB
Stylesheet	54.129 kB	47.911 kB 🏆	48.088 kB
Document	6.226 kB	5.474 kB	5.462 kB 🏆
Font	126.987 kB	126.604 kB 🏆	126.624 kB

Request Count

Category	Latest	Main	Branch
Total	52	52	52
Scripts	41	41	41
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

[github-actions[bot]]

Tachometer results

Chrome

action-bar permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	739 kB	54.22ms - 55.54ms	-	faster ✔ 3% - 7% 1.83ms - 4.38ms
branch	716 kB	56.89ms - 59.08ms	slower ❌ 3% - 8% 1.83ms - 4.38ms	-

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	959 kB	140.37ms - 143.29ms	-	faster ✔ 4% - 6% 5.44ms - 9.54ms
branch	918 kB	147.88ms - 150.76ms	slower ❌ 4% - 7% 5.44ms - 9.54ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	917 kB	70.72ms - 71.90ms	-	faster ✔ 4% - 6% 3.01ms - 4.72ms
branch	875 kB	74.55ms - 75.80ms	slower ❌ 4% - 7% 3.01ms - 4.72ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	916 kB	69.12ms - 70.16ms	-	faster ✔ 5% - 7% 3.36ms - 4.86ms
branch	874 kB	73.22ms - 74.28ms	slower ❌ 5% - 7% 3.36ms - 4.86ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.09 MB	1873.99ms - 1876.69ms	-	unsure 🔍 -0% - +0% -2.83ms - +0.99ms
branch	1.05 MB	1874.91ms - 1877.61ms	unsure 🔍 -0% - +0% -0.99ms - +2.83ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.09 MB	1877.36ms - 1879.78ms	-	unsure 🔍 -0% - -0% -4.36ms - -0.18ms
branch	1.05 MB	1879.14ms - 1882.54ms	unsure 🔍 +0% - +0% +0.18ms - +4.36ms	-

breadcrumbs permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	978 kB	516.54ms - 522.09ms	-	faster ✔ 2% - 4% 11.13ms - 21.19ms
branch	937 kB	531.28ms - 539.67ms	slower ❌ 2% - 4% 11.13ms - 21.19ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1 MB	42.03ms - 42.48ms	-	faster ✔ 1% - 3% 0.54ms - 1.17ms
branch	959 kB	42.89ms - 43.33ms	slower ❌ 1% - 3% 0.54ms - 1.17ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1 MB	390.43ms - 394.73ms	-	faster ✔ 1% - 3% 5.18ms - 12.16ms
branch	959 kB	398.49ms - 404.00ms	slower ❌ 1% - 3% 5.18ms - 12.16ms	-

contextual-help permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	945 kB	53.32ms - 54.44ms	-	faster ✔ 6% - 9% 3.37ms - 5.20ms
branch	899 kB	57.43ms - 58.88ms	slower ❌ 6% - 10% 3.37ms - 5.20ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	741 kB	206.61ms - 209.07ms	-	faster ✔ 2% - 4% 5.00ms - 8.57ms
branch	718 kB	213.34ms - 215.92ms	slower ❌ 2% - 4% 5.00ms - 8.57ms	-

overlay permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.06 MB	431.85ms - 435.23ms	-	slower ❌ 0% - 1% 1.28ms - 5.51ms
branch	940 kB	428.88ms - 431.41ms	faster ✔ 0% - 1% 1.28ms - 5.51ms	-

directive-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.07 MB	26.06ms - 26.59ms	-	faster ✔ 4% - 6% 1.08ms - 1.81ms
branch	1.02 MB	27.52ms - 28.02ms	slower ❌ 4% - 7% 1.08ms - 1.81ms	-

element-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.05 MB	353.53ms - 356.79ms	-	faster ✔ 2% - 3% 8.38ms - 12.59ms
branch	1.01 MB	364.32ms - 366.97ms	slower ❌ 2% - 4% 8.38ms - 12.59ms	-

lazy-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	852 kB	45.02ms - 45.78ms	-	faster ✔ 5% - 8% 2.63ms - 3.95ms
branch	806 kB	48.15ms - 49.23ms	slower ❌ 6% - 9% 2.63ms - 3.95ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	817 kB	501.10ms - 508.52ms	-	faster ✔ 3% - 5% 14.45ms - 24.63ms
branch	776 kB	520.87ms - 527.84ms	slower ❌ 3% - 5% 14.45ms - 24.63ms	-

popover permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	817 kB	107.64ms - 108.23ms	-	faster ✔ 2% - 3% 1.97ms - 3.37ms
branch	775 kB	109.97ms - 111.24ms	slower ❌ 2% - 3% 1.97ms - 3.37ms	-

tooltip permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	873 kB	35.43ms - 35.92ms	-	faster ✔ 3% - 5% 1.27ms - 2.01ms
branch	832 kB	37.04ms - 37.60ms	slower ❌ 4% - 6% 1.27ms - 2.01ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	831 kB	25.29ms - 25.85ms	-	faster ✔ 7% - 10% 2.01ms - 2.83ms
branch	789 kB	27.69ms - 28.29ms	slower ❌ 8% - 11% 2.01ms - 2.83ms	-

test-element permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	957 kB	53.76ms - 54.93ms	-	faster ✔ 5% - 7% 2.77ms - 4.32ms
branch	912 kB	57.39ms - 58.39ms	slower ❌ 5% - 8% 2.77ms - 4.32ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	933 kB	44.06ms - 44.92ms	-	faster ✔ 6% - 9% 2.90ms - 4.30ms
branch	887 kB	47.54ms - 48.64ms	slower ❌ 6% - 10% 2.90ms - 4.30ms	-

truncated permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	789 kB	62.54ms - 63.41ms	-	faster ✔ 3% - 5% 2.04ms - 3.38ms
branch	763 kB	65.17ms - 66.20ms	slower ❌ 3% - 5% 2.04ms - 3.38ms	-

Firefox

action-bar permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	739 kB	121.78ms - 129.46ms	-	unsure 🔍 -8% - +1% -10.66ms - +1.34ms
branch	716 kB	125.67ms - 134.89ms	unsure 🔍 -1% - +9% -1.34ms - +10.66ms	-

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	959 kB	292.58ms - 295.38ms	-	faster ✔ 16% - 17% 54.39ms - 60.93ms
branch	918 kB	348.68ms - 354.60ms	slower ❌ 18% - 21% 54.39ms - 60.93ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	917 kB	160.90ms - 165.14ms	-	slower ❌ 1% - 5% 2.14ms - 8.26ms
branch	875 kB	155.62ms - 160.02ms	faster ✔ 1% - 5% 2.14ms - 8.26ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	916 kB	142.60ms - 150.76ms	-	unsure 🔍 -5% - +1% -7.76ms - +1.76ms
branch	874 kB	147.22ms - 152.14ms	unsure 🔍 -1% - +5% -1.76ms - +7.76ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.09 MB	1898.45ms - 1903.67ms	-	faster ✔ 0% - 1% 3.04ms - 12.20ms
branch	1.05 MB	1904.92ms - 1912.44ms	slower ❌ 0% - 1% 3.04ms - 12.20ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.09 MB	1900.81ms - 1905.23ms	-	faster ✔ 1% - 1% 13.42ms - 23.38ms
branch	1.05 MB	1916.96ms - 1925.88ms	slower ❌ 1% - 1% 13.42ms - 23.38ms	-

breadcrumbs permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	978 kB	862.57ms - 872.63ms	-	faster ✔ 1% - 4% 11.79ms - 40.05ms
branch	937 kB	880.31ms - 906.73ms	slower ❌ 1% - 5% 11.79ms - 40.05ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1 MB	66.63ms - 71.49ms	-	unsure 🔍 -4% - +3% -2.79ms - +2.39ms
branch	959 kB	68.38ms - 70.14ms	unsure 🔍 -3% - +4% -2.39ms - +2.79ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1 MB	768.79ms - 796.49ms	-	slower ❌ 5% - 9% 35.67ms - 67.29ms
branch	959 kB	723.54ms - 738.78ms	faster ✔ 5% - 8% 35.67ms - 67.29ms	-

contextual-help permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	945 kB	115.41ms - 120.23ms	-	faster ✔ 1% - 6% 0.83ms - 7.81ms
branch	899 kB	119.61ms - 124.67ms	slower ❌ 1% - 7% 0.83ms - 7.81ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	741 kB	409.14ms - 423.18ms	-	faster ✔ 1% - 6% 4.93ms - 26.03ms
branch	718 kB	423.76ms - 439.52ms	slower ❌ 1% - 6% 4.93ms - 26.03ms	-

overlay permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.06 MB	709.14ms - 721.14ms	-	slower ❌ 13% - 16% 81.77ms - 97.07ms
branch	1.02 MB	620.97ms - 630.47ms	faster ✔ 12% - 13% 81.77ms - 97.07ms	-

directive-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.07 MB	52.99ms - 54.37ms	-	faster ✔ 2% - 5% 1.12ms - 2.76ms
branch	1.02 MB	55.17ms - 56.07ms	slower ❌ 2% - 5% 1.12ms - 2.76ms	-

element-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	1.05 MB	710.57ms - 723.95ms	-	slower ❌ 6% - 9% 38.19ms - 61.21ms
branch	1.01 MB	658.20ms - 676.92ms	faster ✔ 5% - 8% 38.19ms - 61.21ms	-

lazy-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	852 kB	94.35ms - 97.33ms	-	faster ✔ 5% - 10% 4.84ms - 10.16ms
branch	806 kB	101.14ms - 105.54ms	slower ❌ 5% - 11% 4.84ms - 10.16ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	817 kB	1033.53ms - 1044.99ms	-	faster ✔ 4% - 7% 45.66ms - 74.38ms
branch	776 kB	1086.12ms - 1112.44ms	slower ❌ 4% - 7% 45.66ms - 74.38ms	-

popover permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	817 kB	152.44ms - 159.68ms	-	unsure 🔍 -6% - +1% -9.71ms - +0.99ms
branch	775 kB	156.48ms - 164.36ms	unsure 🔍 -1% - +6% -0.99ms - +9.71ms	-

tooltip permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	957 kB	89.57ms - 96.47ms	-	slower ❌ 13% - 23% 10.32ms - 17.88ms
branch	912 kB	77.38ms - 80.46ms	faster ✔ 12% - 19% 10.32ms - 17.88ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	831 kB	51.80ms - 52.56ms	-	faster ✔ 1% - 3% 0.66ms - 1.62ms
branch	789 kB	53.02ms - 53.62ms	slower ❌ 1% - 3% 0.66ms - 1.62ms	-

test-element permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	957 kB	154.81ms - 160.11ms	-	slower ❌ 14% - 20% 19.37ms - 26.07ms
branch	912 kB	132.70ms - 136.78ms	faster ✔ 12% - 16% 19.37ms - 26.07ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	933 kB	90.25ms - 92.95ms	-	faster ✔ 27% - 32% 34.61ms - 43.51ms
branch	887 kB	126.42ms - 134.90ms	slower ❌ 38% - 48% 34.61ms - 43.51ms	-

truncated permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	789 kB	117.33ms - 123.27ms	-	faster ✔ 2% - 9% 2.61ms - 12.31ms
branch	763 kB	123.93ms - 131.59ms	slower ❌ 2% - 10% 2.61ms - 12.31ms	-

[Rajdeepc]

@jcmitch Thanks for doing this! Can you please help us with some corner case test cases to help in testing this issue which you faced in your environment. We want to capture all these corner cases and run these in isolated environments to reduce such issues going forward.

[coveralls]

Pull Request Test Coverage Report for Build 12660154690

Details

• 20 of 20 (100.0%) changed or added relevant lines in 1 file are covered.
• 278 unchanged lines in 8 files lost coverage.
• Overall coverage decreased (-0.8%) to 97.416%

---

Files with Coverage Reduction	New Missed Lines	%
packages/overlay/src/OverlayPopover.ts	2	84.81%
packages/overlay/src/OverlayDialog.ts	5	87.05%
packages/overlay/src/Overlay.ts	8	97.27%
packages/overlay/src/OverlayStack.ts	14	85.87%
packages/dialog/src/DialogBase.ts	16	87.02%
packages/overlay/src/HoverController.ts	24	87.56%
packages/overlay/src/OverlayTrigger.ts	50	79.43%
packages/overlay/src/LongpressController.ts	159	22.64%
<!--	Total:	278

Totals
Change from base Build 12658723443:	-0.8%
Covered Lines:	32743
Relevant Lines:	33445

---

💛 - Coveralls

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 168797a233586ecb10248cc2016bf728868266ed

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

[caseyisonit]

@jcmitch @Rajdeepc @rubencarvalho was this PR issue resolved in the merged PR #4689 and can thus be closed? Or does this PR need review still?

[rubencarvalho]

This got solved by https://github.com/adobe/spectrum-web-components/pull/5046, issue https://github.com/adobe/spectrum-web-components/issues/4689 is closed.