Use mark element to indicate highlighted lines, ensure formatting when styles omitted, add Wrap Lines option #80

westonruter · 2020-04-22T23:06:00Z

Fixes #79.
Fixes #78.

Use mark element to indicate highlighted lines.
Prefix class names.
Ensure Code block renders properly when no styles are loaded.
Prevent flash of unstyled content by printing stylesheet before first Code block.
Add Wrap Lines option, and ensure do not wrap by default for consistency.
Improve code style.

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:

mark {
	background-color: #eee;
	color: #222;
}

* Use CSS hack to force empty cell to show instead of line break * Prefix class names

allejo · 2020-04-22T23:13:32Z

style.css

+}
+
+.hljs.shcb-line-numbers .shcb-loc > span {
 	padding-left: 0.75em;


Add this 0de9d10 and you'll knock out #78 also

Humm. I'm not seeing line wrapping at all anymore.

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; }

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 😅)

I'm not against adding another div wrapper! Please amend the PR with what you have in mind.

Take a look at a503c31 and let me know what you think. Improvements welcome!

style.css

allejo · 2020-04-23T17:44:22Z

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 PlainText's attributes to make it nowrap?

westonruter · 2020-04-26T00:16:37Z

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.

westonruter · 2020-04-26T00:18:46Z

@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:

westonruter · 2020-04-26T00:29:25Z

@allejo The Wrap Lines checkbox logic seems to be inverted:

vs:

westonruter · 2020-04-26T00:33:59Z

I'm not sure how we want to handle that in Gutenberg. Do we want to modify the PlainText's attributes to make it nowrap?

Yes, I suppose so?

Also, supply default values and proper types for attributes.

westonruter · 2020-04-26T00:59:20Z

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 PlainText's attributes to make it nowrap?

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.

allejo · 2020-04-26T01:21:14Z

I just implemented this using CSS in b4c09ce. Maybe there's a better way.

I'm liking this implementation! Thanks for the fix

allejo

This feels pretty solid to me!

… Code block

westonruter · 2020-04-26T07:23:59Z

I broke it in 28a8c20 somehow.

…Twenty Nineteen

westonruter · 2020-04-26T07:50:51Z

style.css

+
+.hljs.shcb-code-table {
 	display: table;
 	width: 100%;


@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?

Suggested change

width: 100%;

Humm. Seems like it is needed. If line numbers are turned off and the contents don't cause any wrapping, then the table is not wide enough:

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.

…sing overflow

Use mark element to indicate highlighted lines

68eccc3

* Use CSS hack to force empty cell to show instead of line break * Prefix class names

allejo reviewed Apr 22, 2020

View reviewed changes

style.css Outdated Show resolved Hide resolved

westonruter and others added 3 commits April 22, 2020 22:33

Move line break inside of span to elimiante CSS hack and fix selection

297d469

Fix horizontal scrolling + add line wrapping option

a503c31

Make lines of code consistently table cells

7efa9d3

allejo and others added 6 commits April 23, 2020 16:46

Mirror line wrapping of code blocks in Gutenberg

6b4a8f2

Fix styling lint issue

ae1dacd

Fix prettier version in lock file after a bad install

d3067f0

Install prettier as direct devDependency

0b8d2d1

Fix prettier issues in Gruntfile.js

ec99051

Fix prettier issues in index.js

5ae1e3e

westonruter closed this Apr 26, 2020

westonruter reopened this Apr 26, 2020

Ensure line wrapping applied (or prevented) on frontend and editor.

b4c09ce

Also, supply default values and proper types for attributes.

allejo approved these changes Apr 26, 2020

View reviewed changes

westonruter added 2 commits April 25, 2020 22:55

Prevent flash of unstyled content by printing stylesheet before first…

28a8c20

… Code block

Discontinue using hasBlockSupport since unclear arguments

54b1db7

This comment has been minimized.

Sign in to view

westonruter added 3 commits April 26, 2020 00:25

Fix injection of styles

ff6fc58

Fix placement of returned styles

7d5d58f

Revise insertion of inline styles now that style is printed earlier

21fb49c

Increase specificity and adjust selector for white-space to override …

a5571f4

…Twenty Nineteen

westonruter commented Apr 26, 2020

View reviewed changes

westonruter added 3 commits April 26, 2020 01:04

Remove extraneous semicolon

ce28e25

Use version as specified in script assets

6ea40e4

Ensure box-sizing:border-box for .hljs to prevent width:100% from cau…

e236851

…sing overflow

westonruter changed the title ~~Use mark element to indicate highlighted lines~~ Use mark element to indicate highlighted lines, ensure formatting when styles omitted, add Wrap Lines option Apr 26, 2020

westonruter merged commit ede8daa into master Apr 26, 2020

westonruter deleted the use/mark branch April 26, 2020 19:37

allejo mentioned this pull request Feb 9, 2021

HTML validation errors #280

Closed

Use mark element to indicate highlighted lines, ensure formatting when styles omitted, add Wrap Lines option #80

Use mark element to indicate highlighted lines, ensure formatting when styles omitted, add Wrap Lines option #80

Uh oh!

Conversation

westonruter commented Apr 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allejo Apr 22, 2020

Choose a reason for hiding this comment

Uh oh!

westonruter Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

allejo Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allejo Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

allejo Apr 23, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

allejo commented Apr 23, 2020

Uh oh!

westonruter commented Apr 26, 2020

Uh oh!

westonruter commented Apr 26, 2020

Uh oh!

westonruter commented Apr 26, 2020

Uh oh!

westonruter commented Apr 26, 2020

Uh oh!

westonruter commented Apr 26, 2020

Uh oh!

allejo commented Apr 26, 2020

Uh oh!

allejo left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

westonruter commented Apr 26, 2020

Uh oh!

westonruter Apr 26, 2020

Choose a reason for hiding this comment

Uh oh!

westonruter Apr 26, 2020

Choose a reason for hiding this comment

Uh oh!

westonruter Apr 26, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

westonruter commented Apr 22, 2020 •

edited

Loading

allejo Apr 23, 2020 •

edited

Loading

allejo Apr 23, 2020 •

edited

Loading