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

Delete forward on empty line before code block unwraps code line #2686 #2687

Merged
merged 18 commits into from
Oct 10, 2023

Conversation

archie9211
Copy link
Contributor

@archie9211 archie9211 commented Oct 8, 2023

/claim #2686

Idea of implemention was to delete the current line if its empty and move the cursor to next line without impacting the next block (be it code block or listing or anything)

I have also handled a case when empty line is the first line of the page, in this case, delete the current line and move the cursor to next line.

Behavior after fix:

2023-10-08_07-45-44.mp4

@changeset-bot
Copy link

changeset-bot bot commented Oct 8, 2023

🦋 Changeset detected

Latest commit: 33f4129

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@udecode/plate-select Minor
@udecode/plate Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 8, 2023

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

Name Status Preview Comments Updated (UTC)
plate ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2023 4:43pm

@reviewpad

This comment was marked as resolved.

@12joan

This comment was marked as resolved.

packages/paragraph/src/withParagraph.ts Outdated Show resolved Hide resolved
packages/paragraph/src/withParagraph.ts Outdated Show resolved Hide resolved
@zbeyens
Copy link
Member

zbeyens commented Oct 8, 2023

I think Notion's behavior is the expected one for many consumers:

2023-10-08.at.09.42.20.mp4

It resets the type of the next (nested) block then let the default merge to be applied.

So this should not be done in the paragraph plugin. I'd create another plugin with query?: QueryNodeOptions; (like createSelectOnBackspacePlugin) to apply this behavior on any block type.

@12joan

This comment was marked as resolved.

@archie9211

This comment was marked as resolved.

@archie9211

This comment was marked as resolved.

@12joan

This comment was marked as resolved.

@12joan

This comment was marked as resolved.

@12joan

This comment was marked as resolved.

@archie9211

This comment was marked as resolved.

@12joan

This comment was marked as resolved.

@12joan
Copy link
Collaborator

12joan commented Oct 8, 2023

Would you be able to rename this plugin createDeletePlugin? Keeping it in /packages/select is fine.

To summarise:

  • The plugin should change the behaviour of delete forward to remove the current block if the selection is collapsed and the current block is empty
  • By default, it should be active for all block types, but there should be a query option so that consumers can disable it in certain circumstances
  • In the playground, can you use the query option to only apply the plugin when inside paragraph elements?
  • Can you add tests to verify that the plugin is behaving as expected?

@archie9211
Copy link
Contributor Author

Hi @12joan,

I have one doubt,

there is a file customizer-items.ts

and referenced plugin is registered here (KEY_SELECT_ON_BACKSPACE)

CONTRIBUTING.md seems outdated for plugins, is my plugin needed to be registered in same way in this file?

image

@archie9211

This comment was marked as resolved.

packages/select/src/withCreateRemoveOnDeleteForward.ts Outdated Show resolved Hide resolved
packages/select/src/createRemoveOnDeleteForwardPlugin.ts Outdated Show resolved Hide resolved
apps/www/src/registry/default/example/playground-demo.tsx Outdated Show resolved Hide resolved
.changeset/shy-billo-bagga.md Outdated Show resolved Hide resolved
packages/select/src/withCreateRemoveOnDeleteForward.ts Outdated Show resolved Hide resolved
packages/select/src/createRemoveOnDeleteForwardPlugin.ts Outdated Show resolved Hide resolved
packages/select/src/withCreateRemoveOnDeleteForward.ts Outdated Show resolved Hide resolved
@zbeyens
Copy link
Member

zbeyens commented Oct 8, 2023

As commented in the review, this plugin can be enabled on all blocks by default (condition: !query).

@reviewpad reviewpad bot removed the small Pull request is small label Oct 8, 2023
@archie9211
Copy link
Contributor Author

Hi @12joan @zbeyens I just verified that expected behavior has stopped working after the changed from db1c9bd commit.

Copy link
Collaborator

@12joan 12joan left a comment

Choose a reason for hiding this comment

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

Thanks for all your hard work. I've awarded the bounty.

@zbeyens Everything looks good on my end. If you're satisfied with the fixes, I think we're ready to merge?

@12joan 12joan linked an issue Oct 10, 2023 that may be closed by this pull request
@zbeyens zbeyens merged commit 6caf98b into udecode:main Oct 10, 2023
7 checks passed
@zbeyens zbeyens mentioned this pull request Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 Bounty claim medium Pull request is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete forward on empty line before code block unwraps code line
3 participants