desktop: fix issues with codeblock toggling #6435
Conversation
| let hasParagraph = false; | ||
|
|
||
| state.doc.nodesBetween(from, to, (node) => { | ||
| if (node.type.name === "paragraph") { | ||
| hasParagraph = true; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Instead of checking whether selection has paragraph, we can just check whether a codeblock node is active using isNodeActive function.
There was a problem hiding this comment.
Are you saying that the selected text should only be converted if there are no active codeblock nodes?
I wonder if the behavior should be to convert all selected text to a codeblock, if any one of the selected nodes is of a type other than codeblock?
There was a problem hiding this comment.
Are you saying that the selected text should only be converted if there are no active codeblock nodes?
Yes. Because if the cursor is inside a codeblock we'll just convert it into a paragraph. Otherwise, everything gets converted into a codeblock.
| return commands.insertContent({ | ||
| type: this.name, | ||
| attrs: { | ||
| ...attributes, | ||
| id: createCodeblockId() | ||
| }, | ||
| content: [ | ||
| { | ||
| type: "text", | ||
| text: state.doc.textBetween(from, to, "\n") | ||
| } | ||
| ] | ||
| }); |
There was a problem hiding this comment.
This can be simplified like so:
return commands.insertContent(
this.type.create(
{ ...attributes, id: createCodeblockId() },
state.schema.text(state.doc.textBetween(from, to, "\n"))
)
);However, I don't recommend depending on insertContent for replacing the selection. Instead we can directly use the replaceSelectionWith method on Transaction:
const { from, to } = tr.selection;
const text = tr.doc.textBetween(from, to, "\n");
tr.replaceSelectionWith(
this.type.create(
{ ...attributes, id: createCodeblockId() },
state.schema.text(text)
)
);Not to mention, that after replacing the node we also need to set the selection like so:
commands.setTextSelection({ from, to: tr.mapping.map(to) });This is all very rough so you might need to make adjustments.
There was a problem hiding this comment.
Sounds good, just changed this
No problem! I think using the transactions directly solved this issue. Also made the tests a bit more thorough to make sure there's no other nodes other than the codeblock |
|
@seanwynwins I think you didn't push the changes. |
Oh I didn't push yet because I wanted to check on this-did you have any thoughts?: |
Just got back from vacation, will push tomorrow |
seanwynwins
force-pushed
the
togglecodeblockrevision
branch
from
696c913
to
3fe5352
Compare
|
Sorry for the delay, can you review? @thecodrr |
seanwynwins
force-pushed
the
togglecodeblockrevision
branch
3 times, most recently
from
8e0774e
to
4822005
Compare
Signed-off-by: Sean-Wyn Ng <[email protected]>
seanwynwins
force-pushed
the
togglecodeblockrevision
branch
from
4822005
to
d4a1abb
Compare
|
Thanks! |
Addresses #6321