search

adobe / spectrum-web-components

Spectrum Web Components
https://opensource.adobe.com/spectrum-web-components/
Apache License 2.0
1.21k stars 192 forks source link

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

Closed nikkimk closed 1 month ago

nikkimk commented 1 month ago

Description

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

Related issue(s)

How has this been tested?

Types of changes

Checklist

github-actions[bot] commented 1 month ago

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: - [High Contrast Mode | Medium | LTR](https://1d45f7adda6948e8d6072ad425ec5bd7--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Medium | LTR](https://a5fa313405c3dfab1b7bec73fbc725f2--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Medium | RTL](https://3a72954f3e2940888ccfc0915c8467d4--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Large | LTR](https://09b51867e1950468c28a72b2f340bf4b--spectrum-web-components.netlify.app/review/) - [Spectrum | Lightest | Large | RTL](https://8887b5f0f9463d4e3c5a21312a44f563--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Medium | LTR](https://ee6d4207b437fb44f57f026bdc353293--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Medium | RTL](https://977024630aed1b745d57e926ae5da5c4--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Large | LTR](https://286bb416ae325377b246f1473b4e1082--spectrum-web-components.netlify.app/review/) - [Spectrum | Light | Large | RTL](https://88dbee8c2f0faa537c1a55b452e3a32d--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Medium | LTR](https://c5b3c6737941fb30a3b26fe6d4522ea0--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Medium | RTL](https://63395b5151a16429860081eeba0587f2--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Large | LTR](https://a76b563608305de3bb3df8ad25c3430d--spectrum-web-components.netlify.app/review/) - [Spectrum | Dark | Large | RTL](https://e2803fdf0ed5a094ec1b09d429ec741d--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Medium | LTR](https://d36a647a4b027f48ef9495da050e2b57--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Medium | RTL](https://ac123418892c8d278419ffe644f69a4d--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Large | LTR](https://dc36d61b3a3d7d6d580e6b3f76c0805a--spectrum-web-components.netlify.app/review/) - [Spectrum | Darkest | Large | RTL](https://f4bef167b19bb7560202ec3024918611--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Medium | LTR](https://7a9d572ed7eb5ead33c6ea844e261301--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Medium | RTL](https://607456acadda092a1987c96c1eb161c1--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Large | LTR](https://d79b2a1b9d99a95a0d3e38c2ee4f9baf--spectrum-web-components.netlify.app/review/) - [Express | Lightest | Large | RTL](https://59350d6335f7116264edc9a32381b949--spectrum-web-components.netlify.app/review/) - [Express | Light | Medium | LTR](https://75c796bc7ecef5ab9d109ad1f34b387a--spectrum-web-components.netlify.app/review/) - [Express | Light | Medium | RTL](https://04f1c5ffc3f5d673e253318894c0b050--spectrum-web-components.netlify.app/review/) - [Express | Light | Large | LTR](https://2aeaee7164fdb4dbcbea3d857e1a0af2--spectrum-web-components.netlify.app/review/) - [Express | Light | Large | RTL](https://7a1004b3e20d64147b1938a27e28d6b6--spectrum-web-components.netlify.app/review/) - [Express | Dark | Medium | LTR](https://2f64ccb496a22cd323a6f193328b9583--spectrum-web-components.netlify.app/review/) - [Express | Dark | Medium | RTL](https://cf1527adeca3bff285e68a24b9971a1b--spectrum-web-components.netlify.app/review/) - [Express | Dark | Large | LTR](https://37794e26dabe05d13bc50a855cd5cb56--spectrum-web-components.netlify.app/review/) - [Express | Dark | Large | RTL](https://3a1f7b85af71eb297fe797e81d501588--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Medium | LTR](https://dcd69cbd6e6350094a64f605b4ff2dc9--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Medium | RTL](https://b09f2b87f662b46e136d91387e53a695--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Large | LTR](https://ce28970046b22938cc282e4675383dba--spectrum-web-components.netlify.app/review/) - [Express | Darkest | Large | RTL](https://7d659def9acd012d02491f63fce55e96--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Medium | LTR](https://5b0a2bb3bdaf5f127d3ab5826c7c6301--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Medium | RTL](https://8f28a53309b97852946331f0fc58c76a--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Large | LTR](https://6c0ab0ce39c37720def9512d9779bb49--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Light | Large | RTL](https://e691536fee1e6f92765ad9d994cd1e8e--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Medium | LTR](https://914fb1252481699b667ea0ca91502b0d--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Medium | RTL](https://02d004b93c21212cab3718d3072c6673--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Large | LTR](https://dee2749ffd82e7aac3fc0cb15244c966--spectrum-web-components.netlify.app/review/) - [Spectrum-two | Dark | Large | RTL](https://d7b087228f14b634cc133f1f3e6bd193--spectrum-web-components.netlify.app/review/)
github-actions[bot] commented 1 month ago

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](https://github.com/GoogleChrome/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[bot] commented 1 month ago

Tachometer results

Chrome ## action-menu [_permalink_](#user-content-action-menu) ### 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_](#user-content-action-menu-test-directive) | 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_](#user-content-action-menu-test-lazy) | 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_](#user-content-action-menu-test-open-close-directive) | 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_](#user-content-action-menu-test-open-close) | 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_](#user-content-combobox) ### 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_](#user-content-combobox-light-dom-test) | 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_](#user-content-menu) ### 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_](#user-content-picker) ### 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_](#user-content-split-button) ### 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_](#user-content-action-menu) ### 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_](#user-content-action-menu-test-directive) | 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_](#user-content-action-menu-test-lazy) | 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_](#user-content-action-menu-test-open-close-directive) | 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_](#user-content-action-menu-test-open-close) | 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_](#user-content-combobox) ### 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_](#user-content-combobox-light-dom-test) | 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_](#user-content-menu) ### 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_](#user-content-picker) ### 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_](#user-content-split-button) ### 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 commented 1 month ago

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

nikkimk commented 1 month ago
  • (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 commented 1 month ago
  • (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.

https://github.com/adobe/spectrum-web-components/assets/32033946/4aae6748-a3ad-44f0-81d2-8bf887822088

nikkimk commented 1 month ago

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