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: replace the whole node in nodeInputRule #3859

Closed
wants to merge 1 commit into from

Conversation

ngryman
Copy link
Contributor

@ngryman ngryman commented Mar 17, 2023

Hi,

This PR fixes some issues with the horizontal rule:

The new behavior creates a new paragraph below the horizontal rule, instead of above.

Implementation

I changed the implementation of nodeInputRule for the case when there's no group matched in the input rule regexp.

As far as I understand, the previous implementation was trying to replace the content of the existing node, not the node itself. We need to include the node boundaries as well to do so: start - 1 and end + 1.

The new implementation first inserts the new node at the existing node position (start - 1) and then delete the remaining content. Let me know if that makes sense or if there's a better approach.

Grepping the codebase quickly, it seems that other usages of nodeInputRule always use a matching group, so this change should only affect the HorizontalRule extension.

@bdbch bdbch added this to the 2.1.0 Release milestone Mar 28, 2023
@bdbch bdbch added Status: Review Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap. labels Mar 29, 2023
@bdbch bdbch self-assigned this Mar 29, 2023
@chrisdeeming
Copy link

Anyone wishing to shim this to fix the HorizontalRule issue can use this extension:

const ExtHorizontalRule = HorizontalRule.extend({
	addInputRules() {
		return [
			new InputRule({
				find: /^(?:---|—-|___\s|\*\*\*\s)$/,
				handler: ({ state, range, match }) => {
					const attributes = {};

					const { tr } = state
					const start = range.from
					let end = range.to

					tr.insert(start - 1, this.type.create(attributes)).delete(
						tr.mapping.map(start),
						tr.mapping.map(end),
					);
				}
			})
		]
	}
});

@steven-tey
Copy link

@chrisdeeming you're a lifesaver – that worked! Thank you!

@rfgamaral
Copy link
Contributor

FYI, Tiptap v2.1 was meant to fix this issue (there was some work done in that regard here), but I'm finding that the change on this PR works much better, more details in this comment.

@bdbch
Copy link
Member

bdbch commented Aug 17, 2023

Thanks for letting me know about the patch @rfgamaral – I'll check the functionality of it and if it works fine replace my changes with it.

@bdbch
Copy link
Member

bdbch commented Aug 17, 2023

I merged this fix with develop on this new PR here where your commit is included @ngryman:
#4341

I'll close this PR in favor for the new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Open Source The issue or pull reuqest is related to the open source packages of Tiptap.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants