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.

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

fix(overlay): remove flex display for dialog #4902

Merged
merged 8 commits into from
Nov 11, 2024
Merged

Conversation

AndreiBaicu26
Copy link
Contributor

@AndreiBaicu26 AndreiBaicu26 commented Nov 5, 2024

Description

Display flex would affect the positioning of the dialog in Safari.

BEFORE:
https://github.com/user-attachments/assets/a284dee0-9102-4944-87e8-6e052a34ac0f

AFTER:
https://github.com/user-attachments/assets/8476b0c2-bd7a-4957-8107-c8bfcc1ed393

Related issue(s)

Fixes #4734

Motivation and context

How has this been tested?

  • Test case 1

  • Create a parent element that has transform: translate() style applied.

  • Add a child element that opens a tooltip/popover.

  • The popover should render correctly.

  • Did it pass in Desktop?

  • Did it pass in Mobile?

  • Did it pass in iPad?

Screenshots (if appropriate)

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.

@coveralls
Copy link
Collaborator

coveralls commented Nov 5, 2024

Pull Request Test Coverage Report for Build 11775051345

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.194%

Totals Coverage Status
Change from base Build 11756411167: 0.0%
Covered Lines: 32315
Relevant Lines: 32733

💛 - Coveralls

Copy link

github-actions bot commented Nov 5, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.99 0.98 1
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.238 kB 236.70 kB 🏆 236.737 kB
Scripts 60.339 kB 54.205 kB 🏆 54.342 kB
Stylesheet 53.863 kB 47.995 kB 47.951 kB 🏆
Document 6.238 kB 5.465 kB 5.464 kB 🏆
Font 126.949 kB 126.684 kB 126.632 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

Copy link

github-actions bot commented Nov 5, 2024

Tachometer results

Chrome

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 740 kB 54.66ms - 57.33ms - unsure 🔍
-8% - -0%
-4.72ms - +0.04ms
branch 715 kB 56.36ms - 60.30ms unsure 🔍
-0% - +8%
-0.04ms - +4.72ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 960 kB 138.56ms - 141.84ms - faster ✔
4% - 7%
5.99ms - 10.42ms
branch 916 kB 146.92ms - 149.89ms slower ❌
4% - 7%
5.99ms - 10.42ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 917 kB 69.24ms - 70.28ms - faster ✔
5% - 7%
3.76ms - 5.42ms
branch 874 kB 73.70ms - 74.99ms slower ❌
5% - 8%
3.76ms - 5.42ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 916 kB 68.00ms - 68.99ms - faster ✔
5% - 8%
3.89ms - 5.67ms
branch 873 kB 72.54ms - 74.02ms slower ❌
6% - 8%
3.89ms - 5.67ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1875.44ms - 1877.94ms - unsure 🔍
-0% - +0%
-1.93ms - +2.03ms
branch 1.05 MB 1875.11ms - 1878.17ms unsure 🔍
-0% - +0%
-2.03ms - +1.93ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1878.57ms - 1881.44ms - unsure 🔍
-0% - +0%
-1.69ms - +2.02ms
branch 1.05 MB 1878.66ms - 1881.02ms unsure 🔍
-0% - +0%
-2.02ms - +1.69ms
-

breadcrumbs permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 979 kB 525.41ms - 530.82ms - unsure 🔍
-0% - +1%
-1.56ms - +5.72ms
branch 935 kB 523.59ms - 528.47ms unsure 🔍
-1% - +0%
-5.72ms - +1.56ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 41.92ms - 42.45ms - faster ✔
2% - 3%
0.67ms - 1.44ms
branch 957 kB 42.96ms - 43.51ms slower ❌
2% - 3%
0.67ms - 1.44ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 406.12ms - 412.66ms - faster ✔
2% - 5%
9.80ms - 20.10ms
branch 957 kB 420.36ms - 428.32ms slower ❌
2% - 5%
9.80ms - 20.10ms
-

contextual-help permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 944 kB 55.29ms - 57.54ms - faster ✔
3% - 7%
1.50ms - 4.41ms
branch 898 kB 58.44ms - 60.31ms slower ❌
3% - 8%
1.50ms - 4.41ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 741 kB 212.00ms - 214.78ms - faster ✔
2% - 4%
3.34ms - 8.56ms
branch 716 kB 217.13ms - 221.56ms slower ❌
2% - 4%
3.34ms - 8.56ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 983 kB 433.98ms - 438.25ms - faster ✔
1% - 2%
2.34ms - 8.95ms
branch 939 kB 439.24ms - 444.27ms slower ❌
1% - 2%
2.34ms - 8.95ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.06 MB 25.88ms - 26.35ms - faster ✔
6% - 8%
1.61ms - 2.40ms
branch 1.02 MB 27.80ms - 28.44ms slower ❌
6% - 9%
1.61ms - 2.40ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.05 MB 350.55ms - 367.28ms - faster ✔
0% - 5%
0.58ms - 17.84ms
branch 1.01 MB 366.01ms - 370.24ms slower ❌
0% - 5%
0.58ms - 17.84ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 851 kB 44.39ms - 45.28ms - faster ✔
7% - 9%
3.13ms - 4.61ms
branch 805 kB 48.11ms - 49.30ms slower ❌
7% - 10%
3.13ms - 4.61ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 512.78ms - 521.77ms - faster ✔
2% - 5%
11.88ms - 26.11ms
branch 774 kB 530.76ms - 541.79ms slower ❌
2% - 5%
11.88ms - 26.11ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 107.18ms - 108.24ms - faster ✔
1% - 2%
1.00ms - 2.64ms
branch 774 kB 108.90ms - 110.16ms slower ❌
1% - 2%
1.00ms - 2.64ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 874 kB 35.74ms - 36.68ms - faster ✔
3% - 6%
1.04ms - 2.23ms
branch 827 kB 37.49ms - 38.21ms slower ❌
3% - 6%
1.04ms - 2.23ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 831 kB 25.36ms - 25.80ms - faster ✔
7% - 10%
2.02ms - 2.76ms
branch 788 kB 27.68ms - 28.27ms slower ❌
8% - 11%
2.02ms - 2.76ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 957 kB 54.32ms - 55.30ms - faster ✔
5% - 7%
2.61ms - 3.97ms
branch 910 kB 57.63ms - 58.58ms slower ❌
5% - 7%
2.61ms - 3.97ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 932 kB 44.09ms - 44.99ms - faster ✔
6% - 9%
2.94ms - 4.27ms
branch 886 kB 47.66ms - 48.64ms slower ❌
7% - 10%
2.94ms - 4.27ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 62.29ms - 62.97ms - faster ✔
4% - 6%
2.43ms - 3.69ms
branch 761 kB 65.16ms - 66.21ms slower ❌
4% - 6%
2.43ms - 3.69ms
-
Firefox

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 740 kB 119.93ms - 127.11ms - faster ✔
2% - 9%
1.92ms - 11.68ms
branch 715 kB 127.01ms - 133.63ms slower ❌
1% - 10%
1.92ms - 11.68ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 960 kB 286.82ms - 289.90ms - faster ✔
15% - 16%
49.09ms - 55.31ms
branch 916 kB 337.86ms - 343.26ms slower ❌
17% - 19%
49.09ms - 55.31ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 917 kB 158.24ms - 161.28ms - slower ❌
1% - 5%
1.55ms - 7.89ms
branch 874 kB 152.25ms - 157.83ms faster ✔
1% - 5%
1.55ms - 7.89ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 916 kB 139.03ms - 144.93ms - unsure 🔍
-5% - +0%
-6.99ms - +0.23ms
branch 873 kB 143.27ms - 147.45ms unsure 🔍
-0% - +5%
-0.23ms - +6.99ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1890.89ms - 1901.47ms - unsure 🔍
-1% - +0%
-9.74ms - +2.82ms
branch 1.05 MB 1896.26ms - 1903.02ms unsure 🔍
-0% - +1%
-2.82ms - +9.74ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.09 MB 1900.40ms - 1904.40ms - faster ✔
1% - 1%
10.87ms - 18.69ms
branch 1.05 MB 1913.82ms - 1920.54ms slower ❌
1% - 1%
10.87ms - 18.69ms
-

breadcrumbs permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 979 kB 849.43ms - 859.89ms - faster ✔
1% - 4%
4.95ms - 31.05ms
branch 935 kB 860.70ms - 884.62ms slower ❌
1% - 4%
4.95ms - 31.05ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 70.44ms - 73.36ms - unsure 🔍
-4% - +1%
-2.59ms - +0.47ms
branch 957 kB 72.49ms - 73.43ms unsure 🔍
-1% - +4%
-0.47ms - +2.59ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1 MB 761.81ms - 782.23ms - slower ❌
4% - 8%
29.64ms - 59.92ms
branch 957 kB 716.05ms - 738.43ms faster ✔
4% - 8%
29.64ms - 59.92ms
-

contextual-help permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 944 kB 115.37ms - 121.51ms - unsure 🔍
-7% - +0%
-8.21ms - +0.65ms
branch 898 kB 119.02ms - 125.42ms unsure 🔍
-1% - +7%
-0.65ms - +8.21ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 741 kB 405.50ms - 420.54ms - faster ✔
1% - 6%
4.53ms - 27.35ms
branch 716 kB 420.37ms - 437.55ms slower ❌
1% - 7%
4.53ms - 27.35ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 1.06 MB 691.67ms - 705.29ms - slower ❌
7% - 9%
44.33ms - 60.27ms
branch 1.01 MB 642.04ms - 650.32ms faster ✔
6% - 9%
44.33ms - 60.27ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.06 MB 53.73ms - 54.67ms - faster ✔
2% - 6%
1.33ms - 3.19ms
branch 1.02 MB 55.66ms - 57.26ms slower ❌
2% - 6%
1.33ms - 3.19ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 1.05 MB 702.13ms - 714.87ms - slower ❌
5% - 9%
35.79ms - 59.49ms
branch 1.01 MB 650.86ms - 670.86ms faster ✔
5% - 8%
35.79ms - 59.49ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 851 kB 94.91ms - 100.45ms - faster ✔
2% - 8%
2.19ms - 8.81ms
branch 805 kB 101.38ms - 104.98ms slower ❌
2% - 9%
2.19ms - 8.81ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 1022.71ms - 1038.37ms - faster ✔
4% - 7%
43.97ms - 75.87ms
branch 774 kB 1076.56ms - 1104.36ms slower ❌
4% - 7%
43.97ms - 75.87ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 817 kB 153.74ms - 161.26ms - faster ✔
0% - 7%
0.13ms - 11.83ms
branch 774 kB 159.00ms - 167.96ms slower ❌
0% - 8%
0.13ms - 11.83ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 956 kB 90.60ms - 96.76ms - slower ❌
15% - 24%
11.59ms - 18.45ms
branch 910 kB 77.15ms - 80.17ms faster ✔
13% - 19%
11.59ms - 18.45ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 831 kB 51.88ms - 53.68ms - faster ✔
0% - 4%
0.14ms - 2.06ms
branch 788 kB 53.57ms - 54.19ms slower ❌
0% - 4%
0.14ms - 2.06ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 957 kB 152.80ms - 158.20ms - slower ❌
10% - 16%
14.37ms - 22.11ms
branch 910 kB 134.49ms - 140.03ms faster ✔
9% - 14%
14.37ms - 22.11ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 932 kB 89.25ms - 91.71ms - faster ✔
27% - 33%
33.85ms - 43.71ms
branch 886 kB 124.48ms - 134.04ms slower ❌
37% - 48%
33.85ms - 43.71ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 117.40ms - 125.68ms - unsure 🔍
-6% - +3%
-7.69ms - +3.25ms
branch 761 kB 120.20ms - 127.32ms unsure 🔍
-3% - +6%
-3.25ms - +7.69ms
-

@AndreiBaicu26 AndreiBaicu26 changed the title fix(overlay): use overlaynopopover for webkit fix(overlay): remove flex display for dialog Nov 6, 2024
@AndreiBaicu26 AndreiBaicu26 marked this pull request as ready for review November 6, 2024 13:46
@AndreiBaicu26 AndreiBaicu26 requested a review from a team as a code owner November 6, 2024 13:46
Copy link
Collaborator

@rubencarvalho rubencarvalho left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! 👍

@rubencarvalho rubencarvalho merged commit 48448ea into main Nov 11, 2024
41 of 45 checks passed
@rubencarvalho rubencarvalho deleted the abaicu/safari-tooltip branch November 11, 2024 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug][Safari]: Tooltip tip is misaligned
4 participants