Problem/Motivation

At the moment whenever a piece of code is wrapped in a code block the library is automatically used.

I don't think that's desirable.

You would want to have it fired on code block (pre code). But you also want to be able to add inline code like this in a text where you don't want to use this library.

Steps to reproduce

Add code blocks with and without pre around it and you'll notice the library is always fired.

Proposed resolution

Alter the JS so it's only fired on pre code.

Issue fork highlight_js-3550427

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

f0ns created an issue. See original summary.

f0ns’s picture

f0ns commented
Issue summary: View changes
f0ns’s picture

f0ns commented
Issue summary: View changes
f0ns’s picture

f0ns commented
Issue summary: View changes

f0ns opened merge request !5

f0ns’s picture

f0ns commented
Status: Active » Needs review
f0ns’s picture

f0ns commented
Issue summary: View changes
f0ns’s picture

f0ns commented
Issue summary: View changes
f0ns’s picture

f0ns commented

You can see it in action on my live site here:

https://webhaven.io/documentation/theming/theme-structure

sujan shrestha’s picture

sujan shrestha commented

Hi Fons, thanks for the suggestion. i see the issue you’re pointing out, and i agree it would be better if the library only fires on code blocks wrapped in pre tags. i will review the JS and make the necessary changes to ensure that inline code doesn’t trigger the library.

f0ns’s picture

f0ns commented

Thank you!

It's a minor but important change.

All the best, Fons

f0ns’s picture

f0ns commented
Status: Needs review » Reviewed & tested by the community
f0ns’s picture

f0ns commented

I have it running on 3 websites in production and all is well (it's also a minor change) so I've changed the status to reviewed & tested.

Would love to get this change in so I can get rid of the patches.

Thanks in advance!

norman.lol’s picture

norman.lol
he/him
Primary language German
Location Berlin
commented
Version: 1.1.6 » 1.1.x-dev
Status: Reviewed & tested by the community » Closed (duplicate)
Related issues: +#3550427: The highlight js code runs on all code blocks

The change in the patch only does one thing: it prevents that the "Copy" functionality is getting added to standalone <code> blocks.

The highlighting itself is added via hljs.highlightAll() which by default runs only on pre code, see https://highlightjs.readthedocs.io/en/latest/api.html.

The whole code in the current JS is problematic because it partially gets executed multiple times. First via once(.., 'code', ..).foreach and inside it via $('code').each(function (i, block) { again.

In #3556145: Prevent code to be executed multiple times we got the feedback from this issue included and the duplicate execution fixed, please review over there.

  • Wrap all <code> elements for the copy functionality to work
  • Only overflow-x on the <pre><code> elements (inline <code> elements without <pre> parent must not have it)
  • Only syntax highlight on the <pre><code> elements (inline <code> elements without <pre> parent must not have it)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

norman.lol’s picture

norman.lol
he/him
Primary language German
Location Berlin
commented
Related issues: -#3550427: The highlight js code runs on all code blocks +#3556145: Prevent code to be executed multiple times
norman.lol’s picture

norman.lol
he/him
Primary language German
Location Berlin
commented

If possible for you to take a quick look at #3556145: Prevent code to be executed multiple times, your opinion would be much appreciated. If you like the new approach or if you would still prefer a new option to prevent the highlighting and copy functionality kick in on inline code (would be really easy to implement). But maybe the new approach actually fixes the problem already for you..

Chime in when you can 🙂

f0ns’s picture

f0ns commented

Just had a look and looks like a good solution.

Thank you for taking the time to come up with it.

When it’s pushed to a release I’ll update my projects and the SDC components where needed to go with the new flow.

Thank you, if anything would occur I’ll let you know. 🙏

norman.lol’s picture

norman.lol
he/him
Primary language German
Location Berlin
commented

You're welcome! ☺️

Yes, that would be great if you could keep us in the loop when anything looks odd after the update in your project(s).

f0ns’s picture

f0ns commented

Just installed the update and altered my SDC component a little.

Works perfectly now.

You can see the code container and the inline code container in action on this page:

https://webhaven.io/documentation/theming/theme-structure

Thanks!