Skip to content

[lexical-react] Bug Fix: a menu with no options no longer swallows the arrow keys - #9017

Closed
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix/menu-arrow-keys-empty-options
Closed

[lexical-react] Bug Fix: a menu with no options no longer swallows the arrow keys#9017
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:fix/menu-arrow-keys-empty-options

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

LexicalMenu's arrow key handlers return true unconditionally, including on the path where the body never ran:

KEY_ARROW_DOWN_COMMAND,
payload => {
  const event = payload;
  if (options !== null && options.length) {
    ...
    event.preventDefault();
    event.stopImmediatePropagation();
  }
  return true;   // <- also returned when there are no options
},

true means the command was handled and must stop propagating. With an empty option list nothing is highlighted, nothing is scrolled, preventDefault is not even called — the key is simply eaten, and every lower-priority KEY_ARROW_UP/DOWN_COMMAND handler is skipped.

That state is not hypothetical, and it is invisible. defaultMenuRenderFn renders null when there are no options — the existing test in this package is named "should render nothing when options array is empty" — while LexicalTypeaheadMenuPlugin and LexicalNodeMenuPlugin both render <LexicalMenu options={options} .../> gated only on resolution !== null, never on options.length. So a trigger that is still matching but whose query filters everything out (async results not yet in, no match, a menuRenderFn that filters) leaves a resolved menu with zero options showing nothing. Typing / and then a query that matches no component in the playground's component picker is exactly this. Arrow Up/Down then do nothing at all: the caret will not move out of the line, and rich-text decorator/node-selection navigation and table arrow handling never see the key.

The two sibling handlers registered in the same mergeRegister already get this right:

KEY_TAB_COMMAND,
payload => {
  ...
  if (options === null || selectedIndex === null || options[selectedIndex] == null) {
    return false;
  }

This makes the arrow handlers agree with them: bail out with false when there is nothing to move through, and keep returning true on every path that actually moves the selection. The inner if (!option) branch, which consumes the key after resetting the index, is unchanged.

The diff is larger than it reads because dropping the wrapping if re-indents the body; git diff -w is 12 insertions / 9 deletions.

Test plan

New unit test packages/lexical-react/src/__tests__/unit/LexicalMenuEmptyOptions.test.tsx (a separate file, so as not to collide with the a11y work in flight on LexicalMenu.test.tsx). For each of ArrowUp/ArrowDown it asserts both directions: the key propagates when there are no options, and is still consumed when there are — the second pair guards against over-narrowing and passes before and after.

Before

 ❯ packages/lexical-react/src/__tests__/unit/LexicalMenuEmptyOptions.test.tsx (4 tests | 2 failed)
   × lets ArrowDown through when there are no options
     AssertionError: expected true to be false // Object.is equality
   × lets ArrowUp through when there are no options
     AssertionError: expected true to be false // Object.is equality
   ✓ still consumes ArrowDown when there are options
   ✓ still consumes ArrowUp when there are options

 Test Files  1 failed (1)
      Tests  2 failed | 2 passed (4)

After

 Test Files  1 passed (1)
      Tests  4 passed (4)

Package suite is unchanged:

$ npx vitest run packages/lexical-react
 Test Files  32 passed (32)
      Tests  186 passed (186)

…e arrow keys

## Description

`LexicalMenu`'s arrow key handlers return `true` unconditionally, including on the path where the body never ran:

```tsx
KEY_ARROW_DOWN_COMMAND,
payload => {
  const event = payload;
  if (options !== null && options.length) {
    ...
    event.preventDefault();
    event.stopImmediatePropagation();
  }
  return true;   // <- also returned when there are no options
},
```

`true` means the command was handled and must stop propagating. With an empty option list nothing is highlighted, nothing is scrolled, `preventDefault` is not even called — the key is simply eaten, and every lower-priority `KEY_ARROW_UP/DOWN_COMMAND` handler is skipped.

That state is not hypothetical, and it is invisible. `defaultMenuRenderFn` renders `null` when there are no options — the existing test in this package is named *"should render nothing when options array is empty"* — while `LexicalTypeaheadMenuPlugin` and `LexicalNodeMenuPlugin` both render `<LexicalMenu options={options} .../>` gated only on `resolution !== null`, never on `options.length`. So a trigger that is still matching but whose query filters everything out (async results not yet in, no match, a `menuRenderFn` that filters) leaves a resolved menu with zero options showing nothing. Typing `/` and then a query that matches no component in the playground's component picker is exactly this. Arrow Up/Down then do nothing at all: the caret will not move out of the line, and rich-text decorator/node-selection navigation and table arrow handling never see the key.

The two sibling handlers registered in the same `mergeRegister` already get this right:

```tsx
KEY_TAB_COMMAND,
payload => {
  ...
  if (options === null || selectedIndex === null || options[selectedIndex] == null) {
    return false;
  }
```

This makes the arrow handlers agree with them: bail out with `false` when there is nothing to move through, and keep returning `true` on every path that actually moves the selection. The inner `if (!option)` branch, which consumes the key after resetting the index, is unchanged.

The diff is larger than it reads because dropping the wrapping `if` re-indents the body; `git diff -w` is 12 insertions / 9 deletions.

## Test plan

New unit test `packages/lexical-react/src/__tests__/unit/LexicalMenuEmptyOptions.test.tsx` (a separate file, so as not to collide with the a11y work in flight on `LexicalMenu.test.tsx`). For each of ArrowUp/ArrowDown it asserts both directions: the key propagates when there are no options, and is still consumed when there are — the second pair guards against over-narrowing and passes before and after.

### Before

```
 ❯ packages/lexical-react/src/__tests__/unit/LexicalMenuEmptyOptions.test.tsx (4 tests | 2 failed)
   × lets ArrowDown through when there are no options
     AssertionError: expected true to be false // Object.is equality
   × lets ArrowUp through when there are no options
     AssertionError: expected true to be false // Object.is equality
   ✓ still consumes ArrowDown when there are options
   ✓ still consumes ArrowUp when there are options

 Test Files  1 failed (1)
      Tests  2 failed | 2 passed (4)
```

### After

```
 Test Files  1 passed (1)
      Tests  4 passed (4)
```

Package suite is unchanged:

```
$ npx vitest run packages/lexical-react
 Test Files  32 passed (32)
      Tests  186 passed (186)
```
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 9, 2026
@LeSingh1

Copy link
Copy Markdown
Contributor Author

Consolidated into #9051 with the other PRs that share this defect, per @etrepum's note on #9027 and @mayrang's on #9035. Same fix and same tests, one review.

@LeSingh1 LeSingh1 closed this Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant