Skip to content

Remove blank line under a 'none' content block will remove 'none' block together #605

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

Closed
1 task
tcper opened this issue Mar 5, 2024 · 5 comments · Fixed by #1505
Closed
1 task

Remove blank line under a 'none' content block will remove 'none' block together #605

tcper opened this issue Mar 5, 2024 · 5 comments · Fixed by #1505
Labels
bug Something isn't working prio:mid Medium priority

Comments

@tcper
Copy link
Contributor

tcper commented Mar 5, 2024

Describe the bug
When I need to remove a blank line under a 'none' type content, it never works, remove the 'none' content together.
I think remove a blank line is reasonable action.
20240305-161541

const blockConfig = {
  type: "image" as const,
  propSchema: imagePropSchema,
  content: "none",
} satisfies CustomBlockConfig;

I tried, all content: "none", will cause this bug.

To Reproduce

  1. Insert a 'none' content block, like 'image'
  2. Insert a blank line under the 'none' content block
  3. Press Backspace in the blank line

Misc

  • Node version:
  • Package manager:
  • Browser:
  • I'm a sponsor and would appreciate if you could look into this sooner than later 💖
@tcper tcper added the bug Something isn't working label Mar 5, 2024
@matthewlipski matthewlipski added the prio:mid Medium priority label Jun 11, 2024
@sb8244
Copy link

sb8244 commented Aug 12, 2024

Is the solution here to check if the block is paragraph, has no content, and focus is start of block (should be impossible to meet these criteria and not meet this one). If that matches, then delete the block rather than merging it?

@matthewlipski
Copy link
Collaborator

matthewlipski commented Aug 27, 2024

Yeah that sounds right - we should already have quite some backspace handling logic here for that, but seems like this case isn't accounted for

@sb8244
Copy link

sb8244 commented Aug 27, 2024

@matthewlipski I'm not sure any of the code blocks there handle that cases this. At least when I looked into it, I came to that conclusion.

I wrote some custom keydown handlers (on BlockNoteView) that handle this case and another. One big thing here is pulling the children from the block to the prevBlock:

if (e.key === "Backspace") {
          const selection = blockNoteEditor.getSelection()
          const position = blockNoteEditor.getTextCursorPosition()

          if (selection === undefined && position && position.block.type === "paragraph") {
            const block = position.block
            const blockPos = getBlockInfoFromPos(
              blockNoteEditor._tiptapEditor.state.doc,
              blockNoteEditor._tiptapEditor.state.selection.anchor
            )

            const isTopBlock = blockPos.depth === 2
            const isEmptyBlock = block.content.length === 0 && block.children.length === 0
            const isEmptyContentWithChildren = block.content.length === 0 && block.children.length > 0

            if (isTopBlock && isEmptyBlock) {
              e.stopPropagation()
              e.preventDefault()
              blockNoteEditor.removeBlocks([block])

              if (position.prevBlock) {
                blockNoteEditor.setTextCursorPosition(position.prevBlock, "end")
              }
            } else if (isTopBlock && position.prevBlock && isEmptyContentWithChildren && Array.isArray(position.prevBlock.children)) {
              e.stopPropagation()
              e.preventDefault()
              blockNoteEditor.updateBlock(position.prevBlock, { children: position.prevBlock.children.concat(block.children) })
              blockNoteEditor.removeBlocks([block])

              if (position.prevBlock) {
                blockNoteEditor.setTextCursorPosition(position.prevBlock, "end")
              }
            }
          }
        }

@matthewlipski
Copy link
Collaborator

Hmm have you tried modifying the code from BNMergeBlocks? I think if you just remove the .replace here, you should get pretty close to the correct behaviour. The difference is that BNMergeBlocks will just un-nest the children of the later block, rather than attaching them to the previous one which might still be a dealbreaker. Either way I think it's smth you might want to look into

@lamtuanamc
Copy link

lamtuanamc commented Nov 25, 2024

Hi @matthewlipski, I've created a pull request to resolve this issue. Could you please review it at your convenience? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working prio:mid Medium priority
Projects
None yet
4 participants