prosemirror/prosemirror-commands
1
0
Fork 0
Code Pull requests Activity

Support spec.keepAdjacent in joinForwards and joinBackwards #12

Closed
tslocke wants to merge 1 commit from keep-adjacent into master
pull from: keep-adjacent
merge into: prosemirror:master
Conversation 4 Commits 1 Files changed 1 +1
tslocke commented 2021-11-25 14:47:21 +01:00 (Migrated from github.com)
Copy link

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?

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 commented 2021-11-25 15:16:29 +01:00 (Migrated from github.com)
Copy link

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.

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 commented 2021-11-25 17:49:35 +01:00 (Migrated from github.com)
Copy link

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...

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 commented 2021-11-25 20:11:36 +01:00 (Migrated from github.com)
Copy link

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.

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 commented 2021-11-25 21:43:25 +01:00 (Migrated from github.com)
Copy link

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

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

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-commands!12
No description provided.