docs: refactor navigation and settings UI to leverage trigger directive #4247

Westbrook · 2024-04-05T14:58:59Z

Description

Move from custom inert based left/right navigation UI to use the trigger directive

How has this been tested?

Test case 1
1. Go here
2. In mobile screen sizes (less than 960px)
3. See that the left hand navigation opens/closes
4. See that the right hand options panel opens/closes
Test case 2
1. Go here
2. In mobile screen sizes (less than 960px)
3. See that the Overlay code is only loaded when opening the settings panel, side nav, or Picker.

Types of changes

Refactor

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

github-actions · 2024-04-05T15:01:32Z

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:

github-actions · 2024-04-05T15:03:27Z

Lighthouse scores

Category	Latest (report)	Main (report)	Branch (report)
Performance	0.95	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	229.073 kB	211.798 kB 🏆	212.184 kB
Scripts	60.761 kB	49.641 kB 🏆	49.92 kB
Stylesheet	35.641 kB	30.381 kB 🏆	30.451 kB
Document	5.959 kB	5.18 kB 🏆	5.198 kB
Font	126.712 kB	126.596 kB 🏆	126.615 kB

Request Count

Category	Latest	Main	Branch
Total	43	43	46
Scripts	35	35	38
Stylesheet	5	5	5
Document	1	1	1
Font	2	2	2

github-actions · 2024-04-05T15:03:29Z

Tachometer results

Chrome

action-bar permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	483 kB	48.96ms - 51.01ms	-	unsure 🔍 -5% - +1% -2.75ms - +0.64ms
branch	475 kB	49.69ms - 52.39ms	unsure 🔍 -1% - +6% -0.64ms - +2.75ms	-

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	646 kB	134.81ms - 138.24ms	-	unsure 🔍 -1% - +2% -1.90ms - +2.90ms
branch	638 kB	134.35ms - 137.71ms	unsure 🔍 -2% - +1% -2.90ms - +1.90ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	603 kB	62.18ms - 63.51ms	-	unsure 🔍 -1% - +2% -0.91ms - +1.08ms
branch	595 kB	62.02ms - 63.51ms	unsure 🔍 -2% - +1% -1.08ms - +0.91ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	602 kB	60.27ms - 61.91ms	-	unsure 🔍 -2% - +2% -1.05ms - +1.01ms
branch	594 kB	60.49ms - 61.73ms	unsure 🔍 -2% - +2% -1.01ms - +1.05ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	787 kB	1872.57ms - 1875.52ms	-	unsure 🔍 -0% - +0% -3.74ms - +0.56ms
branch	779 kB	1874.07ms - 1877.19ms	unsure 🔍 -0% - +0% -0.56ms - +3.74ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	786 kB	1858.62ms - 1861.78ms	-	unsure 🔍 -0% - +0% -3.99ms - +0.23ms
branch	778 kB	1860.68ms - 1863.47ms	unsure 🔍 -0% - +0% -0.23ms - +3.99ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	707 kB	36.50ms - 37.21ms	-	unsure 🔍 -2% - +1% -0.64ms - +0.23ms
branch	700 kB	36.80ms - 37.32ms	unsure 🔍 -1% - +2% -0.23ms - +0.64ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	707 kB	396.57ms - 403.34ms	-	unsure 🔍 -2% - +1% -8.30ms - +3.35ms
branch	700 kB	397.69ms - 407.17ms	unsure 🔍 -1% - +2% -3.35ms - +8.30ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	476 kB	218.03ms - 221.70ms	-	unsure 🔍 -0% - +2% -0.63ms - +4.39ms
branch	467 kB	216.27ms - 219.70ms	unsure 🔍 -2% - +0% -4.39ms - +0.63ms	-

overlay permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	666 kB	424.57ms - 430.19ms	-	unsure 🔍 -1% - +0% -5.51ms - +2.01ms
branch	668 kB	426.62ms - 431.63ms	unsure 🔍 -0% - +1% -2.01ms - +5.51ms	-

directive-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	769 kB	24.41ms - 25.65ms	-	slower ❌ 8% - 15% 1.89ms - 3.25ms
branch	776 kB	22.18ms - 22.74ms	faster ✔ 8% - 13% 1.89ms - 3.25ms	-

element-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	759 kB	345.73ms - 350.19ms	-	unsure 🔍 -1% - +0% -5.12ms - +1.38ms
branch	751 kB	347.46ms - 352.19ms	unsure 🔍 -0% - +1% -1.38ms - +5.12ms	-

