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-playground][lexical-react] Feature: Push Draggable Element to Parent #7338

Merged
merged 7 commits into from
Mar 17, 2025

Conversation

sescandell
Copy link
Contributor

Description

Allows passing a function to receive the DOM element identified by the DraggableBlockPlugin.

An example application of this is the addition of a "+" button that allows the creation of a block above the identified one. This feature is particularly useful in the playground, as it allows adding a new block at the top of the page without having to drag and drop, for example.

Test plan

Before

image

After

image

Copy link

vercel bot commented Mar 15, 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 Mar 17, 2025 8:43pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 17, 2025 8:43pm

@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 Mar 15, 2025
Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation looks good but I think it might make sense to mirror Notion's UI and have the default behavior for this button be insert after rather than insert before.

Screenshot 2025-03-15 at 21 50 24

@ivailop7
Copy link
Collaborator

+1 on Bob's comment for "insert after" I'd even go as far and say it even makes sense to render the component picker menu for the user to select any widget, not just a paragraph.

@sescandell
Copy link
Contributor Author

This implementation looks good but I think it might make sense to mirror Notion's UI and have the default behavior for this button be insert after rather than insert before.

I just pushed an update based on your comment. I replicated Notion's behavior: by default, the new node is inserted after the current one. If you press the Cmd (or Ctrl) key while clicking, it inserts the new node before the current one.

+1 on Bob's comment for "insert after" I'd even go as far and say it even makes sense to render the component picker menu for the user to select any widget, not just a paragraph.

I completely agree! However, there isn't a ready-to-use "component picker" at the moment. The existing one is tightly coupled with the typeahead plugin, so extracting it or duplicating some of its code would be necessary.

I'm not sure how to easily create a "menu" within the playground next to my button. Maybe we could tackle this in a separate PR?

@etrepum
Copy link
Collaborator

etrepum commented Mar 17, 2025

I agree that it makes sense to handle the dialog in a separate PR. It is basically the same interface as the / typeahead if following notion's UX. The block paragraph is created eagerly anyway, so the menu would just be a state change that happens after this behavior.

Any reason why you chose cmd-click instead of opt-click like Notion?

@sescandell
Copy link
Contributor Author

Any reason why you chose cmd-click instead of opt-click like Notion?

I just used the same as in FloatingLinkEditorPlugin: https://github.com/facebook/lexical/blob/main/packages/lexical-playground/src/plugins/FloatingLinkEditorPlugin/index.tsx#L369
I can change it. What is "Option-Click"?

@etrepum
Copy link
Collaborator

etrepum commented Mar 17, 2025

Option is the Mac equivalent of the Alt key https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/altKey

@ivailop7
Copy link
Collaborator

Can I pitch shift+click on Windows, which always made more sense to me for such actions.

@etrepum
Copy link
Collaborator

etrepum commented Mar 17, 2025

We should maybe do whatever notion does by default since that UI is inspired by it. I don’t have a windows machine around to see what the keybinding is there

@sescandell
Copy link
Contributor Author

sescandell commented Mar 17, 2025

Notion is using the AltKey on windows... which seems... unnatural for me. But I'm not a big user of notion.
image

I changed for the option command. Let me know if you want to change for the AltKey on Windows

@sescandell
Copy link
Contributor Author

@etrepum Sorry... careless mistake. Thanks for the correction!

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice job

@etrepum etrepum added this pull request to the merge queue Mar 17, 2025
Merged via the queue into facebook:main with commit 0c2875e Mar 17, 2025
39 checks passed
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. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants