Skip to content

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

Merged
zbeyens merged 18 commits intoudecode:mainfrom
archie9211:main
Oct 10, 2023
Merged

Delete forward on empty line before code block unwraps code line #2686#2687
zbeyens merged 18 commits intoudecode:mainfrom
archie9211:main

Conversation

@archie9211
Copy link
Copy Markdown
Contributor

@archie9211 archie9211 commented Oct 8, 2023 •
edited
Loading

/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
Copy Markdown

changeset-bot bot commented Oct 8, 2023 •
edited
Loading

🦋 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
Copy Markdown

vercel bot commented Oct 8, 2023 •
edited
Loading

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

@algora-pbc algora-pbc bot mentioned this pull request Oct 8, 2023
Closed
@reviewpad

This comment was marked as resolved.

Sign in to view
@reviewpad reviewpad bot added the small Pull request is small label Oct 8, 2023
@12joan

This comment was marked as resolved.

Sign in to view
12joan
12joan requested changes Oct 8, 2023
View reviewed changes
@zbeyens
Copy link
Copy Markdown
Member

zbeyens commented Oct 8, 2023 •
edited
Loading

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.

Sign in to view
@archie9211

This comment was marked as resolved.

Sign in to view
@archie9211

This comment was marked as resolved.

Sign in to view
@12joan

This comment was marked as resolved.

Sign in to view
@12joan

This comment was marked as resolved.

Sign in to view
@12joan

This comment was marked as resolved.

Sign in to view
@archie9211

This comment was marked as resolved.

Sign in to view
@12joan

This comment was marked as resolved.

Sign in to view
@12joan
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

archie9211 commented Oct 8, 2023

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.

Sign in to view
zbeyens
zbeyens reviewed Oct 8, 2023
View reviewed changes
@zbeyens
Copy link
Copy Markdown
Member

zbeyens commented Oct 8, 2023 •
edited
Loading

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
zbeyens
zbeyens reviewed Oct 10, 2023
View reviewed changes
zbeyens
zbeyens reviewed Oct 10, 2023
View reviewed changes
zbeyens
zbeyens reviewed Oct 10, 2023
View reviewed changes
@archie9211
Copy link
Copy Markdown
Contributor Author

archie9211 commented Oct 10, 2023

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

archie9211
archie9211 commented Oct 10, 2023
View reviewed changes
12joan
12joan approved these changes Oct 10, 2023
View reviewed changes
Copy link
Copy Markdown
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
Delete forward on empty line before code block unwraps code line #2686
Closed
@zbeyens zbeyens merged commit 6caf98b into udecode:main Oct 10, 2023
@zbeyens zbeyens mentioned this pull request Oct 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@12joan 12joan 12joan approved these changes

@zbeyens zbeyens zbeyens approved these changes

Assignees

No one assigned

Labels

🙋 Bounty claim medium Pull request is medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Delete forward on empty line before code block unwraps code line

3 participants

@archie9211 @12joan @zbeyens