fix(menu): enable numpad arrow and Enter keys #4492

nikkimk · 2024-05-23T16:34:45Z

Description

Handles keydown event by event.key instead of event.code so that numpad keys will work with menu.

Related issue(s)

fixes [Bug]: sp-menu does not honor Numpad keys for Arrow ⬆️ ⬇️ and Enter #3751

How has this been tested?

Test case 1: No breaking changes for regular keys
1. Go here
2. Press Tab
3. From the standard keypad, press ↓
4. From the standard keypad, press Enter
Test case 2: Fix for Numpad keys
1. Go here
2. Press Tab
3. From the Numpad, press ↓
4. From the Numpad, press Enter

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. (Note: There is no way to use SendKeys to test Numpad keys)
All new and existing tests passed.
I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

github-actions · 2024-05-23T16:36:58Z

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-05-23T16:39:00Z

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.54 kB	210.768 kB 🏆	211.012 kB
Scripts	54.598 kB	48.307 kB 🏆	48.592 kB
Stylesheet	35.098 kB	30.553 kB	30.522 kB 🏆
Document	5.997 kB	5.285 kB	5.275 kB 🏆
Font	126.847 kB	126.623 kB	126.623 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

github-actions · 2024-05-23T17:00:15Z

Tachometer results

Chrome

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	647 kB	133.34ms - 135.64ms	-	faster ✔ 5% - 7% 6.93ms - 10.66ms
branch	634 kB	141.81ms - 144.75ms	slower ❌ 5% - 8% 6.93ms - 10.66ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	604 kB	60.84ms - 61.89ms	-	faster ✔ 7% - 10% 4.70ms - 6.82ms
branch	591 kB	66.20ms - 68.05ms	slower ❌ 8% - 11% 4.70ms - 6.82ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	603 kB	59.05ms - 60.08ms	-	faster ✔ 7% - 9% 4.20ms - 6.01ms
branch	590 kB	63.93ms - 65.41ms	slower ❌ 7% - 10% 4.20ms - 6.01ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	790 kB	1872.06ms - 1875.02ms	-	unsure 🔍 -0% - +0% -2.13ms - +1.96ms
branch	777 kB	1872.22ms - 1875.04ms	unsure 🔍 -0% - +0% -1.96ms - +2.13ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	788 kB	1858.61ms - 1861.68ms	-	unsure 🔍 -0% - +0% -1.65ms - +2.60ms
branch	775 kB	1858.20ms - 1861.14ms	unsure 🔍 -0% - +0% -2.60ms - +1.65ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	709 kB	34.95ms - 35.59ms	-	faster ✔ 4% - 6% 1.50ms - 2.34ms
branch	696 kB	36.93ms - 37.46ms	slower ❌ 4% - 7% 1.50ms - 2.34ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	710 kB	385.58ms - 391.56ms	-	faster ✔ 2% - 5% 9.90ms - 21.28ms
branch	697 kB	399.32ms - 408.99ms	slower ❌ 3% - 5% 9.90ms - 21.28ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	475 kB	208.83ms - 212.07ms	-	faster ✔ 2% - 4% 3.86ms - 8.13ms
branch	463 kB	215.05ms - 217.83ms	slower ❌ 2% - 4% 3.86ms - 8.13ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	513 kB	512.66ms - 522.59ms	-	faster ✔ 3% - 5% 13.45ms - 26.86ms
branch	500 kB	533.28ms - 542.29ms	slower ❌ 3% - 5% 13.45ms - 26.86ms	-

split-button permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	723 kB	1858.51ms - 1863.30ms	-	unsure 🔍 -0% - +0% -3.79ms - +2.02ms
branch	710 kB	1860.14ms - 1863.43ms	unsure 🔍 -0% - +0% -2.02ms - +3.79ms	-

Firefox

action-menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	647 kB	271.51ms - 277.09ms	-	faster ✔ 12% - 15% 37.80ms - 47.80ms
branch	634 kB	312.95ms - 321.25ms	slower ❌ 14% - 18% 37.80ms - 47.80ms	-

test-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	604 kB	128.21ms - 132.95ms	-	unsure 🔍 -4% - +0% -5.62ms - +0.42ms
branch	591 kB	131.30ms - 135.06ms	unsure 🔍 -0% - +4% -0.42ms - +5.62ms	-

test-lazy permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	603 kB	150.29ms - 156.11ms	-	slower ❌ 6% - 12% 8.07ms - 16.21ms
branch	590 kB	138.21ms - 143.91ms	faster ✔ 5% - 10% 8.07ms - 16.21ms	-

test-open-close-directive permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	790 kB	1909.80ms - 1919.00ms	-	slower ❌ 1% - 2% 23.97ms - 34.11ms
branch	777 kB	1883.25ms - 1887.47ms	faster ✔ 1% - 2% 23.97ms - 34.11ms	-

test-open-close permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	788 kB	1881.32ms - 1885.96ms	-	unsure 🔍 -0% - +0% -5.69ms - +1.45ms
branch	775 kB	1883.04ms - 1888.48ms	unsure 🔍 -0% - +0% -1.45ms - +5.69ms	-

combobox permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	709 kB	63.16ms - 69.48ms	-	slower ❌ 3% - 14% 1.96ms - 8.48ms
branch	696 kB	60.30ms - 61.90ms	faster ✔ 3% - 12% 1.96ms - 8.48ms	-

light-dom-test permalink

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	710 kB	721.40ms - 734.76ms	-	slower ❌ 1% - 4% 8.10ms - 28.98ms
branch	697 kB	701.51ms - 717.57ms	faster ✔ 1% - 4% 8.10ms - 28.98ms	-

menu permalink

test-basic

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	475 kB	425.19ms - 438.33ms	-	faster ✔ 0% - 4% 0.50ms - 18.22ms
branch	463 kB	435.17ms - 447.07ms	slower ❌ 0% - 4% 0.50ms - 18.22ms	-

picker permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	513 kB	980.84ms - 1008.16ms	-	faster ✔ 3% - 5% 27.74ms - 56.94ms
branch	500 kB	1031.69ms - 1041.99ms	slower ❌ 3% - 6% 27.74ms - 56.94ms	-

split-button permalink

basic-test

Version	Bytes	Avg Time	vs remote	vs branch
npm latest	723 kB	1876.40ms - 1880.80ms	-	unsure 🔍 -0% - +0% -2.57ms - +3.97ms
branch	710 kB	1875.48ms - 1880.32ms	unsure 🔍 -0% - +0% -3.97ms - +2.57ms	-

Rajdeepc · 2024-05-27T12:55:07Z

@nikkimk Tests are making some noise! Can you please check and do the needful so that we can land this!

…be/spectrum-web-components into nikkimk/3751-bug-sp-menu-numpad-keys

blunteshwar

Can we add relevant tests for this

nikkimk · 2024-05-29T13:24:14Z

(Note: There is no way to use SendKeys to test Numpad keys)

@ blunteshwar any ideas on how to add tests for this given the above?

blunteshwar · 2024-05-29T15:03:24Z

(Note: There is no way to use SendKeys to test Numpad keys)

@ blunteshwar any ideas on how to add tests for this given the above?

I think you can use SendKeys to send Numpad keys await sendKeys({press: 'NumpadEnter',}); will work just fine.
I am attaching a videos as well where it is shown that replacing Enter with NumpadEnter has no effect on tests.
We can write a test for 'Enter' and 'NumpadEnter', i think a single test with enter key can verify the changes.

Screen.Recording.2024-05-29.at.8.29.40.PM.mov

…be/spectrum-web-components into nikkimk/3751-bug-sp-menu-numpad-keys

nikkimk · 2024-05-30T17:48:07Z

@blunteshwar thanks. I've added the update. We just won't be able to do Numpad arrow keys that way.

fix(menu): enable numpad arrow and Enter keys

2c12a75

nikkimk linked an issue May 23, 2024 that may be closed by this pull request

[Bug]: sp-menu does not honor Numpad keys for Arrow ⬆️ ⬇️ and Enter #3751

Closed

1 task

nikkimk added 2 commits May 23, 2024 12:40

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

d8b98aa

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

2dbc829

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

d10d9dd

nikkimk marked this pull request as ready for review May 23, 2024 17:23

Rajdeepc requested a review from a team May 28, 2024 02:57

Rajdeepc added the Component: Menu label May 28, 2024

blunteshwar and others added 4 commits May 28, 2024 10:31

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

d1d68d1

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

6065b81

fix(menu): update Numpad Space key from code to key

fa5039f

Merge branch 'nikkimk/3751-bug-sp-menu-numpad-keys' of github.com:ado…

13fffa8

…be/spectrum-web-components into nikkimk/3751-bug-sp-menu-numpad-keys

blunteshwar requested changes May 29, 2024

View reviewed changes

blunteshwar and others added 3 commits May 29, 2024 20:42

Merge branch 'main' into nikkimk/3751-bug-sp-menu-numpad-keys

5a0a4a4

fix(menu): updated test for NumpadEnter

26aead9

Merge branch 'nikkimk/3751-bug-sp-menu-numpad-keys' of github.com:ado…

06ce3e1

…be/spectrum-web-components into nikkimk/3751-bug-sp-menu-numpad-keys

blunteshwar approved these changes Jun 3, 2024

View reviewed changes

Rajdeepc approved these changes Jun 3, 2024

View reviewed changes

Rajdeepc merged commit 012c411 into main Jun 3, 2024
58 checks passed

Rajdeepc deleted the nikkimk/3751-bug-sp-menu-numpad-keys branch June 3, 2024 07:23

fix(menu): enable numpad arrow and Enter keys #4492

fix(menu): enable numpad arrow and Enter keys #4492

Conversation

nikkimk commented May 23, 2024 • edited

Description

Related issue(s)

How has this been tested?

Types of changes

Checklist

github-actions bot commented May 23, 2024

Branch preview

github-actions bot commented May 23, 2024 • edited

Lighthouse scores

Transfer Size

Request Count

github-actions bot commented May 23, 2024 • edited

Tachometer results

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

picker permalink

basic-test

split-button 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

picker permalink

basic-test

split-button permalink

basic-test

Rajdeepc commented May 27, 2024

blunteshwar left a comment

Choose a reason for hiding this comment

nikkimk commented May 29, 2024

blunteshwar commented May 29, 2024 • edited

nikkimk commented May 30, 2024

nikkimk commented May 23, 2024 •

edited

github-actions bot commented May 23, 2024 •

edited

github-actions bot commented May 23, 2024 •

edited

blunteshwar commented May 29, 2024 •

edited