Improve selected text stylization #2961
Conversation
| let text = textarea.value.slice(textarea.selectionStart, textarea.selectionEnd); | ||
| let selectionStart = textarea.selectionStart; | ||
| let selectionEnd = textarea.selectionEnd; | ||
| const lines = text.split('\n'); | ||
| const undoStyle = lines.every((line) => line.startsWith(prefix) && line.endsWith(suffix)); | ||
| let prefixToUse = blockPrefix.length > 0 ? blockPrefix : prefix; |
There was a problem hiding this comment.
Maybe a falsy vs nonfalsy check would be more appropriate here? Also, shouldn't this be based on the number of lines selected?
There was a problem hiding this comment.
It's under the multilineStyle function. So this part of code will affect multiple selections only.
I can't see the advantage of using falsy check here? What could possibly go wrong when we keep it this way?
|
@askvortsov1 @luceos I couldn't find time to review change request due to my job, huge sorry for that. I see that you've merged the PR from the markdown extension (and tagged with 1.1?) but this one is still open. As you're preparing to drop a new release, I'm just here to remind you that this PR must be merged in order to make the spoiler button work properly. |
I tested it out, currently it works, but just uses many line spoilers instead of a single block spoiler. This isn't ideal, but it's also not entirely broken, so since we're practically released, we'll push this improvement to the next version. |
askvortsov1
requested review from
dsevillamartin,
davwheat,
SychO9 and
tankerkiller125
Changes proposed in this pull request:
It was using inline affix if you have block prefix but don't have block suffix.
This makes impossible to use suffix-less block syntaxes (e.g. spoiler).
It makes sense to use block affix (if there are any) for multiline selection.
It makes sense to surround multiline selection with new lines if you have block prefix but don't have block suffix.
I needed to set
surroundWithNewlinesargument totrueto achieve this behaviour on the block spoiler. But this also affected the inline spoiler and that's undesired. So I assumed that If you don't have any suffix and try to style multiple lines at once, then you probably will want to surround selection with new lines.Confirmed
Required for: