Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(OverlayTrigger): delay update for overlay trigger to avoid infinite loop #5012

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(OverlayTrigger): delay update for overlay trigger to avoid infinite loop #5012

wants to merge 2 commits into from

Conversation

jcmitch
Copy link
Contributor

@jcmitch jcmitch commented Jan 9, 2025 •
edited
Loading

Description

Overlay Trigger has potential to get into an infinite loop on initial render. This PR adds a delay using requestAnimationFrame to avoid this issue. As discussed in person this does not solve the root cause but finding that will require a deeper dive by the core SWC team.

Related issue(s)

Motivation and context

Resolves an issue related to endless render loop.

How has this been tested?

Manual testing in Adobe Express

  • Did it pass in Desktop?
  • Did it pass in Mobile?
  • Did it pass in iPad?

Screenshots (if appropriate)

N/A

Types of changes

  • 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

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • 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.

@jcmitch jcmitch requested a review from a team as a code owner January 9, 2025 23:33
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jan 9, 2025

⚠️ No Changeset found

Latest commit: 19e4455

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2025

Branch preview

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:

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 GitHub Actions
Copy link

github-actions bot commented Jan 9, 2025

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.99 0.99 0.98
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 251.78 kB 237.03 kB 236.678 kB 🏆
Scripts 61.587 kB 54.392 kB 54.21 kB 🏆
Stylesheet 53.972 kB 48.198 kB 48.012 kB 🏆
Document 6.247 kB 5.46 kB 🏆 5.462 kB
Font 127.016 kB 126.632 kB 🏆 126.641 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

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 12700368298

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • 85 unchanged lines in 6 files lost coverage.
  • Overall coverage decreased (-0.2%) to 97.973%
Files with Coverage Reduction New Missed Lines %
packages/overlay/src/OverlayPopover.ts 2 84.95%
packages/overlay/src/OverlayTrigger.ts 3 98.39%
packages/overlay/src/Overlay.ts 7 97.57%
packages/dialog/src/DialogBase.ts 8 90.1%
packages/overlay/src/OverlayStack.ts 9 88.07%
packages/overlay/src/LongpressController.ts 56 72.41%
Totals Coverage Status
Change from base Build 12658723443: -0.2%
Covered Lines: 32927
Relevant Lines: 33436
💛 - Coveralls

@github-actions GitHub Actions
Copy link

Tachometer results

Chrome

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 739 kB 56.75ms - 57.70ms - faster ✔
4% - 6%
2.17ms - 3.84ms
branch 716 kB 59.55ms - 60.92ms slower ❌
4% - 7%
2.17ms - 3.84ms		 -

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 959 kB 148.24ms - 151.19ms - faster ✔
4% - 6%
6.27ms - 10.15ms
branch 918 kB 156.66ms - 159.18ms slower ❌
4% - 7%
6.27ms - 10.15ms		 -

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 917 kB 75.39ms - 76.53ms - faster ✔
4% - 6%
3.21ms - 4.98ms
branch 875 kB 79.38ms - 80.72ms slower ❌
4% - 7%
3.21ms - 4.98ms		 -

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 916 kB 73.73ms - 75.19ms - faster ✔
7% - 11%
6.05ms - 8.72ms
branch 874 kB 80.73ms - 82.96ms slower ❌
8% - 12%
6.05ms - 8.72ms		 -

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1877.39ms - 1880.42ms - unsure 🔍
-0% - +0%
-3.12ms - +1.21ms
branch 1.05 MB 1878.31ms - 1881.41ms unsure 🔍
-0% - +0%
-1.21ms - +3.12ms		 -

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1885.09ms - 1887.93ms - unsure 🔍
+0% - +0%
+0.69ms - +4.38ms
branch 1.05 MB 1882.79ms - 1885.15ms unsure 🔍
-0% - -0%
-4.38ms - -0.69ms		 -

breadcrumbs permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 978 kB 542.68ms - 547.65ms - faster ✔
2% - 4%
13.01ms - 20.09ms
branch 937 kB 559.19ms - 564.24ms slower ❌
2% - 4%
13.01ms - 20.09ms		 -

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 44.54ms - 45.15ms - faster ✔
2% - 4%
0.98ms - 1.83ms
branch 959 kB 45.96ms - 46.54ms slower ❌
2% - 4%
0.98ms - 1.83ms		 -

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 421.32ms - 429.25ms - faster ✔
1% - 4%
5.26ms - 16.40ms
branch 959 kB 432.21ms - 440.03ms slower ❌
1% - 4%
5.26ms - 16.40ms		 -

