Use mark element to indicate highlighted lines, ensure formatting when styles omitted, add Wrap Lines option#80
Conversation
* Use CSS hack to force empty cell to show instead of line break * Prefix class names
| } | ||
|
|
||
| .hljs.shcb-line-numbers .shcb-loc > span { | ||
| padding-left: 0.75em; |
There was a problem hiding this comment.
Humm. I'm not seeing line wrapping at all anymore.
There was a problem hiding this comment.
I think we may want to address #78 (comment) at this point. If we add white-space: pre-wrap to each line, then we'll get the wrapping behavior and adding display: table-cell to the span will fix the indentation.
diff --git a/style.css b/style.css
index 14033b3..5597986 100644
--- a/style.css
+++ b/style.css
@@ -5,6 +5,7 @@
.hljs.shcb-code-table > .shcb-loc {
display: table-row;
+ white-space: pre-wrap;
width: 100%;
}
@@ -18,6 +19,7 @@
}
.hljs.shcb-line-numbers .shcb-loc > span {
+ display: table-cell;
padding-left: 0.75em;
}
There was a problem hiding this comment.
At the moment, I'm having a hard time getting the <code> to respect the <pre>'s padding. This is because when I use display: table, it will disregard a parent's padding for overflow and only respect the parent's boundary. Resulting in this behavior:
If you're open to changing our output from <pre><code> to <pre><div><code>, then that will let us have wrapping and overflow enabled very easily. The extra div would work because it will be a display: block, meaning it will respect the <pre>'s padding and if we add an overflow to the div, then <code> (a display: table) will respect the div's boundary. Thoughts?
(p.s. and if I'm not making sense, it's because I'm sleepy and can write up a better explanation in the morning 😅)
There was a problem hiding this comment.
I'm not against adding another div wrapper! Please amend the PR with what you have in mind.
There was a problem hiding this comment.
Take a look at a503c31 and let me know what you think. Improvements welcome!
|
If we do introduce the line wrapping option (which I just added to show how it'd work), I'm not sure how we want to handle that in Gutenberg. Do we want to modify the |
|
In regards to the wrapping I can see that regular block editor does line wrapping in the editor: But on the frontend no wrapping is done: Ah, but this is in Twenty Twenty. In Twenty Nineteen there is line wrapping on the frontend: So the inconsistent line wrapping seems like an inconsistency in the themes. Therefore, if we add a line wrapping option we'd need not only have it add line wrapping, but also to force line wrapping to be removed if not enabled. |
|
@allejo your changes fix the line wrapping in Twenty Nineteen: And in Twenty Twenty (which does not do line wrapping on the frontend), it also looks good: |
|
@allejo The Wrap Lines checkbox logic seems to be inverted: vs: |
Yes, I suppose so? |
Also, supply default values and proper types for attributes.
I just implemented this using CSS in b4c09ce. Maybe there's a better way. Also fixed the inversion I noted in #80 (comment). This is feeling solid. |
I'm liking this implementation! Thanks for the fix |
allejo
left a comment
There was a problem hiding this comment.
This feels pretty solid to me!
This comment has been minimized.
This comment has been minimized.
|
I broke it in 28a8c20 somehow. |
|
|
||
| .hljs.shcb-code-table { | ||
| display: table; | ||
| width: 100%; |
There was a problem hiding this comment.
@allejo Is the width:100% required? I'm finding in some themes, like Twenty Fourteen, it causes a bit of horizontal scrolling even when line wrapping is enabled. Can we remove it?
| width: 100%; |
There was a problem hiding this comment.
I found the problem with Twenty Fourteen: the .hljs element wasn't getting box-sizing:border-box, so the combination of width:100% and the padding resulted in an overflow. Fixed in e236851.









Fixes #79.
Fixes #78.
markelement to indicate highlighted lines.With styles (note the lack of highlights and extra line breaks):
Before without styles:
After without styles (with line spacing and highlights preserved):
Note this is in the Twenty Twenty theme which has a default style of: