feat: drop down keyboard navigation

[ivan-ngchakming]

Added keyboard navigation feature to drop down UI component

• Set focus to first drop down item on open
• Allow navigation using Tab
• Allow navigation using arrow keys (ArrowUp, ArrowDown)
• Allow closing drop down using Escape and return focus to the drop down button

closes #1923

Screens

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
lexical	✅ Ready (Inspect)	Visit Preview	May 20, 2022 at 5:15AM (UTC)
lexical-playground	✅ Ready (Inspect)	Visit Preview	May 20, 2022 at 5:15AM (UTC)

[trueadm]

Should be memoized to prevent React doing extra context updates.

[ivan-ngchakming]

I am not sure how to do this. Wrapping contextValue in useMemo with all its entries as dependencies doesnt make sense? Updates won't be reduced this way?

Anyway, I have moved the highlightedItem out of the contextValue object, and call the focus method from the parent instead of the within the item component, this should reduce the extra renders triggered by change in the highlighted item.

[zurfyx]

@ivan-ngchakming - useMemo will preserve the same object reference which means that Context.Provider will see as the same exact value and won't trigger further re-renders when that is the case.

Without useMemo, every time this components rerenders you're essentially generating a brand new object/value.

[always-maap]

I think when the drop-down is open, pressing Tab should close and focus on the next element but currently, it cycles between items

[acywatson]

I think when the drop-down is open, pressing Tab should close and focus on the next element but currently, it cycles between items

Is that correct? I'm not sure. @a11yHolli can your clarify the correct behavior for us here? Would love to get this one updated and merged. @ivan-ngchakming

[ivan-ngchakming]

I think when the drop-down is open, pressing Tab should close and focus on the next element but currently, it cycles between items

I believe this would be consistent with the native select.

I am currently working on my finals, will work on this in a few days as soon as I am free.

[acywatson]

This looks good to me now - thanks!

[acywatson]

We typically avoid optional chaining - I don't think we have the transpilation in place necessary to support various targets we support.

Does it work if you do something likee:

if (highlightedItem !== null && highlightedItem.current instanceof HTMLElement) {
  highlightedItem.current.focus();
}

[trueadm]

What if you did:

const highlightedItemElem: null | HTMLElement = highlightedItem.current
if (highlightedItemElem !== null) {
  highlightedItemElem.focus();
} 

[ivan-ngchakming]

I tried out a few ways

    // works
    if (highlightedItem && highlightedItem.current instanceof HTMLElement) {
      highlightedItem.current.focus();
    }

    // also works, since highlightedItem.current is either `HTMLElement` or `null`
    if (highlightedItem && highlightedItem.current) {
      highlightedItem.current.focus();
    }

    // type error: highlightedItem is possibly 'undefined' in `highlightedItem.current instanceof HTMLElement`
    if (highlightedItem !== null && highlightedItem.current instanceof HTMLElement) {
      highlightedItem.current.focus();
    }

    // type error: highlightedItem is possibly 'undefined'
    const highlightedItemElem: null | HTMLElement = highlightedItem.current
    if (highlightedItemElem !== null) {
      highlightedItemElem.focus();
    }

If its all the same, I will go with the second option as its more concise, but still satisfying all the type checks.

    if (highlightedItem && highlightedItem.current) {
      highlightedItem.current.focus();
    }

[always-maap]

what exactly is the active class doing?
without it the button is not full width

[ivan-ngchakming]

what exactly is the active class doing? without it the button is not full width

It was behaving like this before this PR if I remember correctly. I am guessing the style that should be applied on it is missing, maybe there should be some kind of icon or indicator to show the style applied on the selected text? But the non-active buttons not being full width issue would still needs to be fixed.