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

IndentPlugin stops FOCUS_COMMAND propagation in Lexical #10049

Open
rolandsaven opened this issue Dec 18, 2024 · 3 comments · May be fixed by #10051 or #10128
Open

IndentPlugin stops FOCUS_COMMAND propagation in Lexical #10049

rolandsaven opened this issue Dec 18, 2024 · 3 comments · May be fixed by #10051 or #10128
Assignees
Labels
plugin: richtext-lexical @payloadcms/richtext-lexical

Comments

@rolandsaven
Copy link

Describe the Bug

The recently introduced command listener by IndentPlugin stops the propagation of the FOCUS_COMMAND in Lexical.

This means that only plugins that register their FOCUS_COMMAND listeners at the COMMAND_PRIORITY_NORMAL level or above will work.

  1. COMMAND_PRIORITY_EDITOR
  2. COMMAND_PRIORITY_LOW
  3. COMMAND_PRIORITY_NORMAL
  4. COMMAND_PRIORITY_HIGH
  5. COMMAND_PRIORITY_CRITICAL

I recommend returning true from the registered listener instead of false. This way we allow propagation down to other handler.

Listeners that run at a higher priority can "intercept" commands and prevent them from propagating to other handlers by returning true. - according to Lexical Docs

Link to the code that reproduces this issue

https://github.com/rolandsaven/payload-lexical-indent-plugin

Reproduction Steps

Use this branch to test. I've created a plugin that register logging listeners and fails to log at a certain level here -> https://github.com/rolandsaven/payload-lexical-indent-plugin/blob/main/src/lexical/features/focus/plugin.tsx#L20

  1. git clone [email protected]:rolandsaven/payload-lexical-indent-plugin.git (comes with SQLITE db)
  2. cd payload-lexical-indent-plugin
  3. pnpm install
  4. pnpm run dev
  5. Open http://localhost:3000/admin (usr: [email protected] pass: test)
  6. Open Developer Tools and try creating a new Media
  7. See logs in Console while triggering focus on editor

Image

Which area(s) are affected? (Select all that apply)

plugin: richtext-lexical

Environment Info

Binaries:
  Node: 20.11.1
  npm: 10.2.4
  Yarn: 1.22.22
  pnpm: 9.11.0
Relevant Packages:
  payload: 3.8.0
  next: 15.1.0
  @payloadcms/email-nodemailer: 3.8.0
  @payloadcms/graphql: 3.8.0
  @payloadcms/next/utilities: 3.8.0
  @payloadcms/payload-cloud: 3.8.0
  @payloadcms/richtext-lexical: 3.8.0
  @payloadcms/translations: 3.8.0
  @payloadcms/ui/shared: 3.8.0
  react: 19.0.0
  react-dom: 19.0.0
Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 24.1.0: Thu Oct 10 21:02:27 PDT 2024; root:xnu-11215.41.3~2/RELEASE_X86_64
  Available memory (MB): 16384
  Available CPU cores: 12
@rolandsaven rolandsaven added status: needs-triage Possible bug which hasn't been reproduced yet validate-reproduction labels Dec 18, 2024
@AlessioGr AlessioGr added the plugin: richtext-lexical @payloadcms/richtext-lexical label Dec 18, 2024
@github-actions github-actions bot removed the status: needs-triage Possible bug which hasn't been reproduced yet label Dec 18, 2024
@GermanJablo
Copy link
Contributor

GermanJablo commented Dec 19, 2024

I recommend returning true from the registered listener instead of false.

I guess you meant the opposite.

The thing is that if we do that and then we add Firefox to playwright config which is where bug #8654 happened:

projects: [
   { name: "chromium", use: { ...devices["Desktop Chrome"] } },
   { name: "firefox", use: { ...devices["Desktop Firefox"] } },
],

and you run the test that introduced that PR in Firefox, it will fail:

pnpm dev fields
cd test
npx playwright test -g "ensure escape key can be used to move focus away from editor" --project="firefox"; 

So either we make people register the command with a priority higher than NORMAL, or we leave that "bug" in Firefox.

Honestly, I'm inclined to undo the above PR. It seems more Firefox's responsibility than ours, it makes our code more complex and limits what users can do. Also, there are other places where indent doesn't allow you to escape the editor anyway as explained in that PR.

What do you think?

@rilrom
Copy link
Contributor

rilrom commented Dec 20, 2024

I originally created that PR and I'm not against the idea of reverting it, I do agree it adds complexity.

The reverted functionality can be revisited more broadly when we attempt to fix the trapped focus in the other parts of the editor that affect all browsers through the use of the toggle indent idea.

@GermanJablo
Copy link
Contributor

Doing that!
Thanks @rolandsaven and @rilrom!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: richtext-lexical @payloadcms/richtext-lexical
Projects
None yet
4 participants