lazy-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	559 kB	40.64ms - 41.87ms	-	unsure 🔍 -2% - +2% -0.70ms - +0.93ms
branch	552 kB	40.60ms - 41.67ms	unsure 🔍 -2% - +2% -0.93ms - +0.70ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	513 kB	545.01ms - 554.96ms	-	unsure 🔍 -0% - +2% -2.66ms - +11.40ms
branch	504 kB	540.64ms - 550.58ms	unsure 🔍 -2% - +0% -11.40ms - +2.66ms	-

popover permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	383 kB	11.50ms - 11.81ms	-	unsure 🔍 -1% - +2% -0.15ms - +0.19ms
branch	374 kB	11.57ms - 11.71ms	unsure 🔍 -2% - +1% -0.19ms - +0.15ms	-

split-button permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	721 kB	1855.69ms - 1858.70ms	-	unsure 🔍 -0% - -0% -8.29ms - -2.46ms
branch	713 kB	1860.07ms - 1865.07ms	unsure 🔍 +0% - +0% +2.46ms - +8.29ms	-

tooltip permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	548 kB	34.38ms - 35.31ms	-	unsure 🔍 -0% - +3% -0.16ms - +0.89ms
branch	538 kB	34.23ms - 34.73ms	unsure 🔍 -3% - +0% -0.89ms - +0.16ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	643 kB	26.19ms - 26.94ms	-	slower ❌ 9% - 14% 2.17ms - 3.25ms
branch	524 kB	23.47ms - 24.25ms	faster ✔ 8% - 12% 2.17ms - 3.25ms	-

test-element permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	652 kB	51.97ms - 53.25ms	-	unsure 🔍 -2% - +1% -1.15ms - +0.56ms
branch	645 kB	52.34ms - 53.47ms	unsure 🔍 -1% - +2% -0.56ms - +1.15ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	629 kB	42.06ms - 43.06ms	-	unsure 🔍 -3% - +1% -1.42ms - +0.29ms
branch	622 kB	42.43ms - 43.82ms	unsure 🔍 -1% - +3% -0.29ms - +1.42ms	-

truncated permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	525 kB	55.84ms - 56.76ms	-	unsure 🔍 -2% - +0% -1.19ms - +0.09ms
branch	518 kB	56.41ms - 57.30ms	unsure 🔍 -0% - +2% -0.09ms - +1.19ms	-

Firefox

action-bar permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	483 kB	103.13ms - 108.07ms	-	unsure 🔍 -3% - +4% -2.75ms - +4.31ms
branch	475 kB	102.30ms - 107.34ms	unsure 🔍 -4% - +3% -4.31ms - +2.75ms	-

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	646 kB	263.22ms - 265.30ms	-	faster ✔ 8% - 10% 24.04ms - 29.60ms
branch	638 kB	288.50ms - 293.66ms	slower ❌ 9% - 11% 24.04ms - 29.60ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	603 kB	121.75ms - 123.65ms	-	unsure 🔍 -1% - +1% -1.25ms - +1.21ms
branch	595 kB	121.94ms - 123.50ms	unsure 🔍 -1% - +1% -1.21ms - +1.25ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	602 kB	142.95ms - 148.29ms	-	slower ❌ 9% - 14% 11.36ms - 17.92ms
branch	594 kB	129.07ms - 132.89ms	faster ✔ 8% - 12% 11.36ms - 17.92ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	787 kB	1909.29ms - 1915.19ms	-	slower ❌ 1% - 2% 24.10ms - 31.82ms
branch	779 kB	1881.79ms - 1886.77ms	faster ✔ 1% - 2% 24.10ms - 31.82ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	786 kB	1879.52ms - 1884.56ms	-	unsure 🔍 -0% - +0% -2.22ms - +3.82ms
branch	778 kB	1879.58ms - 1882.90ms	unsure 🔍 -0% - +0% -3.82ms - +2.22ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	707 kB	66.19ms - 70.33ms	-	slower ❌ 13% - 21% 7.83ms - 12.13ms
branch	700 kB	57.73ms - 58.83ms	faster ✔ 12% - 17% 7.83ms - 12.13ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	707 kB	684.79ms - 696.69ms	-	slower ❌ 2% - 5% 11.70ms - 35.66ms
branch	700 kB	656.67ms - 677.45ms	faster ✔ 2% - 5% 11.70ms - 35.66ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	476 kB	413.06ms - 425.30ms	-	unsure 🔍 -2% - +3% -7.14ms - +11.42ms
branch	467 kB	410.06ms - 424.02ms	unsure 🔍 -3% - +2% -11.42ms - +7.14ms	-

overlay permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	765 kB	597.33ms - 603.71ms	-	slower ❌ 4% - 6% 21.38ms - 34.90ms
branch	758 kB	566.42ms - 578.34ms	faster ✔ 4% - 6% 21.38ms - 34.90ms	-

directive-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	769 kB	48.06ms - 48.54ms	-	slower ❌ 8% - 10% 3.72ms - 4.52ms
branch	762 kB	43.86ms - 44.50ms	faster ✔ 8% - 9% 3.72ms - 4.52ms	-

element-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	759 kB	610.97ms - 616.79ms	-	slower ❌ 5% - 6% 28.95ms - 35.93ms
branch	751 kB	579.51ms - 583.37ms	faster ✔ 5% - 6% 28.95ms - 35.93ms	-

lazy-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	559 kB	91.59ms - 94.37ms	-	slower ❌ 11% - 14% 8.75ms - 11.69ms
branch	552 kB	82.28ms - 83.24ms	faster ✔ 10% - 12% 8.75ms - 11.69ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	513 kB	940.60ms - 950.20ms	-	unsure 🔍 -1% - +1% -11.78ms - +9.30ms
branch	504 kB	937.25ms - 956.03ms	unsure 🔍 -1% - +1% -9.30ms - +11.78ms	-

popover permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	383 kB	27.88ms - 31.32ms	-	unsure 🔍 -11% - +3% -3.55ms - +0.99ms
branch	374 kB	29.40ms - 32.36ms	unsure 🔍 -4% - +12% -0.99ms - +3.55ms	-

split-button permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	721 kB	1871.16ms - 1874.08ms	-	unsure 🔍 -0% - -0% -9.35ms - -4.73ms
branch	713 kB	1877.87ms - 1881.45ms	unsure 🔍 +0% - +0% +4.73ms - +9.35ms	-

tooltip permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	652 kB	72.91ms - 77.45ms	-	slower ❌ 4% - 13% 2.63ms - 8.77ms
branch	645 kB	67.42ms - 71.54ms	faster ✔ 4% - 11% 2.63ms - 8.77ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	633 kB	49.79ms - 50.57ms	-	faster ✔ 9% - 17% 5.15ms - 10.09ms
branch	524 kB	55.36ms - 60.24ms	slower ❌ 10% - 20% 5.15ms - 10.09ms	-

test-element permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	652 kB	104.02ms - 108.58ms	-	faster ✔ 3% - 9% 3.04ms - 9.88ms
branch	645 kB	110.21ms - 115.31ms	slower ❌ 3% - 9% 3.04ms - 9.88ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	629 kB	87.67ms - 92.05ms	-	unsure 🔍 -6% - +1% -5.14ms - +1.18ms
branch	622 kB	89.56ms - 94.12ms	unsure 🔍 -1% - +6% -1.18ms - +5.14ms	-

truncated permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	525 kB	95.40ms - 100.00ms	-	unsure 🔍 -5% - +2% -4.58ms - +2.38ms
branch	518 kB	96.19ms - 101.41ms	unsure 🔍 -2% - +5% -2.38ms - +4.58ms	-

najikahalsema

Excellent! Thank you for finishing what i could not lol

najikahalsema · 2024-04-26T17:42:19Z

packages/overlay/src/events.ts

I like abstracting the events into their own file. Nice.

Rajdeepc · 2024-04-29T12:18:30Z

Should the left nav have a close button when opened?

Westbrook · 2024-04-29T12:38:13Z

Should the left nav have a close button when opened?

We could add one. It's likely even easier with this version of the UI. The current version doesn't include one, however. It closes by selecting a new page of tapping off of the side nav all together.

Rajdeepc · 2024-04-29T12:57:04Z

Should the left nav have a close button when opened?

We could add one. It's likely even easier with this version of the UI. The current version doesn't include one, however. It closes by selecting a new page of tapping off of the side nav all together.

Okay! Let's not hold coz of the close button for now!

The base branch was changed.

Westbrook requested review from majornista and a team April 5, 2024 14:58

Westbrook force-pushed the docs-with-overlay branch 2 times, most recently from b8643d9 to 271b174 Compare April 5, 2024 17:57

Westbrook force-pushed the docs-with-overlay branch 5 times, most recently from 08d8214 to c454670 Compare April 26, 2024 16:41

Westbrook changed the base branch from main to lazy-interaction-controller April 26, 2024 17:04

Westbrook force-pushed the lazy-interaction-controller branch from d9ef137 to d9d34de Compare April 26, 2024 17:16

Westbrook force-pushed the docs-with-overlay branch from c454670 to c061874 Compare April 26, 2024 17:21

Westbrook marked this pull request as ready for review April 26, 2024 17:25

Westbrook requested review from a team and removed request for a team and majornista April 26, 2024 17:25

najikahalsema previously approved these changes Apr 26, 2024

View reviewed changes

packages/overlay/src/events.ts

Copy link

Collaborator

najikahalsema Apr 26, 2024

Choose a reason for hiding this comment

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

I like abstracting the events into their own file. Nice.

Westbrook force-pushed the lazy-interaction-controller branch from d9d34de to 62ce3a9 Compare April 26, 2024 21:31

Westbrook force-pushed the docs-with-overlay branch from c061874 to df7a3b9 Compare April 26, 2024 21:34

Westbrook changed the title ~~docs: refactor navigation and settings UI to leverage sp-overlay~~ docs: refactor navigation and settings UI to leverage trigger directive Apr 29, 2024

Westbrook force-pushed the lazy-interaction-controller branch 2 times, most recently from 5873eec to f2a7dfa Compare April 29, 2024 12:35

Rajdeepc previously approved these changes Apr 29, 2024

View reviewed changes

Base automatically changed from lazy-interaction-controller to main April 29, 2024 13:09

Westbrook added 4 commits April 29, 2024 09:10

docs: refactor navigation and settings UI to leverage sp-overlay

fb84525

docs: power nav and settings panels with the "trigger" directive

d7c0f65

docs: prevent underlay outside of "mobile"

905f6f1

refactor: move around some code for bundle shape management

9bdac95

Westbrook force-pushed the docs-with-overlay branch from df7a3b9 to 9bdac95 Compare April 29, 2024 13:10

Westbrook merged commit 1df4c34 into main Apr 29, 2024
48 checks passed

Westbrook deleted the docs-with-overlay branch April 29, 2024 13:22

docs: refactor navigation and settings UI to leverage trigger directive #4247

docs: refactor navigation and settings UI to leverage trigger directive #4247

Conversation

Westbrook commented Apr 5, 2024 • edited

Description

How has this been tested?

Types of changes

Checklist

github-actions bot commented Apr 5, 2024 • edited

Branch preview

github-actions bot commented Apr 5, 2024 • edited

Lighthouse scores

Transfer Size

Request Count

github-actions bot commented Apr 5, 2024 • edited

Tachometer results

action-bar permalink

basic-test

action-menu permalink

test-basic

test-directive permalink

test-lazy permalink

test-open-close-directive permalink

test-open-close permalink

combobox permalink

basic-test

light-dom-test permalink

menu permalink

test-basic

overlay permalink

basic-test

directive-test permalink

element-test permalink

lazy-test permalink

picker permalink

basic-test

popover permalink

test-basic

split-button permalink

basic-test

tooltip permalink

test-basic

test-directive permalink

test-element permalink

test-lazy permalink

truncated permalink

basic-test

action-bar permalink

basic-test

action-menu permalink

test-basic

test-directive permalink

test-lazy permalink

test-open-close-directive permalink

test-open-close permalink

combobox permalink

basic-test

light-dom-test permalink

menu permalink

test-basic

overlay permalink

basic-test

directive-test permalink

element-test permalink

lazy-test permalink

picker permalink

basic-test

popover permalink

test-basic

split-button permalink

basic-test

tooltip permalink

test-basic

test-directive permalink

test-element permalink

test-lazy permalink

truncated permalink

basic-test

najikahalsema left a comment

Choose a reason for hiding this comment

Westbrook commented Apr 5, 2024 •

edited

github-actions bot commented Apr 5, 2024 •

edited

github-actions bot commented Apr 5, 2024 •

edited

github-actions bot commented Apr 5, 2024 •

edited