Add support for highlighting specific lines #20

allejo · 2019-11-23T06:42:34Z

Here's my proof of concept for highlighting specific lines. Do you have any thoughts on whether or not to proceed with something like this?

Closes #12

westonruter · 2019-11-28T00:22:41Z

FYI: I'm not going to have a chance to look at this until mid-December.

allejo · 2019-11-28T02:43:51Z

No worries! I'll leave any major further work on this PR until then

allejo · 2019-12-31T01:11:44Z

Gonna gently nudge you on this one, @westonruter

src/index.js

westonruter · 2020-01-07T18:23:23Z

syntax-highlighting-code-block.php

@@ -238,7 +238,7 @@ function render_block( $attributes, $content ) {
 	$transient_key = 'syntax-highlighting-code-block-' . md5( $attributes['showLines'] . $attributes['language'] . $matches['code'] ) . '-v' . PLUGIN_VERSION;
 	$highlighted   = get_transient( $transient_key );

-	if ( $highlighted && isset( $highlighted['code'] ) ) {
+	if ( ! DEVELOPMENT_MODE && $highlighted && isset( $highlighted['code'] ) ) {


Why the ! DEVELOPMENT_MODE check?

I might have been doing something wrong but I believe I kept getting cached results while I was developing on the plugin. So I figured I'd disable transients in dev mode. Is there a better way of handling this?

westonruter · 2020-01-07T18:25:55Z

block-styles.css

+.code-block-bg .loc.highlighted {
+	background: rgba(86, 213, 255, 0.2);
+}


This would also need to get added to the frontend inline styles, correct?

I'm thinking of having separate styles for the frontend vs the editor. Reason being is the frontend handles highlighting lines differently from what the editor is doing. Where in the editor we need to "style" a textarea, the frontend has a lot of dedicated divs we can style.

src/index.js

block-styles.css

allejo · 2020-02-17T01:35:08Z

Just as an update, I'll be getting back to this in about a week or so

allejo · 2020-02-23T03:03:39Z

How should we handle the color for highlighting lines? On the frontend, there can be light or dark themes, we can't just select an arbitrary color since it might conflict with the background of the selected theme. Should the color be an option in Customizer?

westonruter · 2020-02-24T06:16:48Z

How should we handle the color for highlighting lines? On the frontend, there can be light or dark themes, we can't just select an arbitrary color since it might conflict with the background of the selected theme. Should the color be an option in Customizer?

Could a default highlight color be selected for each color scheme as part of highlight.php? Could this be automated perhaps by iterating over all of the styles, obtaining the background-color and color, and somehow computing a highlight color? At very least, could there be one default highlight color for dark scheme and another for a light scheme? Perhaps a light yellow for a light style and a dark yellow for a dark style? A Customizer setting to override this could also be provided, but there should be some good default.

allejo · 2020-02-27T06:38:03Z

Could a default highlight color be selected for each color scheme as part of highlight.php? Could this be automated perhaps by iterating over all of the styles , obtaining the background-color and color, and somehow computing a highlight color?

This is definitely doable but I wouldn't want to modify the distributed CSS files directly because I'm not sure what would be the best way to introduce those classes since line numbers aren't officially part of the highlight.js scope. Do you have any suggestions?

westonruter · 2020-02-28T04:35:11Z

This is definitely doable but I wouldn't want to modify the distributed CSS files directly because I'm not sure what would be the best way to introduce those classes since line numbers aren't officially part of the highlight.js scope. Do you have any suggestions?

Oh yeah, I didn't mean necessarily that the existing CSS files should be modified. What about adding additional CSS files to contain the rules for highlighting? Either that, or a JSON file that contains what background color should be used for each? In either case, the WordPress plugin here could either add the CSS file as an inline style or it could read the JSON file to output the necessary CSS.

westonruter · 2020-02-29T13:07:00Z

I was curious actually how browsers handle highlighting with the mark element.

It turns out, the Chrome's default stylesheet has this:

mark {
    background-color: yellow;
    color: black;
}

So in a light color scheme, it gets rendered as:

And in a dark color scheme, it looks like:

Nevertheless, would this work for adding highlights to syntax-highlighted text which has variable colors set for the text? I suspect not.

allejo · 2020-03-01T04:55:17Z

I think the default styles for <mark> works for <mark> because all of the text is a solid color. I would agree with you that I don't think this would work for themes where some keywords could be yellow-ish. I think we could have the following behavior:

For a light theme, we can be a nice pastel color: yellow or blue
For a dark theme, take the background color and lighten it up; similar to how an IDE dark mode looks for a selected line

What are your thoughts?

westonruter · 2020-03-02T19:23:45Z

Yes, that sounds good to me.

allejo · 2020-03-03T07:31:27Z

I've still got some things to work out on the Customizer settings, like to update colors based on theme changes.

Otherwise, this is my proposal for this feature. What are your thoughts? Too complicated? Too many color calculations?

westonruter · 2020-03-03T23:09:34Z

I just tried testing this but I got an error:

PHP Notice:  Undefined index: theme_name in /app/public/content/plugins/syntax-highlighting-code-block/syntax-highlighting-code-block.php on line 132
PHP Fatal error:  Uncaught DomainException: There is no stylesheet by the name of '' in /app/public/content/plugins/syntax-highlighting-code-block/vendor/scrivo/highlight.php/HighlightUtilities/_themeColors.php:475
Stack trace:
#0 /app/public/content/plugins/syntax-highlighting-code-block/vendor/scrivo/highlight.php/HighlightUtilities/functions.php(91): _getThemeBackgroundColor(NULL)
#1 /app/public/content/plugins/syntax-highlighting-code-block/syntax-highlighting-code-block.php(110): HighlightUtilities\getThemeBackgroundColor(NULL)
#2 /app/public/content/plugins/syntax-highlighting-code-block/syntax-highlighting-code-block.php(132): Syntax_Highlighting_Code_Block\get_default_line_bg_color(NULL)
#3 /app/public/content/plugins/syntax-highlighting-code-block/syntax-highlighting-code-block.php(146): Syntax_Highlighting_Code_Block\get_options()
#4 /app/public/content/plugins/syntax-highlighting-code-block/syntax-highlighting-code-block.php(268): Syntax_Highlighting_Code_Block\get_option('theme_name')
#5 /app/public/core-dev/src/wp in /app/public/content/plugins/syntax-highlighting-code-block/vendor/scrivo/highlight.php/HighlightUtilities/_themeColors.php on line 475

(...if there is a filter available)

allejo · 2020-03-04T05:48:27Z

Welp, sorry about that. I hadn't tested my changes without any plugin options in the database. Time for round 2!

westonruter · 2020-03-05T02:07:32Z

I'm noticing that the highlight lines aren't appearing to line up properly in the editor, at least in the Twenty Twenty theme:

src/index.js

allejo · 2020-04-19T06:29:33Z

No worries about the delay! What WP + highlight.js theme are you using to test? I can't replicate the lack of line highlighting or the increased line-height with the "Twenty" WP themes and highlight.js' default theme.

I did, however, catch this bug in the Twenty Seventeen theme where blank lines don't seem to be respected when the plug-in is enabled. I'll take a look at the CSS for that.

westonruter · 2020-04-20T06:18:46Z

I'm testing specifically in Twenty Twenty.

I think I see the problem. The highlights showed up as soon as I checked “Show Line Numbers”. (The line spacing issue also went away.)

As soon as I re-unchecked the highlights went away.

I believe I've largely fixed this in 4d381e9.

However, the line height issue is still present on the frontend in Twenty Twenty (when the line numbers are turned off):

allejo · 2020-04-20T07:58:14Z

I believe I've largely fixed this in 4d381e9.

Thanks for the catch!

For the line-height issue, can you confirm that your code DOM node has a class of selected-lines and that class has display: table? I made a change to line-numbers.css in 7b02b94 that made code blocks with either line numbers or selected lines to be a display: table.

When I make that code block display: block (the default style), this is what I get. It looks like your screenshots, but I'm not sure if it's the same cause.

The styles are being injected inline, so I don't think it'd be a caching problem. I'll set up a clean install of WP and the plugin tomorrow and try replicating it.

Edit: The length of the highlighted lines are different in my screenshot than yours, so I don't think this is the same problem.

westonruter · 2020-04-20T16:45:28Z

The class is present on code:

However, the display is block not table:

That seems to reveal the issue, namely that this rule is contained in line-numbers.css which is not loaded when selected lines are only supplied:

.hljs.line-numbers,
.hljs.selected-lines {
	display: table;
	width: 100%;
}

So do we need a separate selected-lines.css file that we add independently?

allejo · 2020-04-21T05:26:21Z

OH! I see why I couldn't replicate it! My test page has blocks of all types, including blocks with line numbers, which is why that CSS was being loaded 🤦

So do we need a separate selected-lines.css file that we add independently?

For the very little CSS this plugin is injecting from line-numbers.css, I don't think it's worth having a separate file (585 bytes currently vs 121 bytes with a separate file). I would lean more towards just modifying the logic to include that CSS file if either line numbers are used or lines are highlighted.

westonruter · 2020-04-21T14:45:40Z

That sounds good to me.

westonruter

This is all working now!

If my latest changes all work for you too, I'll merge.

allejo · 2020-04-21T20:59:54Z

Your changes worked fine for me! 👍

Here are a few minor changes that I had overlooked.

The gap between line numbers and code with highlights is gone now

Empty new lines can now be selected. Previously, there were not being rendered.

westonruter · 2020-04-21T21:23:50Z

Empty new lines can now be selected. Previously, there were not being rendered.

I'm not seeing this issue… or any difference between 0b57cc5 and 5ccac85.

I'm trying with this code, having selected lines 3,5:

function foo() {
   var first = 1;

   var second = 2;
      
   var third = 3;
}

Editor:

Frontend:

westonruter · 2020-04-21T21:27:22Z

Oh, I'm also noticing a discrepancy in the highlighted lines between the editor and the frontend (using selected lines 2-4,5):

allejo · 2020-04-21T21:32:58Z

Oh, I'm also noticing a discrepancy in the highlighted lines between the editor and the frontend (using selected lines 2-4,5):

Fixed in 51076ef

allejo · 2020-04-21T21:36:08Z

Empty new lines can now be selected. Previously, there were not being rendered.

I'm not seeing this issue… or any difference between 0b57cc5 and 5ccac85.

I'm trying with this code, having selected lines 3,5:

Ah sorry, instead of JavaScript, try doing some PHP. It's whenever a code block has a sublanguage (PHP is a sub lang of XML according to highlight.js) that newlines aren't rendered correctly.

westonruter · 2020-04-22T00:53:14Z

Ah sorry, instead of JavaScript, try doing some PHP. It's whenever a code block has a sublanguage (PHP is a sub lang of XML according to highlight.js) that newlines aren't rendered correctly.

Humm. Could you share the raw post content for that Code block so I can see what you're seeing? I tried selecting PHP but I didn't see anything amiss about selecting newlines.

allejo · 2020-04-22T01:56:55Z

Sure! Here's my code snippet

<?php

echo "hello world";

Screenshots of running 0b57cc5:

Gutenberg

Line number options

Rendering

westonruter · 2020-04-22T02:15:50Z

Thank you. I was able to reproduce the problem and the fix 👍

westonruter reviewed Jan 7, 2020

View reviewed changes

allejo added 3 commits February 22, 2020 16:55

Always ignore transient in dev mode

53dc41c

Begin support for highlighting specific lines

e1d06f2

Dedupe handling of selected lines

49cde13

allejo mentioned this pull request Feb 29, 2020

Introduce new getThemeBackgroundColor utility function scrivo/highlight.php#69

Merged

allejo added 2 commits March 2, 2020 20:50

Update highlight deps to 9.18.1

c37bf8c

Add Customizer + default colors for line highlighting

7b02b94

allejo added 2 commits March 2, 2020 23:33

Add missing 'selected lines' value from transient

ab49e3b

Add Gutenberg styles to dist build script

8b515de

allejo added 5 commits March 3, 2020 21:43

Fix "dark theme" detection

c80d583

Fix default line color when no theme is selected

97bc07c

Add a filter to set the selected line bg color

9b17856

Hide selected line color selector from Customizer

f3860a0

(...if there is a filter available)

Add uninstall process to remove options

929018c

Improve formatting of uninstall.php

81e18e1

westonruter reviewed Apr 19, 2020

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

Don't recreate parseSelectedLines on each edit

cf5b3f2

Ensure highlight styles are added when line numbers are not shown

4d381e9

westonruter and others added 3 commits April 19, 2020 23:36

Improve storage of transient and usage of attributes

8b33686

Remove redundant variables

44d9150

Render explicit new lines when line features are used

52691e8

westonruter added 2 commits April 21, 2020 12:09

Include inline style consistently

559cade

Fix stylesheet file reference

0b57cc5

westonruter approved these changes Apr 21, 2020

View reviewed changes

allejo added 2 commits April 21, 2020 13:56

Fix weird gap in line highlight CSS

bc6c811

Fix new lines being selectable in code blocks

5ccac85

Consistently have inclusive selected line syntax

51076ef

westonruter merged commit d67de68 into westonruter:master Apr 22, 2020

allejo deleted the feature/highlighting-lines branch April 22, 2020 02:25

westonruter mentioned this pull request Apr 22, 2020

Fix compatibility with lightBlockWrapper for Code block in Gutenberg 7.9 #73

Merged

westonruter mentioned this pull request Jun 27, 2023

Defect: Horizontal scrollbars are not shown. #732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for highlighting specific lines #20

Add support for highlighting specific lines #20

allejo commented Nov 23, 2019

westonruter commented Nov 28, 2019

allejo commented Nov 28, 2019

allejo commented Dec 31, 2019

westonruter Jan 7, 2020

allejo Feb 23, 2020

westonruter Jan 7, 2020

allejo Feb 23, 2020

allejo commented Feb 17, 2020

allejo commented Feb 23, 2020

westonruter commented Feb 24, 2020

allejo commented Feb 27, 2020

westonruter commented Feb 28, 2020

westonruter commented Feb 29, 2020 •

edited

Loading

allejo commented Mar 1, 2020

westonruter commented Mar 2, 2020

allejo commented Mar 3, 2020

westonruter commented Mar 3, 2020

allejo commented Mar 4, 2020

westonruter commented Mar 5, 2020

allejo commented Apr 19, 2020 •

edited

Loading

westonruter commented Apr 20, 2020 •

edited

Loading

allejo commented Apr 20, 2020 •

edited

Loading

westonruter commented Apr 20, 2020

allejo commented Apr 21, 2020

westonruter commented Apr 21, 2020

westonruter left a comment

allejo commented Apr 21, 2020

westonruter commented Apr 21, 2020

westonruter commented Apr 21, 2020

allejo commented Apr 21, 2020

allejo commented Apr 21, 2020

westonruter commented Apr 22, 2020

allejo commented Apr 22, 2020

westonruter commented Apr 22, 2020

Add support for highlighting specific lines #20

Add support for highlighting specific lines #20

Conversation

allejo commented Nov 23, 2019

westonruter commented Nov 28, 2019

allejo commented Nov 28, 2019

allejo commented Dec 31, 2019

westonruter Jan 7, 2020

Choose a reason for hiding this comment

allejo Feb 23, 2020

Choose a reason for hiding this comment

westonruter Jan 7, 2020

Choose a reason for hiding this comment

allejo Feb 23, 2020

Choose a reason for hiding this comment

allejo commented Feb 17, 2020

allejo commented Feb 23, 2020

westonruter commented Feb 24, 2020

allejo commented Feb 27, 2020

westonruter commented Feb 28, 2020

westonruter commented Feb 29, 2020 • edited Loading

allejo commented Mar 1, 2020

westonruter commented Mar 2, 2020

allejo commented Mar 3, 2020

westonruter commented Mar 3, 2020

allejo commented Mar 4, 2020

westonruter commented Mar 5, 2020

allejo commented Apr 19, 2020 • edited Loading

westonruter commented Apr 20, 2020 • edited Loading

allejo commented Apr 20, 2020 • edited Loading

westonruter commented Apr 20, 2020

allejo commented Apr 21, 2020

westonruter commented Apr 21, 2020

westonruter left a comment

Choose a reason for hiding this comment

allejo commented Apr 21, 2020

westonruter commented Apr 21, 2020

westonruter commented Apr 21, 2020

allejo commented Apr 21, 2020

allejo commented Apr 21, 2020

westonruter commented Apr 22, 2020

allejo commented Apr 22, 2020

westonruter commented Apr 22, 2020

westonruter commented Feb 29, 2020 •

edited

Loading

allejo commented Apr 19, 2020 •

edited

Loading

westonruter commented Apr 20, 2020 •

edited

Loading

allejo commented Apr 20, 2020 •

edited

Loading