fix(hooks): do not highlight disabled items on menu open

[silviuaavram]

What:
Check if items are disabled before highlighting them, when opening the menu with initialHighlightedIndex or defaultHighlightedIndex.

Why:
Fixes #1584.

How:
Check if the items are disabled before returning defaultHighlightedIndex / initialHighlightedIndex as the highlightedIndex on menu open.

Checklist:

• Documentation
• Tests
• TypeScript Types
• Flow Types
• Ready to be merged
  

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (57981b2) to head (a06fe67).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #1587   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1721      1724    +3     
  Branches       515       518    +3     
=========================================
+ Hits          1721      1724    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

🎉 This PR is included in version 9.0.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[conflma]

Hi @silviuaavram.

Thanks for the fix. Unfortunately, it does not seem to work, when using the Arrow keys. Say you have a input that you "activate" by Tab, then use the "ArrowDown"-Key to step into the dropdown. Now, even if the first item is disabled, it is highlighted and even selectable by hitting "Enter".

[silviuaavram]

Hi, @conflma ! Could you provide a sandbox or something similar so we can repro the scenario? Thank you!

[silviuaavram]

Found the issue. Will fix it shortly. Indeed, it's a different unhandled case with Arrows.

[conflma]

https://codesandbox.io/p/sandbox/morning-morning-326kwz?file=%2Fsrc%2Findex.tsx%3A12%2C18

Just finished the Sandbox in the moment you commented.

Steps to reproduce (Do NOT use the Mouse, Keys only):

• Step into the input by Hitting "Tab"
• Press "Arrow Down"
• Press "Enter"

=> Not only the disabled item is highlighted, it is even selectable and produces basically a falsy / not allowed value within the input.

[conflma]

@silviuaavram FYI #1295 also describes the problem. Selection of disabled items is not possible, when the menu is opened with a click and you'll then go trough the list with the keys. But if you OPEN the menu with the keyboard (by pressing ArrowDown, ArrowDown) - basically using only the keyboard - it is selectable.

[silviuaavram]

Fix is merged, please wait for 9.0.2 and confirm it works as expected. Thank you!

[conflma]

@silviuaavram Thanks! Looking forward to it :) BTW, you mean 9.0.3? 9.0.2 is currently released?

[silviuaavram]

yeah, sure. my bad :D

[conflma]

@silviuaavram I can confirm it works - it is not selecting a first or last item or any disabled item at the end or the start when, if it is marked as disabled and the menu is either triggered by ArrowUp or ArrowDown.

See updated sandbox (https://codesandbox.io/p/sandbox/morning-morning-326kwz)

Thanks for the quick fix!

However, from a UX / a11y perspective, wouldn't it be nice, if the first item (either from the top if the menu was opened with ArrowDown or the end if the menu was opened with ArrowUp), which is not disabled would be highlighted / selected? Currently you have to press either ArrowUp / ArrowDown twice to get to the first item, that is not disabled.

What is your opinion on that? Valid feature request?

[silviuaavram]

I think it makes sense. We can update getHighlightedIndexOnOpen to leverage getHighlightedIndex. Could your create a ticket with the spec? You can also create a PR with the implementation, I'd be happy to review it. Thanks!

[conflma]

@silviuaavram I've created the ticket (#1593). I fear that I won't be able to implement it myself due to quite a shortage of time in the moment. I've just stumbled across the problem, because I was building some components for an internal library with downshift.