1609 - Do not set first item as active by default #1724
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). widget-test-docs – ./🔍 Inspect: https://vercel.com/dojo/widget-test-docs/4SjVuYk1R7iSjwFuMFiw2PtYnFrL dojo.widgets – ./🔍 Inspect: https://vercel.com/dojo/dojo.widgets/9LjGM31nXubbGdfGF7PMpfsYMaKh |
Codecov Report
@@ Coverage Diff @@
## master #1724 +/- ##
==========================================
+ Coverage 90.24% 90.35% +0.10%
==========================================
Files 94 94
Lines 5116 5120 +4
Branches 1392 1398 +6
==========================================
+ Hits 4617 4626 +9
+ Misses 244 243 -1
+ Partials 255 251 -4
Continue to review full report at Codecov.
|
…-item-selection-behavior
src/list/index.tsx
Outdated
| const first = items.slice(0, computedActiveIndex); | ||
| const second = items.slice(computedActiveIndex); | ||
| let foundIndex = computedActiveIndex; | ||
| const first = computedActiveIndex ? items.slice(0, computedActiveIndex) : []; |
There was a problem hiding this comment.
if computed active index was 0 wouldn't this be falsey rather than selecting the first item?
| @@ -807,7 +805,11 @@ export const List = factory(function List({ | |||
| styles={rootStyles} | |||
| role={menu ? 'menu' : 'listbox'} | |||
| aria-orientation="vertical" | |||
| aria-activedescendant={`${idBase}-item-${computedActiveIndex}`} | |||
| aria-activedescendant={ | |||
| computedActiveIndex !== undefined | |||
src/list/index.tsx
Outdated
| @@ -362,15 +362,21 @@ export const List = factory(function List({ | |||
| if (event.metaKey || event.ctrlKey) { | |||
| setActiveIndex(total - 1); | |||
| } else { | |||
| setActiveIndex((computedActiveIndex + 1) % total); | |||
| setActiveIndex( | |||
| (computedActiveIndex !== undefined ? computedActiveIndex + 1 : 0) % total | |||
There was a problem hiding this comment.
Can we do this outside the function call? It's a bit messy to understand at the moment.
src/list/index.tsx
Outdated
| } | ||
| break; | ||
| case Keys.Up: | ||
| event.preventDefault(); | ||
| if (event.metaKey || event.ctrlKey) { | ||
| setActiveIndex(0); | ||
| } else { | ||
| setActiveIndex((computedActiveIndex - 1 + total) % total); | ||
| setActiveIndex( | ||
| (computedActiveIndex !== undefined |
There was a problem hiding this comment.
Same comment re moving the logic out from the function call.
…-item-selection-behavior
Type: bug
The following has been addressed in the PR:
.dojorctheme.variant()is added to the root domnodetheme.composelike thisDescription: When no active index is set, no list item is selected instead of the previous behavior of defaulting to the first item.
Resolves #1609