AleksandrHovhannisyan / eleventy-plugin-code-demo

Add interactive HTML/CSS/JS code demos to an Eleventy site using Markdown code blocks.
MIT License
11 stars 3 forks source link

Some html tags in the output are without quotes #3

Closed rocc-o closed 1 year ago

rocc-o commented 1 year ago

This plugin is working vey well. But I do have some questions, though.

In title required for accessibility Nunjucks variables do not work (but I can do without it, not so important)

How can I place allowfullscreen into iframeAttributes? It gives me EleventyConfigError allowfullscreen is not defined (via ReferenceError). Also, how to put loading="lazy"?

Entities (" < >) are displayed in the output (_site) instead of quotes, opening and closing tags. Maybe it does not matter as browsers read them anyway?

Why output is not properly minified in some lines (see those starting with /assets/img/audio-player)?

Inside img tag class=full height=160 loading=lazy src=/assets/img/audio-player/landscape-680.jpg width=680 have no quotes, or entities, at all. And this may be a problem as I need to serve a fallback jpeg image to browsers who need it, and size and lazy loading to all anyway. No quotes also on a href at the beginning: class=full and href=https://w.soundcloud.com/player

Immagine 2023-03-03 190647

AleksandrHovhannisyan commented 1 year ago

Hi again! In the future, it would really help if you could link to a minimal sandbox to reproduce the issue. This could be as simple as a bare-bones 11ty project (npm init, npm install @11ty/eleventy) with a minimal .eleventy.js config, a single source file (e.g., src/index.njk), and the plugin set up the way you want. I'll try my best to answer your questions without that, though:

In title required for accessibility Nunjucks variables do not work (but I can do without it, not so important)

Could you please share a small code sample for what you're trying to accomplish?

How can I place allowfullscreen into iframeAttributes? It gives me EleventyConfigError allowfullscreen is not defined (via ReferenceError)

You'll likely need to do allowfullscreen="true". Otherwise, Nunjucks will think you're trying to pass in a variable named allowfullscreen, which does not exist. I can clarify this better in the docs.

Entities (" < >) are displayed in the output (_site) instead of quotes, opening and closing tags. Maybe it does not matter as browsers read them anyway?

This is working as expected—those characters are being escaped as HTML entities so that they don't break the outer quotes in the HTML attribute. Are you seeing any issues where the code is not working as expected?

Or maybe this is the reason why output is not properly minified in some lines (see those starting with /assets/img/audio-player)?

This has more to do with how minify-html works. Since the code in question is inside an HTML attribute, that package has no way of knowing how to minify that particular string. For all it knows, the newline characters may very well be legitimate/valid newlines. Is this causing any issues with malformed output on the page itself?

rocc-o commented 1 year ago

Nunjuck variables in title for accessibility was my mistake, sorry.

Ok for allowfullscreen="true" globally. But if I want to pass allowfullscreen just in {% codeDemo 'Title' %} and loading="lazy" globally?

About entities, I see. There are no issues and code is working as expected. I'm going to test on devtools those with no quotes or entities. Strange that quotes/entities are not displayed at all in some parts, though. Do you think this has to do with how html-minifier works?

Anyway the output is finally minified now with your plugin, and I can make those strings in one line in the .njk file et voilà. Sorry I did not provide a sandbox.

AleksandrHovhannisyan commented 1 year ago

I'm going to test on devtools those with no quotes or entities. Strange that quotes/entities are not displayed at all in some parts, though. Do you think this has to do with how html-minifier works?

Sure, see Per-Usage HTML Attributes and Getting Started. (Note: loading="lazy" won't affect these iframes since they don't have a src.)

rocc-o commented 1 year ago

Tested on devtools and it works like a charm! The output is perfectly minified now and tags are all ok. And I'm using allowfullscreen="" as per your Per-Usage HTML Attributes. This is a very beautiful plugin and it helped me a lot. Thanks so much Aleksandr for you help. I appreciate it. I'm closing this now.

AleksandrHovhannisyan commented 1 year ago

@rocc-o Glad to hear it! Feel free to open another issue if you run into more problems.