[lexical][lexical-playground] Feature: Improve focus management in Toolbar and adds SKIP_SELECTION_FOCUS_TAG by KaiPrince · Pull Request #7804 · facebook/lexical

KaiPrince · 2025-09-08T23:22:32Z

Description

In this PR, I am addressing the issue of focus being moved to the editor when a toolbar button is activated with the keyboard, as reported in #6006.

I have made the following behavioural changes:

Focus outlines are visible on the toolbar buttons
When activating a toolbar button (either with space or enter), the focus remains on the button
When activating a toolbar dropdown item, the focus returns to the dropdown opener button

This PR partially addresses #6006.

This PR does not address the behaviour of modals such as the insert image button, or the trap created by the tab intent plugin.

SKIP_SELECTION_FOCUS_TAG

Normally when an editor.update reconciles its selection to the DOM, the focus is moved to the rootElement if it does not already have focus. In order to ensure that the DOM selection changes but the focus remains on the button, a new SKIP_SELECTION_FOCUS_TAG has been added which is more fine-grained than SKIP_DOM_SELECTION which avoids changing the DOM selection or focus.

Test plan

Before

Toolbar button:

Before	After

Dropdown button:

Before	After

After

Toolbar button:

Before	After

Dropdown button:

Before	After

vercel · 2025-09-08T23:22:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
lexical	Ready	Preview	Comment	Sep 30, 2025 5:08pm
lexical-playground	Ready	Preview	Comment	Sep 30, 2025 5:08pm

meta-cla · 2025-09-08T23:22:38Z

Hi @KaiPrince!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

KaiPrince · 2025-09-08T23:29:24Z

packages/lexical-playground/src/index.css

 }

+.toolbar .toolbar-item:focus {
+  outline: 2px solid rgb(60, 132, 244);


I couldn't find a standardized focus colour, so I copied this colour from

lexical/packages/lexical-playground/src/themes/PlaygroundEditorTheme.css

Line 585 in 9571a9e

outline: 2px solid rgb(60, 132, 244);

packages/lexical-playground/src/ui/DropDown.tsx

KaiPrince · 2025-09-08T23:34:58Z

packages/lexical-playground/src/ui/DropDown.tsx

+    if (key === 'Escape') {
+      onClose();
+    }


Moved this above the (!items) check, because some dropdowns, e.g. the colour picker don't have any buttons registered, but should still be closable.

KaiPrince · 2025-09-08T23:35:39Z

packages/lexical-playground/src/ui/DropDown.tsx

      highlightedItem.current.focus();
+    } else if (!items) {
+      if (dropDownRef && dropDownRef.current) {
+        focusNearestDescendant(dropDownRef.current);


For colour pickers, etc. we want to focus the first field.

KaiPrince · 2025-09-08T23:36:28Z

packages/lexical-playground/src/utils/focusUtils.ts

Created some utility functions for later reuse. Alternatively, could have just added a dependency to a library like tabbable

meta-cla · 2025-09-09T00:05:23Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

etrepum

It looks like this is failing a lot of tests

KaiPrince · 2025-09-09T20:47:35Z

Hmm, yes.. I'll look into it. I'm betting it's just the tests themselves that need to be updated.

KaiPrince · 2025-09-09T21:06:15Z

Nope I take that back it's not the tests it's me.

KaiPrince

Assuming I haven't missed anything significant, I'll look into adding tests next.

KaiPrince · 2025-09-25T08:42:40Z

packages/lexical-playground/src/ui/ColorPicker.tsx

-  useEffect(() => {
-    // Check if the dropdown is actually active
-    if (innerDivRef.current !== null && onChange) {
-      onChange(selfColor.hex, skipAddingToHistoryStack);
-      setInputColor(selfColor.hex);
-    }
-  }, [selfColor, onChange]);


I removed this because it seems to go against React guidelines on useEffect: https://react.dev/learn/you-might-not-need-an-effect#sharing-logic-between-event-handlers. I've replaced it with individual calls in each event handler.

As a side effect, this also fixed the odd behaviour that selecting text and then opening the color picker splits the text node even if you don't change the text color.

packages/lexical-playground/src/ui/DropDown.tsx

KaiPrince · 2025-09-25T08:48:57Z

packages/lexical-playground/src/ui/ColorPicker.tsx

-  useEffect(() => {
-    if (color === undefined) {
-      return;
-    }
-    const newColor = transformColor('hex', color);
    setSelfColor(newColor);
    setInputColor(newColor.hex);
-  }, [color]);


Similarly, this seems to go against React's recommendations: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

etrepum · 2025-09-25T18:35:21Z

npm run ci-check is failing:

Error: packages/lexical-playground/src/ui/DropDown.tsx(281,13): error TS2322: Type 'RefObject<HTMLDivElement | null>' is not assignable to type 'RefObject<HTMLDivElement>'.
  Type 'HTMLDivElement | null' is not assignable to type 'HTMLDivElement'.
    Type 'null' is not assignable to type 'HTMLDivElement'.
ERROR: "tsc" exited with 2.

https://github.com/facebook/lexical/actions/runs/18002165768/job/51264652086?pr=7804

KaiPrince · 2025-09-25T20:12:52Z

Huh. I'm not seeing that issue locally. Let me investigate

etrepum · 2025-09-25T22:59:06Z

packages/lexical-playground/src/ui/DropDown.tsx

@@ -253,7 +276,10 @@ export default function DropDown({

      {showDropDown &&
        createPortal(
-          <DropDownItems dropDownRef={dropDownRef} onClose={handleClose}>
+          <DropDownItems
+            dropDownRef={dropDownRef as React.RefObject<HTMLDivElement>}


You shouldn't need to add a cast, casts are unsafe. The correct place to fix this are the types themselves. This is a direct consequence of the Ref vs. RefObject thing you keep trying to fix but was actually correct before and now it isn't.

type Ref<T> = RefCallback<T> | RefObject<T | null> | null;

The definition of RefCallback contains the internal implementation details that you keep running away from, but using Ref and RefCallback as-is are perfectly correct and is not something that needs to be avoided or "fixed".

Hmm yes I'm not pleased with using a cast here either. Based on observation the casted type seemed to be correct, but now that you pasted the type definition, it seems like I have the wrong version of react installed because I get a different type def:

Could you link me the source for the definition you pasted?

Oh and just to clarify I'm not trying to run away from RefCallback, I'm just trying to best utilize the type system by narrowing the acceptable types to what we actually would expect. Having a stricter interface keeps the implementation simple since we don't have to add behaviour to handle unexpected inputs. e.g. DropDownItems is an internal component of Dropdown, which we know only uses RefObject and never RefCallback, so by being more strict on the input type we don't need to add a check for handling RefCallback, leading to simpler code.

I'll try resetting my node_modules and running npm ci. In the meantime, adjusting the prop type to match the version you pasted fixes it in CI: KaiPrince@3cd1c6c

If you still feel strongly that I should revert it back to just Ref, let me know.

packages/lexical/src/LexicalUpdateTags.ts

etrepum

I think this looks good, great work and nice job cleaning up the inconsistencies with React best practices as well. Will merge assuming that the e2e suite is still passing.

KaiPrince added 7 commits September 8, 2025 15:53

add toolbar focus outline

6792a40

fix: don't move focus when activating toolbar buttons

ecedffa

Add more focus outlines

1639bb4

More focus management on toolbar

5a1e4a5

Add focus utility functions

ce5c99b

Add focus management to dropdowns

25d5afc

Update focus color

ee0b8e5

KaiPrince requested review from Fetz, Sahejkm, acywatson, fantactuka, ivailop7, potatowagon, takuyakanbr and zurfyx as code owners September 8, 2025 23:22

vercel bot deployed to Preview – lexical September 8, 2025 23:23 View deployment

vercel bot deployed to Preview – lexical-playground September 8, 2025 23:24 View deployment

KaiPrince commented Sep 8, 2025

View reviewed changes

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 Sep 9, 2025

etrepum reviewed Sep 9, 2025

View reviewed changes

replace dom selection tag with focus tag

c042e3e

vercel bot deployed to Preview – lexical September 16, 2025 21:51 View deployment

vercel bot deployed to Preview – lexical-playground September 16, 2025 21:52 View deployment

check for input method

ed088c5

vercel bot deployed to Preview – lexical September 17, 2025 01:20 View deployment

vercel bot deployed to Preview – lexical September 25, 2025 08:37 View deployment

vercel bot deployed to Preview – lexical-playground September 25, 2025 08:39 View deployment

remove unused var

7312665

vercel bot deployed to Preview – lexical September 25, 2025 08:47 View deployment

vercel bot deployed to Preview – lexical-playground September 25, 2025 08:48 View deployment

fix typo

823b977

vercel bot deployed to Preview – lexical September 25, 2025 08:51 View deployment

KaiPrince commented Sep 25, 2025

View reviewed changes

vercel bot deployed to Preview – lexical-playground September 25, 2025 08:52 View deployment

fix type mismatch

368719f

vercel bot deployed to Preview – lexical September 25, 2025 22:30 View deployment

vercel bot deployed to Preview – lexical-playground September 25, 2025 22:31 View deployment

etrepum reviewed Sep 25, 2025

View reviewed changes

alternative fix for dropdownRef type

7caf697

vercel bot deployed to Preview – lexical September 26, 2025 00:09 View deployment

vercel bot deployed to Preview – lexical-playground September 26, 2025 00:10 View deployment

etrepum reviewed Sep 30, 2025

View reviewed changes

packages/lexical/src/LexicalUpdateTags.ts Outdated Show resolved Hide resolved

Update packages/lexical/src/LexicalUpdateTags.ts

d8cf25c

vercel bot had a problem deploying to Preview – lexical September 30, 2025 17:04 Failure

Merge branch 'main' into kprince/issue-6006-keyboard-a11y

778796f

vercel bot deployed to Preview – lexical September 30, 2025 17:05 View deployment

etrepum changed the title ~~[lexical-playground] Feature: Improve focus management in Toolbar~~ [lexical][lexical-playground] Feature: Improve focus management in Toolbar and adds SKIP_SELECTION_FOCUS_TAG Sep 30, 2025

vercel bot deployed to Preview – lexical-playground September 30, 2025 17:08 View deployment

etrepum approved these changes Sep 30, 2025

View reviewed changes

etrepum added this pull request to the merge queue Sep 30, 2025

Merged via the queue into facebook:main with commit a42c89b Sep 30, 2025
66 of 67 checks passed

etrepum mentioned this pull request Oct 1, 2025

v0.36.2 #7894

Merged

Conversation

KaiPrince commented Sep 8, 2025 • edited by etrepum Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

SKIP_SELECTION_FOCUS_TAG

Test plan

Before

After

Uh oh!

vercel bot commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meta-cla bot commented Sep 8, 2025

Action Required

Process

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

meta-cla bot commented Sep 9, 2025

Uh oh!

etrepum left a comment

Choose a reason for hiding this comment

Uh oh!

KaiPrince commented Sep 9, 2025

Uh oh!

KaiPrince commented Sep 9, 2025

Uh oh!

KaiPrince left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etrepum commented Sep 25, 2025

Uh oh!

KaiPrince commented Sep 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

etrepum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

KaiPrince commented Sep 8, 2025 •

edited by etrepum

Loading

vercel bot commented Sep 8, 2025 •

edited

Loading