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

[lexical-utils] Bug Fix: Validate decorator node is not isolated in needsBlockCursor #7108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

churichard
Copy link

Description

Currently, when a decorator block node is isolated and at the beginning of the document, you can still place the block cursor next to it. This leads to behavior where the cursor can appear to be "stuck" next to the node even if you are trying to select text elsewhere (since the real cursor is transparent). This is especially evident when testing on iOS/Android.

This PR fixes this issue by preventing the fake block cursor from appearing next to an isolated node.

Test plan

  1. Add the following code to YouTubeNode.tsx to make it an isolated node (there isn't a good one to test with by default):
isIsolated(): boolean {
  return true;
}
  1. Open Lexical playground
  2. Add a YouTube video and move it up to the beginning of the document
  3. Select the video
  4. Press Up arrow key

Before

The block cursor is getting placed above the YouTube video:

before.mov

After

The block cursor is no longer being placed. Instead, the cursor moves back down

after.mov

Copy link

vercel bot commented Jan 28, 2025

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

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2025 7:58pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 28, 2025 7:58pm

@facebook-github-bot facebook-github-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 Jan 28, 2025
Copy link

size-limit report 📦

Path Size
lexical - cjs 29.05 KB (-0.07% 🔽)
lexical - esm 28.88 KB (+0.08% 🔺)
@lexical/rich-text - cjs 38.04 KB (-0.02% 🔽)
@lexical/rich-text - esm 30.97 KB (+0.18% 🔺)
@lexical/plain-text - cjs 36.6 KB (+0.13% 🔺)
@lexical/plain-text - esm 28.2 KB (-0.1% 🔽)
@lexical/react - cjs 39.87 KB (+0.08% 🔺)
@lexical/react - esm 32.29 KB (+0.03% 🔺)

@etrepum
Copy link
Collaborator

etrepum commented Jan 28, 2025

For what it's worth, there are a lot of edge cases with isolated decorator nodes especially around selections and they have no tests. They are probably best avoided.

@ivailop7
Copy link
Collaborator

I don't think moving the cursor back down is the right UX, having it above the decorator node is fine. It should still allow you to type and insert a new paragraph above the decorator.

@etrepum
Copy link
Collaborator

etrepum commented Jan 29, 2025

isolated decorator nodes, are supposed to be sort of outside of the normal editing flow. StickyNode is the only example in the playground that tries to be an isolated decorator node, they are always appended to the root (you wouldn't know where they really are in the document due to their absolutely positioned layout) and you're not supposed to be able to select them as part of the document.

They work about as well as you'd expect a complicated feature with no documentation and no tests to work.

@churichard
Copy link
Author

I'm open to alternative solutions; using an isolated node is not strictly necessary but I thought it might be the easiest way to achieve what I'm looking for.

For context, I have a DecoratorBlockNode that should be fixed to the top of the document - the user shouldn't be able to add text next to or above it. Unfortunately the block cursor appears next to it by default, so it would be ideal if I could make it so that doesn't happen.

@etrepum
Copy link
Collaborator

etrepum commented Jan 29, 2025

I think the only straightforward solution is to have nested editors, either:

ReadonlyEditor

  • DecoratorBlockNode
  • NestedWritableEditor

or

ReadonlyEditor

  • DecoratorBlockNode
    • NestedWritableEditor (implemented inside the DecoratorBlockNode)

You might have to implement your own NestedLexicalComposer to do this because IIRC it forces the propagation of the readOnly state from the parent to the child. It's fairly simple to copy/paste/delete to make that work though, and/or you could submit a PR to allow that to be overridden.

The single editor solution would be to find the list of commands to listen for and to fix up the selection (SELECTION_CHANGE_COMMAND) and trap other events (DELETE_CHARACTER_COMMAND, probably other things) that would cause the block node to be affected.

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.

4 participants