Skip to content

Wysiwyg: preserves line feeds in code block mode #3246

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

Merged

Conversation

bourdaisj
Copy link
Contributor

Fix #3200

@ssddanbrown ssddanbrown added this to the BookStack v21.12.5 milestone Feb 6, 2022
ssddanbrown added a commit that referenced this pull request Feb 6, 2022
- Updated code content to get specific text selection instead of using
  node-based handling which could return the whole document when
  multiple top-level nodes were in selection.
- Simplified how code gets applied into the page to not be node based
  but use native editor methods to replace the selection. Allows
  creation from half-way through a block.

Tested on chrome+Firefox on Fedora 35.
Builds upon changes in #3246.
For #3200.
@ssddanbrown ssddanbrown merged commit 049d6ba into BookStackApp:development Feb 6, 2022
@ssddanbrown
Copy link
Member

Thank you once again @Julesdevops!

This change was definitely an improvement to preserve the newlines.
Unfortunately it alone would not fully address the issue in #3200.

When the selection spanned across multiple top-level nodes, inserting a code block would attempt to (and breakingly) convert the whole page into a code block. You would also see all text within the code content editor popup. This was due to my usages of getNode() within the below lines:

const providedCode = editor.selection.getNode().textContent;
window.components.first('code-editor').open(providedCode, '', (code, lang) => {
const wrap = document.createElement('div');
wrap.innerHTML = `<pre><code class="language-${lang}"></code></pre>`;
wrap.querySelector('code').innerText = code;
editor.formatter.toggle('pre');
const node = editor.selection.getNode();
editor.dom.setHTML(node, wrap.querySelector('pre').innerHTML);
editor.fire('SetContent');

When multiple top-level nodes are selected this would resolve to the shared parent body, hence we would fetch all text from those and attempt to replace the whole body with code on code-block-save.

I found a handy editor.selection.getContent({format: 'text'}) method which is smart and provides just the selection, even if terminating part-way through a block.

I followed up your changes with b2f863e which uses this method and simplifies the strange stuff I was doing to set the code block, to make the selection replacement more accurate.

@bourdaisj
Copy link
Contributor Author

Indeed a far better solution! thanks for following up on this

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

Successfully merging this pull request may close these issues.

[Bug Report]: selecting multiple lines to make them code block
2 participants