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

fix: only start at block with removing if not an inline node #4791

Merged
merged 1 commit into from
May 9, 2024

Conversation

PHillemans
Copy link

@PHillemans PHillemans commented Jan 16, 2024

Please describe your changes

In the pull request: #4341 (originally: #3859), a fix was implemented for HorizontalRule. This made sure that for this extension/node, which is a block node, it would replace the whole block. This lead to problems in my implementation where I use an inline node. This is why I decided to, instead of overwriting the inputrule handing, fixing the issue in this PR.

How did you accomplish your changes

I've looked into previous versions of @tiptap/core to see where this issue first occured, and back-tracked it to the PR's mentioned above. It seemed like this change was made taking into account existing nodes in the TipTap repository, which use NodeInputRule and are block.
From what I could figure out, I don't think you want the replacement of the whole block to happen in case of an inline node, which is why I decided to create this PR.

How have you tested your changes

I've been testing this change in the example of the HorizontalRule, but also creating a small inline node. This confirmed my suspisions, as it seemed like it now supported both only adding and removing the node, but still also supported for block nodes to replace the whole block.

How can we verify your changes

Use the demo environment from the HorizontalRule, and see if this does still work like expected. Then create a small node like

Node.create({
 name: 'variable',

 group: 'inline',
 inline: true,
 atom: true,

 // Parse attributes
 addAttributes() {
   return {
     value: { },
   }
 },
 // Render replacementField in html
 renderHTML({ node }) {
   return ['span', {}, `|${node.attrs.value}|`]
 },

 addInputRules() {
   return [
     nodeInputRule({
       find: /\{\{\d\}\}/,
       type: this.type,
       getAttributes: node => {
         return {
           value: node.input,
         }
       },
     }),
   ]
 },
})

and add that as well. From this you should see that you can add the inline node on the same line, but still keep the HorizontalRule replacing the whole block.

Remarks

Tried to do everything as would be expected, but this is honestly my first time contributing to an open source project. I've tried to look if I needed to add tests somewhere, but couldn't find an already existing test file for this, so didn't add them for now. Please let me know if you need any more information.

Checklist

  • The changes are not breaking the editor
  • Added tests where possible
  • Followed the guidelines
  • Fixed linting issues

Related issues

#3859
#4341
#4796

Copy link

netlify bot commented Jan 16, 2024

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit df2921c
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/65a6a0bba14ed80008dad27a
😎 Deploy Preview https://deploy-preview-4791--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@muteor
Copy link

muteor commented Feb 25, 2024

@bdbch Any chance to get this into an upcoming release?

@muteor
Copy link

muteor commented May 9, 2024

@svenadlung @bdbch any way to push this forward?

@bdbch bdbch merged commit fb2b1c0 into ueberdosis:develop May 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants