Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboarding is inconsistent when interacting with a SearchBox in a menu #20074

Closed
makopch-ms opened this issue Oct 1, 2021 · 5 comments · Fixed by #25172
Closed

Keyboarding is inconsistent when interacting with a SearchBox in a menu #20074

makopch-ms opened this issue Oct 1, 2021 · 5 comments · Fixed by #25172
Labels
Component: ContextualMenu Fluent UI react (v8) Issues about @fluentui/react (v8) From Shield Issue has been set for investigation Partner Ask Status: Fixed Fixed in some PR

Comments

@makopch-ms
Copy link
Contributor

Please provide a reproduction of the bug in a codepen:

https://codepen.io/makopch/pen/rNwbwgp

Open the Menu and use the keyboard to cycle through the items

Actual behavior:

The keyboarding is not consistent. If you down arrow in the search box, focus will go back to the top menuitem of the menu. If you up-arrow, focus will go to the last item in the menu.

Expected behavior:

The searchbox should behave like the other menu items and you should be able to arrow in and out of it without jumping to the top or bottom of the menu.

Priorities and help requested:

Are you willing to submit a PR to fix? (Yes, No)

Requested priority: (Blocking, High, Normal, Low) Normal

Products/sites affected: (if applicable)

@gouttierre gouttierre added Component: ContextualMenu Fluent UI react (v8) Issues about @fluentui/react (v8) Needs: Investigation The Shield Dev should investigate this issue and propose a fix and removed Needs: Triage 🔍 labels Oct 25, 2021
@gouttierre
Copy link
Contributor

gouttierre commented Oct 25, 2021

Thanks for filing this issue with us. To set expectations the developers will review this issue once capacity allows.

@gouttierre gouttierre moved this to Good Issues (High Priority) in Fluent UI - Shield Priors Jan 10, 2022
@JustSlone JustSlone moved this from Good Issues (High Priority) to Help Wanted (Lower Priority) in Fluent UI - Shield Priors Mar 23, 2022
@JustSlone JustSlone moved this from Help Wanted (Lower Priority) to Backlog in Fluent UI - Shield Priors Mar 23, 2022
@kysedate
Copy link
Contributor

@gouttierre Just checking in as this bug has been active for a while. Is this issue still on your radar?

@Aidam1
Copy link

Aidam1 commented Aug 3, 2022

@gouttierre Can you please suggest workaround for this issue in the meantime? How can I at least programatically focus correct MenuItem? I cannot seem to find componentRef that can be focused.

@gouttierre gouttierre moved this from Backlog to Parking-Lot in Fluent UI - Shield Priors Oct 3, 2022
@gouttierre
Copy link
Contributor

This issue has been queued for upcoming investigations. To set expectations the developers will review this issue once capacity allows and all updates will be posted on the issue accordingly. If a work around is found during investigations they will share the implementations with you.

@miroslavstastny miroslavstastny added the 🧐 Under investigation Issue being investigated by shield label Oct 11, 2022
@miroslavstastny
Copy link
Member

The focus inside the menu is managed by a FocusZone component. By default, it does not allow the <input> to lose focus on arrow key. There is a shouldInputLoseFocusOnArrowKey prop which allows to override the default behavior.

In the case of the ContextualMenu you can pass focusZoneProps to the menuProps:

<DefaultButton
  ...
  menuProps={{
    ...
    focusZoneProps: {
      shouldInputLoseFocusOnArrowKey: () => true
    }
  }}
/> 

Now Arrow Up and Arrow Down navigation should work as expected as long as the input is empty.

But once there is a content inside the input, the navigation is broken again. The reason is onMenuKeyDown handler in ContextualMenu.base.tsx:

const onMenuKeyDown = (ev: React.KeyboardEvent<HTMLElement>) => {
// Mark as handled if onKeyDown returns true (for handling collapse cases)
// or if we are attempting to expand a submenu
const handled = onKeyDown(ev);
if (handled || !hostElement.current) {
return;
}
// If we have a modifier key being pressed, we do not want to move focus.
// Otherwise, handle up and down keys.
const hasModifier = !!(ev.altKey || ev.metaKey);
// eslint-disable-next-line deprecation/deprecation
const isUp = ev.which === KeyCodes.up;
// eslint-disable-next-line deprecation/deprecation
const isDown = ev.which === KeyCodes.down;
if (!hasModifier && (isUp || isDown)) {
const elementToFocus = isUp
? getLastFocusable(hostElement.current, hostElement.current.lastChild as HTMLElement, true)
: getFirstFocusable(hostElement.current, hostElement.current.firstChild as HTMLElement, true);
if (elementToFocus) {
elementToFocus.focus();
ev.preventDefault();
ev.stopPropagation();
}
}
};

When there is content inside the input and cursor is not at the end of the text in case of Arrow Down (at the beginning of the text in case of Up Arrow) FocusZone does not move focus out of the input.
But the handler in ContextualMenu when focus is not moved focuses first/last focusable item in the menu which is not what we want in this case.

There is a hacky workaround - add key down handler to the SearchBox which will stop the event propagation in this case to avoid the ContextualMenu to move the focus:

const onKeyDown = React.useCallback((e, ...args) => {
  /* Key Up, but we are not at the beginning of the text: stop event propagation to prevent ContextualMenu to focus */
  if (e.target.selectionStart > 0 && e.which === KeyCodes.up) {
    e.stopPropagation();
  }
  /* Key Down, but we are not at the end of the text: stop event propagation to prevent ContextualMenu to focus */
  if (e.target.selectionStart !== e.target.value.length && e.which === KeyCodes.down) {
    e.stopPropagation();
  }
}, []);

Here is a codesandbox demonstrating the working solution: https://codesandbox.io/s/fui-menu-searchbox-dfbx8q.

I will create a PR to apply the same changes to the ContextualMenuWithCustomMenuListExample.

@miroslavstastny miroslavstastny added From Shield Issue has been set for investigation and removed Needs: Investigation The Shield Dev should investigate this issue and propose a fix 🧐 Under investigation Issue being investigated by shield labels Oct 11, 2022
@miroslavstastny miroslavstastny moved this from Parking-Lot to Done in Fluent UI - Shield Priors Oct 11, 2022
@msft-fluent-ui-bot msft-fluent-ui-bot added Status: Fixed Fixed in some PR and removed Status: In PR labels Apr 22, 2023
@microsoft microsoft locked as resolved and limited conversation to collaborators May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: ContextualMenu Fluent UI react (v8) Issues about @fluentui/react (v8) From Shield Issue has been set for investigation Partner Ask Status: Fixed Fixed in some PR
Projects
7 participants