mermaid,themes - update themes

[cscheid]

This PR makes mermaid flowcharts work well across all bootswatch themes, following our work-week discussions on Friday.

Specifically, it specializes the mermaid SCSS variables to each theme, and tweaks them as necessary.

It also changes one of the defaults across all themes in the CSS emitted by mermaid-init.js itself, so that the arrow end markers look nicer.

[cscheid]

Note that it's very hard to add regression tests here, so this would be a good candidate for future visual tests.

[cderv]

So cool ! Thanks for this.

Initially I was thinking we could add each mermaid variables into an additional theme file per bootswatch theme. idea was to simplify the update of bootstrap and keep initial bootswatch without modification for quarto.

But using git diff in an update is easy enough I guess.

Just sharing what I would have done. All good - I'll note the idea for visual testing.

[cscheid]

Initially I was thinking we could add each mermaid variables into an additional theme file per bootswatch theme. idea was to simplify the update of bootstrap and keep initial bootswatch without modification for quarto.

That might be the right way to do solve the problem. I already needed to add an additional theme file for the default theme, which doesn't have an .scss file by itself. But then, that caused all of these failures in CI for reasons I don't understand.

[cderv]

The issue you get in CI for revealjs format is because quarto-rules.scss is shared with revealjs too

quarto-cli/src/format/reveal/format-reveal-theme.ts

Lines 157 to 161 in f4786f4

	// Quarto layers
	const quartoLayers = [
	quartoBaseLayer(format, true, true, false, true),
	quartoLayer(),
	];

So you need to defined the default variables for mermaid also in a layer used with revealjs.

quarto-cli/src/format/html/format-html-shared.ts

Lines 266 to 274 in f4786f4

	export const quartoBaseLayer = (
	format: Format,
	codeCopy = false,
	tabby = false,
	figResponsive = false,
	codeFilename = false,
	) => {
	const rules: string[] = [quartoRules()];
	const defaults: string[] = [quartoDefaults(format), quartoVariables()];

• _quarto-variables.scss ?
• not in a file like quartoDefaults ?
• An additional specifial _quarto_mermaid_variable.scss to make them separate ?

With our SASS layering system, this is a question of where and which order so that it keeps easy to maintain in the future.

[cderv]

The error are related with all the missing them options. By adding the rules into the _quarto_rules layer, it means we are making it default for Quarto base layer

quarto-cli/src/format/html/format-html-shared.ts

Lines 266 to 290 in c73c11d

	export const quartoBaseLayer = (
	format: Format,
	codeCopy = false,
	tabby = false,
	figResponsive = false,
	codeFilename = false,
	) => {
	const rules: string[] = [quartoRules()];
	const defaults: string[] = [quartoDefaults(format), quartoVariables()];
	if (codeCopy) {
	rules.push(quartoCopyCodeRules());
	defaults.push(quartoCopyCodeDefaults());
	}
	if (tabby) {
	rules.push(quartoTabbyRules());
	}
	if (figResponsive) {
	rules.push(quartoFigResponsiveRules());
	}
	if (codeFilename) {
	rules.push(quartoCodeFilenameRules());
	}
	if (format.render[kLinkExternalIcon]) {
	rules.push(quartoLinkExternalRules());
	}

So the variables associated with those rules, should also be defined in the defaults layer. See above what we do for each feature we have and need variables.

I would really add some basic default in this base layer, which is used for HTML document and not for reveal. This base layer will apply for HTML format without bootstrap

quarto-cli/src/format/html/format-html.ts

Lines 417 to 431 in c73c11d

	// add quarto sass bundle of we aren't in bootstrap
	const minimal = format.metadata[kMinimal] === true;
	if (!bootstrap && !minimal) {
	if (scssOptions.quartoBase) {
	sassBundles.push({
	dependency: kQuartoHtmlDependency,
	key: kQuartoHtmlDependency,
	quarto: quartoBaseLayer(
	format,
	!!options.copyCode,
	!!options.tabby,
	!!options.figResponsive,
	),
	});
	}

I really wonder if we should not try to make those mermaid SCSS information a specific quarto internal layer that we could add in every place (no bootstrap, bootstrap, and revealjs

This will also make themes updates when we update dependencies easier. Adding to each revealjs themes, will mean manual editing when updating theme I think

[cscheid]

I tried doing that, but then I couldn't make the non-default themes override the defaults. I think I tried adding them to different layers and couldn't make any of it work.

I know SCSS is hard, but honestly, I think we should consider rethinking this whole thing. It feels hopeless to do something robustly and extensibly.

[cderv]

I know SCSS is hard, but honestly, I think we should consider rethinking this whole thing. It feels hopeless to do something robustly and extensible.

I was started to have the feeling to understand it ! 😅 But why not rethinking all this, at least for our internal code base so that it is easier to maintain.

I find this layering system quite useful to split content, but the order matters, and currently the fact everything is split across our code base makes it hard to fully reason about.

Do you want me to give it a try based on what I learnt by looking into this for revealjs theme ?