contextual-help permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 945 kB 56.96ms - 58.79ms - faster ✔
5% - 9%
2.87ms - 5.56ms
branch 899 kB 61.10ms - 63.08ms slower ❌
5% - 10%
2.87ms - 5.56ms		 -

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 741 kB 224.16ms - 226.39ms - faster ✔
1% - 4%
3.08ms - 8.28ms
branch 718 kB 228.61ms - 233.30ms slower ❌
1% - 4%
3.08ms - 8.28ms		 -

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 982 kB 460.54ms - 467.08ms - slower ❌
4% - 6%
17.43ms - 24.72ms
branch 942 kB 441.12ms - 444.34ms faster ✔
4% - 5%
17.43ms - 24.72ms		 -

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.07 MB 27.30ms - 27.87ms - faster ✔
5% - 7%
1.34ms - 2.17ms
branch 1.02 MB 29.04ms - 29.63ms slower ❌
5% - 8%
1.34ms - 2.17ms		 -

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.05 MB 373.16ms - 377.92ms - faster ✔
3% - 4%
10.13ms - 16.54ms
branch 1.01 MB 386.72ms - 391.02ms slower ❌
3% - 4%
10.13ms - 16.54ms		 -

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 852 kB 46.40ms - 47.58ms - faster ✔
6% - 9%
2.98ms - 4.55ms
branch 806 kB 50.23ms - 51.28ms slower ❌
6% - 10%
2.98ms - 4.55ms		 -

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 534.15ms - 542.68ms - faster ✔
4% - 6%
19.75ms - 32.69ms
branch 776 kB 559.77ms - 569.50ms slower ❌
4% - 6%
19.75ms - 32.69ms		 -

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 111.98ms - 112.51ms - faster ✔
1% - 3%
1.29ms - 3.50ms
branch 775 kB 113.56ms - 115.71ms slower ❌
1% - 3%
1.29ms - 3.50ms		 -

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 873 kB 37.74ms - 38.30ms - faster ✔
4% - 6%
1.44ms - 2.61ms
branch 829 kB 39.53ms - 40.56ms slower ❌
4% - 7%
1.44ms - 2.61ms		 -

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 831 kB 26.79ms - 27.31ms - faster ✔
8% - 11%
2.44ms - 3.27ms
branch 789 kB 29.58ms - 30.22ms slower ❌
9% - 12%
2.44ms - 3.27ms		 -

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 957 kB 56.52ms - 57.62ms - faster ✔
4% - 7%
2.50ms - 4.07ms
branch 912 kB 59.79ms - 60.91ms slower ❌
4% - 7%
2.50ms - 4.07ms		 -

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 933 kB 46.60ms - 47.48ms - faster ✔
5% - 8%
2.41ms - 3.81ms
branch 887 kB 49.61ms - 50.69ms slower ❌
5% - 8%
2.41ms - 3.81ms		 -

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 66.58ms - 67.96ms - faster ✔
2% - 4%
1.35ms - 2.99ms
branch 763 kB 69.01ms - 69.87ms slower ❌
2% - 4%
1.35ms - 2.99ms		 -
Firefox

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 739 kB 122.17ms - 129.95ms - faster ✔
2% - 10%
2.42ms - 13.62ms
branch 716 kB 130.05ms - 138.11ms slower ❌
2% - 11%
2.42ms - 13.62ms		 -

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 959 kB 285.65ms - 290.03ms - faster ✔
15% - 17%
49.32ms - 56.76ms
branch 918 kB 337.87ms - 343.89ms slower ❌
17% - 20%
49.32ms - 56.76ms		 -

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 917 kB 156.71ms - 161.33ms - slower ❌
3% - 10%
4.61ms - 15.19ms
branch 875 kB 144.36ms - 153.88ms faster ✔
3% - 10%
4.61ms - 15.19ms		 -

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 916 kB 144.54ms - 149.70ms - unsure 🔍
-2% - +4%
-2.71ms - +6.07ms
branch 874 kB 141.89ms - 148.99ms unsure 🔍
-4% - +2%
-6.07ms - +2.71ms		 -

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1897.24ms - 1905.28ms - faster ✔
0% - 1%
6.49ms - 18.11ms
branch 1.05 MB 1909.36ms - 1917.76ms slower ❌
0% - 1%
6.49ms - 18.11ms		 -

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1900.27ms - 1905.05ms - faster ✔
0% - 1%
1.52ms - 10.04ms
branch 1.05 MB 1904.91ms - 1911.97ms slower ❌
0% - 1%
1.52ms - 10.04ms		 -

breadcrumbs permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 978 kB 825.03ms - 846.01ms - faster ✔
1% - 4%
6.21ms - 30.59ms
branch 937 kB 847.70ms - 860.14ms slower ❌
1% - 4%
6.21ms - 30.59ms		 -

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 76.50ms - 84.06ms - slower ❌
11% - 23%
7.89ms - 15.75ms
branch 959 kB 67.39ms - 69.53ms faster ✔
10% - 19%
7.89ms - 15.75ms		 -

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 711.78ms - 727.78ms - slower ❌
1% - 6%
9.12ms - 41.68ms
branch 959 kB 680.20ms - 708.56ms faster ✔
1% - 6%
9.12ms - 41.68ms		 -

contextual-help permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 945 kB 112.41ms - 119.03ms - unsure 🔍
-6% - +1%
-7.03ms - +1.55ms
branch 899 kB 115.73ms - 121.19ms unsure 🔍
-1% - +6%
-1.55ms - +7.03ms		 -

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 741 kB 392.07ms - 405.09ms - faster ✔
1% - 7%
5.38ms - 28.06ms
branch 718 kB 406.01ms - 424.59ms slower ❌
1% - 7%
5.38ms - 28.06ms		 -

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1.06 MB 659.24ms - 677.36ms - faster ✔
8% - 12%
54.46ms - 87.46ms
branch 1.02 MB 725.47ms - 753.05ms slower ❌
8% - 13%
54.46ms - 87.46ms		 -

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.07 MB 51.59ms - 52.57ms - faster ✔
2% - 5%
1.28ms - 2.88ms
branch 1.02 MB 53.52ms - 54.80ms slower ❌
2% - 6%
1.28ms - 2.88ms		 -

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.05 MB 683.92ms - 694.32ms - slower ❌
8% - 10%
51.68ms - 63.76ms
branch 1.01 MB 628.33ms - 634.47ms faster ✔
8% - 9%
51.68ms - 63.76ms		 -

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 852 kB 95.92ms - 106.00ms - unsure 🔍
-2% - +8%
-2.34ms - +7.94ms
branch 806 kB 97.14ms - 99.18ms unsure 🔍
-8% - +2%
-7.94ms - +2.34ms		 -

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 979.22ms - 1004.38ms - faster ✔
4% - 7%
39.22ms - 69.14ms
branch 776 kB 1037.88ms - 1054.08ms slower ❌
4% - 7%
39.22ms - 69.14ms		 -

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 146.25ms - 152.99ms - faster ✔
1% - 7%
1.27ms - 11.57ms
branch 775 kB 152.14ms - 159.94ms slower ❌
1% - 8%
1.27ms - 11.57ms		 -

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 957 kB 87.07ms - 94.29ms - slower ❌
14% - 25%
10.96ms - 18.64ms
branch 912 kB 74.57ms - 77.19ms faster ✔
13% - 20%
10.96ms - 18.64ms		 -

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 831 kB 49.84ms - 52.40ms - faster ✔
0% - 5%
0.03ms - 2.77ms
branch 789 kB 52.02ms - 53.02ms unsure 🔍
-0% - +5%
+0.03ms - +2.77ms		 -

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 957 kB 148.58ms - 153.94ms - slower ❌
12% - 18%
15.77ms - 23.03ms
branch 912 kB 129.41ms - 134.31ms faster ✔
11% - 15%
15.77ms - 23.03ms		 -

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 933 kB 87.39ms - 89.89ms - faster ✔
22% - 29%
24.94ms - 36.18ms
branch 887 kB 113.72ms - 124.68ms slower ❌
28% - 41%
24.94ms - 36.18ms		 -

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 113.27ms - 120.57ms - unsure 🔍
-8% - +0%
-10.43ms - +0.47ms
branch 763 kB 117.86ms - 125.94ms unsure 🔍
-0% - +9%
-0.47ms - +10.43ms		 -

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jcmitch @coveralls