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

Support spec.keepAdjacent in joinForwards and joinBackwards #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tslocke
Copy link

@tslocke tslocke commented Nov 25, 2021

I don't expect this to be ready to merge - just starting the conversation.

Some questions:

Any good ideas for spec property name? I've gone with keepAdjacent but I don't love it.

Tests? I noticed there are no tests for the isolating spec property, so maybe you are OK without.

Perhaps the deleteBarrier behaviour should only be suppressed if both the before and after nodes have keepAdjacent. Right now, backspace only checks the before node, and delete the after node.

I assume a corresponding PR for prosemirror-model will be needed. It looks like this will only be to include the new spec property in the doc comment. Do you follow any protocol for keeping such things in sync across repos?

@marijnh
Copy link
Member

marijnh commented Nov 25, 2021

I think the test will have to be a bit more specific, further down in the code—deleteBarrier does a lot more than the joining that you want to turn off here, and most of that should not be turned off.

@tslocke
Copy link
Author

tslocke commented Nov 25, 2021

Thanks for the advice. This is still giving the desired behaviour, and still keeps a lot of deleteBarrier active. I'll confess I'm fumbling in the dark a bit here. It's very difficult to think through the consequences of such changes in the face of arbitrary schemas!

I did a force-push btw since if you do merge you won't want my experiments in your history. Not sure if that's the done thing or not...

@tslocke
Copy link
Author

tslocke commented Nov 25, 2021

BTW having looked more closely into this, I'm starting to think the best approach might be to make the node isolating and re-introduce the other behaviour I need with custom commands.

@marijnh
Copy link
Member

marijnh commented Nov 25, 2021

All right, I'll let this sit until you know whether it'll be something you need then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants