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(coachmark,overlay): adjust imports of overlay and coachmark #4455

Merged
merged 2 commits into from
May 16, 2024

Conversation

jcmitch
Copy link
Contributor

@jcmitch jcmitch commented May 15, 2024

Fixing issue with various lit imports introduced in 0.42.3 release.

Similar to #4416 but found a few more issues that didn't show up on install but did show up when trying to upgrade. I looked at adding a import/no-unresolved eslint rule but ran into a few issues but we should probably look at doing this in the future.

Description

Both Coachmark and Overlay included imports directly from lit while neither package has lit as a direct dependency. Moving these imports to pull from base as this seems to be the established standard.

Related issue(s)

Motivation and context

Unable to use the latest release without needing to add additional dependencies.

How has this been tested?

  • Addressing all the various issues in error report
  • Code search for any remaining direct lit imports outside of storybook or base

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.

@jcmitch jcmitch requested a review from Westbrook May 15, 2024 04:40
Copy link

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:

Copy link

github-actions bot commented May 15, 2024

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 222.103 kB 210.167 kB 🏆 210.225 kB
Scripts 54.407 kB 47.869 kB 🏆 47.96 kB
Stylesheet 34.873 kB 30.497 kB 30.451 kB 🏆
Document 5.895 kB 5.187 kB 5.183 kB 🏆
Font 126.928 kB 126.614 kB 🏆 126.631 kB

Request Count

Category Latest Main Branch
Total 45 45 45
Scripts 37 37 37
Stylesheet 5 5 5
Document 1 1 1
Font 2 2 2

Copy link

github-actions bot commented May 15, 2024

Tachometer results

Chrome

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 484 kB 61.24ms - 65.32ms - faster ✔
3% - 11%
2.04ms - 7.88ms
branch 472 kB 66.15ms - 70.34ms slower ❌
3% - 13%
2.04ms - 7.88ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 171.85ms - 180.53ms - faster ✔
5% - 11%
9.63ms - 21.20ms
branch 634 kB 187.78ms - 195.44ms slower ❌
5% - 12%
9.63ms - 21.20ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 79.75ms - 85.82ms - faster ✔
4% - 13%
3.48ms - 12.15ms
branch 591 kB 87.51ms - 93.69ms slower ❌
4% - 15%
3.48ms - 12.15ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 78.11ms - 83.18ms - faster ✔
2% - 10%
1.52ms - 8.71ms
branch 590 kB 83.21ms - 88.31ms slower ❌
2% - 11%
1.52ms - 8.71ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 1891.84ms - 1900.79ms - unsure 🔍
-0% - +0%
-6.69ms - +5.69ms
branch 777 kB 1892.54ms - 1901.09ms unsure 🔍
-0% - +0%
-5.69ms - +6.69ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1885.55ms - 1892.12ms - unsure 🔍
-0% - +0%
-8.68ms - +1.21ms
branch 775 kB 1888.87ms - 1896.27ms unsure 🔍
-0% - +0%
-1.21ms - +8.68ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 47.66ms - 51.30ms - faster ✔
0% - 9%
0.09ms - 4.97ms
branch 489 kB 50.38ms - 53.64ms slower ❌
0% - 10%
0.09ms - 4.97ms
-

coachmark permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 630 kB 117.29ms - 121.86ms - faster ✔
3% - 9%
3.25ms - 11.13ms
branch 618 kB 123.56ms - 129.97ms slower ❌
3% - 9%
3.25ms - 11.13ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 46.28ms - 50.19ms - unsure 🔍
-10% - +1%
-4.86ms - +0.61ms
branch 697 kB 48.45ms - 52.27ms unsure 🔍
-1% - +10%
-0.61ms - +4.86ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 488.96ms - 508.08ms - faster ✔
1% - 6%
6.40ms - 31.25ms
branch 697 kB 509.41ms - 525.28ms slower ❌
1% - 6%
6.40ms - 31.25ms
-

illustrated-message permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 404 kB 16.69ms - 18.06ms - unsure 🔍
-7% - +5%
-1.31ms - +0.80ms
branch 393 kB 16.83ms - 18.44ms unsure 🔍
-5% - +8%
-0.80ms - +1.31ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 475 kB 287.41ms - 302.31ms - unsure 🔍
-5% - +2%
-15.58ms - +6.54ms
branch 463 kB 291.21ms - 307.55ms unsure 🔍
-2% - +5%
-6.54ms - +15.58ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 678 kB 556.75ms - 583.09ms - unsure 🔍
-4% - +2%
-20.86ms - +13.78ms
branch 657 kB 562.22ms - 584.70ms unsure 🔍
-2% - +4%
-13.78ms - +20.86ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 773 kB 27.72ms - 30.15ms - faster ✔
7% - 18%
2.33ms - 5.93ms
branch 761 kB 31.73ms - 34.40ms slower ❌
8% - 21%
2.33ms - 5.93ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 763 kB 455.33ms - 473.72ms - faster ✔
1% - 7%
5.55ms - 34.12ms
branch 750 kB 473.44ms - 495.29ms slower ❌
1% - 7%
5.55ms - 34.12ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 561 kB 53.26ms - 56.57ms - faster ✔
3% - 11%
1.55ms - 6.67ms
branch 548 kB 57.07ms - 60.97ms slower ❌
3% - 12%
1.55ms - 6.67ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 513 kB 676.83ms - 698.84ms - faster ✔
1% - 6%
6.32ms - 40.75ms
branch 500 kB 698.13ms - 724.60ms slower ❌
1% - 6%
6.32ms - 40.75ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 384 kB 13.72ms - 14.90ms - unsure 🔍
-12% - +1%
-1.86ms - +0.25ms
branch 372 kB 14.24ms - 15.99ms unsure 🔍
-2% - +13%
-0.25ms - +1.86ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 98.52ms - 105.12ms - unsure 🔍
-8% - +1%
-8.08ms - +0.66ms
branch 467 kB 102.66ms - 108.39ms unsure 🔍
-1% - +8%
-0.66ms - +8.08ms
-

split-button permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 723 kB 1889.43ms - 1894.65ms - unsure 🔍
-0% - +0%
-7.28ms - +1.84ms
branch 710 kB 1891.02ms - 1898.49ms unsure 🔍
-0% - +0%
-1.84ms - +7.28ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 561 kB 42.14ms - 45.41ms - unsure 🔍
-7% - +3%
-3.37ms - +1.33ms
branch 563 kB 43.10ms - 46.49ms unsure 🔍
-3% - +8%
-1.33ms - +3.37ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 533 kB 28.84ms - 31.47ms - faster ✔
3% - 14%
1.06ms - 4.87ms
branch 521 kB 31.74ms - 34.50ms slower ❌
3% - 16%
1.06ms - 4.87ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 65.08ms - 69.04ms - unsure 🔍
-8% - +1%
-5.33ms - +0.61ms
branch 643 kB 67.21ms - 71.63ms unsure 🔍
-1% - +8%
-0.61ms - +5.33ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 632 kB 51.15ms - 54.54ms - faster ✔
4% - 13%
2.34ms - 7.75ms
branch 619 kB 55.78ms - 60.00ms slower ❌
4% - 15%
2.34ms - 7.75ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 528 kB 73.74ms - 80.55ms - unsure 🔍
-11% - +0%
-9.46ms - +0.55ms
branch 516 kB 77.94ms - 85.27ms unsure 🔍
-1% - +12%
-0.55ms - +9.46ms
-
Firefox

action-bar permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 484 kB 110.45ms - 115.55ms - faster ✔
1% - 7%
0.72ms - 8.32ms
branch 472 kB 114.71ms - 120.33ms slower ❌
1% - 7%
0.72ms - 8.32ms
-

action-menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 647 kB 273.84ms - 276.28ms - faster ✔
12% - 14%
38.68ms - 43.32ms
branch 634 kB 314.09ms - 318.03ms slower ❌
14% - 16%
38.68ms - 43.32ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 604 kB 128.64ms - 131.92ms - unsure 🔍
-2% - +1%
-2.63ms - +0.87ms
branch 591 kB 130.55ms - 131.77ms unsure 🔍
-1% - +2%
-0.87ms - +2.63ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 603 kB 152.21ms - 156.47ms - slower ❌
7% - 12%
9.60ms - 16.44ms
branch 590 kB 138.64ms - 144.00ms faster ✔
6% - 11%
9.60ms - 16.44ms
-

test-open-close-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 789 kB 1917.15ms - 1925.65ms - slower ❌
2% - 2%
28.85ms - 38.31ms
branch 777 kB 1885.73ms - 1889.91ms faster ✔
2% - 2%
28.85ms - 38.31ms
-

test-open-close permalink

Version Bytes Avg Time vs remote vs branch
npm latest 788 kB 1884.47ms - 1889.73ms - unsure 🔍
-0% - +0%
-4.56ms - +4.24ms
branch 775 kB 1883.73ms - 1890.79ms unsure 🔍
-0% - +0%
-4.24ms - +4.56ms
-

card permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 502 kB 74.02ms - 80.10ms - unsure 🔍
-10% - +1%
-7.75ms - +1.15ms
branch 489 kB 77.11ms - 83.61ms unsure 🔍
-2% - +10%
-1.15ms - +7.75ms
-

coachmark permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 630 kB 184.42ms - 187.62ms - faster ✔
3% - 5%
5.66ms - 9.58ms
branch 618 kB 192.51ms - 194.77ms slower ❌
3% - 5%
5.66ms - 9.58ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 63.10ms - 69.10ms - slower ❌
2% - 13%
1.09ms - 7.95ms
branch 697 kB 59.92ms - 63.24ms faster ✔
2% - 12%
1.09ms - 7.95ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 716.84ms - 727.40ms - slower ❌
2% - 4%
10.82ms - 28.18ms
branch 697 kB 695.73ms - 709.51ms faster ✔
2% - 4%
10.82ms - 28.18ms
-

illustrated-message permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 404 kB 25.47ms - 26.61ms - unsure 🔍
-4% - +2%
-1.17ms - +0.45ms
branch 393 kB 25.82ms - 26.98ms unsure 🔍
-2% - +5%
-0.45ms - +1.17ms
-

menu permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 475 kB 423.86ms - 437.42ms - faster ✔
1% - 5%
4.74ms - 23.62ms
branch 463 kB 438.25ms - 451.39ms slower ❌
1% - 6%
4.74ms - 23.62ms
-

overlay permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 770 kB 625.26ms - 634.94ms - slower ❌
3% - 5%
16.39ms - 28.37ms
branch 757 kB 604.19ms - 611.25ms faster ✔
3% - 4%
16.39ms - 28.37ms
-

directive-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 773 kB 45.48ms - 46.80ms - faster ✔
4% - 7%
1.70ms - 3.58ms
branch 761 kB 48.11ms - 49.45ms slower ❌
4% - 8%
1.70ms - 3.58ms
-

element-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 763 kB 645.42ms - 649.54ms - slower ❌
4% - 5%
24.89ms - 30.83ms
branch 750 kB 617.48ms - 621.76ms faster ✔
4% - 5%
24.89ms - 30.83ms
-

lazy-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 561 kB 102.24ms - 111.72ms - slower ❌
10% - 20%
9.10ms - 18.66ms
branch 548 kB 92.46ms - 93.74ms faster ✔
9% - 17%
9.10ms - 18.66ms
-

picker permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 513 kB 995.39ms - 1020.49ms - faster ✔
3% - 5%
30.90ms - 57.18ms
branch 500 kB 1048.08ms - 1055.88ms slower ❌
3% - 6%
30.90ms - 57.18ms
-

popover permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 384 kB 28.48ms - 31.32ms - unsure 🔍
-15% - +1%
-4.91ms - +0.35ms
branch 372 kB 29.96ms - 34.40ms unsure 🔍
-1% - +17%
-0.35ms - +4.91ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 163.26ms - 170.74ms - unsure 🔍
-5% - +1%
-8.11ms - +1.51ms
branch 467 kB 167.28ms - 173.32ms unsure 🔍
-1% - +5%
-1.51ms - +8.11ms
-

split-button permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 723 kB 1873.46ms - 1878.50ms - unsure 🔍
-0% - +0%
-5.16ms - +2.72ms
branch 710 kB 1874.18ms - 1880.22ms unsure 🔍
-0% - +0%
-2.72ms - +5.16ms
-

tooltip permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 80.61ms - 83.35ms - slower ❌
11% - 16%
7.73ms - 11.23ms
branch 643 kB 71.40ms - 73.60ms faster ✔
10% - 14%
7.73ms - 11.23ms
-

test-directive permalink

Version Bytes Avg Time vs remote vs branch
npm latest 533 kB 46.11ms - 47.05ms - faster ✔
28% - 33%
17.88ms - 22.32ms
branch 521 kB 64.52ms - 68.84ms slower ❌
38% - 48%
17.88ms - 22.32ms
-

test-element permalink

Version Bytes Avg Time vs remote vs branch
npm latest 656 kB 109.96ms - 115.08ms - faster ✔
6% - 11%
7.22ms - 13.94ms
branch 643 kB 120.92ms - 125.28ms slower ❌
6% - 13%
7.22ms - 13.94ms
-

test-lazy permalink

Version Bytes Avg Time vs remote vs branch
npm latest 632 kB 91.56ms - 95.32ms - faster ✔
6% - 11%
5.73ms - 11.11ms
branch 619 kB 99.93ms - 103.79ms slower ❌
6% - 12%
5.73ms - 11.11ms
-

truncated permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 528 kB 100.10ms - 106.22ms - unsure 🔍
-8% - +0%
-8.45ms - +0.33ms
branch 516 kB 104.07ms - 110.37ms unsure 🔍
-0% - +8%
-0.33ms - +8.45ms
-

@Rajdeepc Rajdeepc requested a review from a team May 15, 2024 09:58
@Rajdeepc Rajdeepc added bug Something isn't working dependencies Pull requests that update a dependency file labels May 15, 2024
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.

Great Catch! We gotta be more careful with these in the future. Thnx for the good work!

@Rajdeepc
Copy link
Contributor

Rajdeepc commented May 16, 2024

@jcmitch Can you please pull this up to main so that we can make sure that the code coverage is not making noise before we can land this to main. Thanks

@Rajdeepc Rajdeepc requested a review from a team May 16, 2024 05:59
@jcmitch
Copy link
Contributor Author

jcmitch commented May 16, 2024

@Rajdeepc done

@TarunAdobe TarunAdobe merged commit 39706da into main May 16, 2024
58 checks passed
@TarunAdobe TarunAdobe deleted the jmitchel/4454-lit-import-fix branch May 16, 2024 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants