quarto-dev / quarto-cli

Open-source scientific and technical publishing system built on Pandoc.
https://quarto.org
Other
3.97k stars 327 forks source link

link are no more underlined by default #11509

Open cderv opened 1 day ago

cderv commented 1 day ago

I think this is a regression Quarto 1.6 compared to Quarto 1.5 due to this

to support brand feature regarding typography on link

The default has been changed to inherit https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/_bootstrap-variables.scss#L272 which overwrite previous rule set by bootstrap https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/dist/scss/_variables.scss#L462

Originally posted by @cderv in https://github.com/quarto-dev/quarto-cli/discussions/11496#discussioncomment-11350473

---
title: Test
format: html
---

Link to [quarto.org](https://quarto.org)

Quarto 1.6

Image

Quarto 1.5

Image

cderv commented 1 day ago

@cscheid is that intended change on our HTML default for brand to match other format ?

gadenbuie commented 1 day ago

I think it's very likely that instead of

$link-decoration: inherit !default;

you'd want to use

$link-decoration: null !default;

That's especially true if we were previously not setting a { text-decoration: $link-decoration; }.

cderv commented 1 day ago

This rule is set by bootstrap in _reboot.scss so it was set previously https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/dist/scss/_reboot.scss#L256

And it was used with own bootstrap defined default value https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/dist/scss/_variables.scss#L462

But now the rule is set by us too https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/_bootstrap-rules.scss#L2173-L2179

and the variable apply before bootstrap one https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/html/bootstrap/_bootstrap-variables.scss#L272

$link-decoration: null !default;

What does null do ? Will it still get the bootstrap default one ? 🤔

gadenbuie commented 1 day ago

IIUC the goal of defining $link-decoration in _bootstrap-variables.scss is to make sure it is defined before it's used, in particular because _bootstrap-variables.scss are used in revealjs without the rest of the Bootstrap stylesheets.

If I'm right about that, then $link-decoration: null !default is the right move here. It defines the variable without interfering with default order. I.e. it's at least defined but a !default before or after will take precedence.

// Bootstrap: _variables.scss
$def: bootstrap !default;

// Quarto _bootstrap-variables.scss
$def: null !default;

a {
  text-decoration: #{$def}
}

It's something like the above snippet, if Bootstrap's _variables.scss are included, the rule has Bootstrap's default. If not (comment out the bootstrap line), then text-decoration isn't set and there aren't Sass compilation errors. If you didn't define $def at all, then the sheet fails to compile. (Try it here

cscheid commented 1 day ago

This is not an intentional change! (Ah, to have good visual testing 🫠)

It's just a matter of replacing inherit with null in that line, right? Let's just do it.

cderv commented 1 day ago

in particular because _bootstrap-variables.scss are used in revealjs without the rest of the Bootstrap stylesheets.

@gadenbuie _bootstrap-variables.scss is not used in reveals format. No bootstrap related file should be used. Only quarto one could be common, and otherwise, this is some reveal only them file.

This is the case here where it has been added in https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/revealjs/quarto.scss#L24

And in revealjs, links are not underlined by default so this is not a visible change; Revealjs sets it to none in its common theme https://github.com/quarto-dev/quarto-cli/blob/88819d5b9f092381d5d20a146ac3121dc2435acc/src/resources/formats/revealjs/reveal/css/theme/template/theme.scss#L268-L272

The new rules will make the element inherit from parent element and so it could be underline or not. 🤷

Also Regarding your example, I believe the layering that happens in quarto is this one

// Quarto _bootstrap-variables.scss
$def: null !default;

// Bootstrap: _variables.scss
$def: bootstrap !default;

a {
  text-decoration: #{$def}
}

it seems to also work.

though it seems to me that as $link-decoration is already bootstrap variable

Revealjs theme doesn't use a variable for this but we could also start patching completely the revealjs themes so that it behaves closer to boostrap on all the pieces we need for brand.