sfirke commented on code in PR #43722:
URL: https://github.com/apache/superset/pull/43722#discussion_r3896221415
##########
superset-frontend/plugins/plugin-chart-handlebars/src/Handlebars.tsx:
##########
@@ -37,7 +37,12 @@ export default function Handlebars(props: HandlebarsProps) {
const handlebarTemplateSource = formData.handlebarsTemplate
? formData.handlebarsTemplate
: '{{data}}';
- const templateSource = `${handlebarTemplateSource}\n${styleTemplateSource} `;
+ // Separate the style block from the template with a blank line. Markdown
+ // treats a template that opens with an HTML tag as a single HTML block that
+ // ends at the first blank line, so without this separator the `<style>` tag
+ // is absorbed into that block and the first blank line inside the CSS closes
+ // it, leaking the remaining rules into the chart as visible text.
+ const templateSource = `${handlebarTemplateSource}\n\n${styleTemplateSource}
`;
Review Comment:
Confirmed, and fixed in 6eed0d4 — thanks.
The reasoning in the comment isn't quite the mechanism (nothing follows the
separator, so no "adjacent Markdown lines" are split), but the conclusion is
right and the effect is real: a trailing blank line makes a **tight list
loose**. With no CSS configured, `- a\n- b` went from `<li>a</li>` to
`<li><p>a</p></li>`, which adds paragraph margins to every Handlebars template
ending in a list.
The concern is also broader than the no-CSS case. Guarding the separator
leaves the same problem when CSS *is* configured, since the blank line is still
appended after the template.
Rather than guard the separator, the style block now goes **before** the
template:
```ts
const templateSource = styleTemplateSource
? `${styleTemplateSource}\n\n${handlebarTemplateSource} `
: `${handlebarTemplateSource}\n `;
```
`<style>` still starts its own CommonMark HTML block (type 1), which ignores
blank lines and runs until `</style>`, but the template is no longer modified
at all — so its rendering is unchanged whether or not CSS is set.
I verified against the pinned pipeline (`[email protected]`,
`[email protected]`, `[email protected]`, `[email protected]`,
`[email protected]`) that the template's rendered output is byte-identical
to the current no-CSS baseline for HTML-table, tight/ordered/nested list,
paragraph, heading, blockquote, GFM-table, `<div>` and `<ul class>` templates.
It also fixes a case the guarded version does not: with an unclosed code fence
in the template, the CSS was previously swallowed into the `<pre>`.
Added a regression test for the tight-list case.
One note on the suggested patch: `formData.styleTemplate || ''` drops the
`<style>` wrapper, so the CSS would be emitted as bare text and no styling
would apply at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
