Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quarto vignette titles are broken with quarto 1.6.x #2823

Open
banfai opened this issue Nov 25, 2024 · 13 comments
Open

Quarto vignette titles are broken with quarto 1.6.x #2823

banfai opened this issue Nov 25, 2024 · 13 comments
Labels

Comments

@banfai
Copy link
Contributor

banfai commented Nov 25, 2024

With the pre-release version of quarto the page title has a glitch, it's in a different div <header id="title-block-header" class="quarto-title-block">) instead of the previous <div class="page-header">. This causes the Source link to be placed above the title:

release version on gh-pages:
quarto-vignette-1 5 x

same version rendered with quarto 1.6.37:
quarto-vignette-1 6 37

The page-header div is still there with the logo and the source code link, but the title is missing: <h1></h1>

quarto::quarto_version()
#> [1] '1.6.37'

It remains the same with specifying

format:
  html:
    title-block-style: none
@banfai
Copy link
Contributor Author

banfai commented Nov 26, 2024

I believe it's caused by this:
quarto-dev/quarto-cli#10567
quarto-dev/quarto-cli#11224

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Nov 26, 2024

Quarto tests are also failing with the new release, this appears to be the same problem:

Failure (test-build-quarto-articles.R:59:3): auto-adjusts heading levels
xpath_text(html, "//h1") (`actual`) not equal to "title" (`expected`).

`actual`:   "" "title"
`expected`:    "title"

Failure (test-build-quarto-articles.R:104:3): quarto headings get anchors
xpath_attr(headings, "./a", "href") (`actual`) not equal to c("#heading-1", "#heading-2") (`expected`).

`actual`:   "#heading-1"             
`expected`: "#heading-1" "#heading-2"

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Dec 2, 2024

It's not clear from the comments whether the change in quarto-dev/quarto-cli#11224 will be permanent. Perhaps we should clarify before updating pkgdown to process the changed HTML.

I'm assuming we don't want to be supporting multiple versions of quarto, so if we update to process the new HTML, we'd need to bump the required quarto version to 1.6.33.

@banfai
Copy link
Contributor Author

banfai commented Dec 2, 2024

It's not clear from the comments whether the change in quarto-dev/quarto-cli#11224 will be permanent. Perhaps we should clarify before updating pkgdown to process the changed HTML.

should I open an issue for quarto?

I'm assuming we don't want to be supporting multiple versions of quarto, so if we update to process the new HTML, we'd need to bump the required quarto version to 1.6.33.

this might be an issue for some time since 1.6.x is still pre-release

@cderv
Copy link
Contributor

cderv commented Dec 2, 2024

It took me some time to understand the problem and this is indeed quarto-dev/quarto-cli#11224 having side effect on the custom processing that pkgdown is doing

Within pkgdown, quarto render is called with a custom template, that create a div structure with some information

<div class="meta">
$if(title)$
<h1>$title$</h1>
$endif$
$if(subtitle)$
<p class="subtitle">$subtitle$</p>
$endif$
$for(author)$
<p class="author">$author$</p>
$endfor$
$if(date)$
<p class="date">$date$</p>
$endif$
$if(abstract)$
<div class="abstract">
$abstract$
</div>
$endif$
</div>

The custom template is applying which leads to this in the HTML before Quarto postprocessing

<div class="meta">
  <h1>My cool title</h1>
</div>

However, as the custom template does not use title-block.html partial, it will trigger the post processing introduced by PR quarto-dev/quarto-cli#11224 as if the document had no title.
https://github.com/quarto-dev/quarto-cli/blob/39baf4081f184fe090974295b48d1beab32cd1e4/src/format/html/format-html-title.ts#L205

and a title block will be created anyway by this new post-processing. The h1 from the custom template is caught
https://github.com/quarto-dev/quarto-cli/blob/39baf4081f184fe090974295b48d1beab32cd1e4/src/format/html/format-html-title.ts#L210
and the h1 that pkgdown expect will be moved
https://github.com/quarto-dev/quarto-cli/blob/39baf4081f184fe090974295b48d1beab32cd1e4/src/format/html/format-html-title.ts#L238

I don't know exactly why pkgdown does this title processing with specific custom template, but this is indeed a Quarto issue in the sense that it should not probably apply this eagerly

@banfai

should I open an issue for quarto?

I'll do it

this might be an issue for some time since 1.6.x is still pre-release

Quarto 1.6 is the latest release since last week.

@banfai
Copy link
Contributor Author

banfai commented Dec 2, 2024

@banfai

should I open an issue for quarto?

I'll do it

thanks!

this might be an issue for some time since 1.6.x is still pre-release

Quarto 1.6 is the latest release since last week.

oops, I missed that

@hadley
Copy link
Member

hadley commented Dec 5, 2024

Can we make pkgdown work either way? This is causing pkgdown to fail on CRAN and it's likely easier to submit an updated pkgdown than get CRAN to update their quarto.

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Dec 5, 2024

@hadley We could fix this to work with quarto 1.6, but may then have to revert if this is "fixed" on the quarto CLI side.

And if we want to fix, should we be supporting quarto 1.5 (where this test passes) as well as quarto 1.6?

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Dec 5, 2024

I am also not clear what specific quarto version that CRAN is installing in their systems for checks. Is that documented somewhere?

@jayhesselberth
Copy link
Collaborator

Quarto devs are planning for a quick fix quarto-dev/quarto-cli#11224 (comment) so maybe we sit tight.

@cderv
Copy link
Contributor

cderv commented Dec 6, 2024

Yes we have a fix that should sove the problem for pkgdown. We'll do a patch release.

I am also not clear what specific quarto version that CRAN is installing in their systems for checks. Is that documented somewhere?

Though I am like you and I don't know for sure if CRAN update Quarto at each stable release we do

@cderv
Copy link
Contributor

cderv commented Dec 10, 2024

FYI we merge a fix in Quarto, including for a patch release. So next 1.6 and 1.7 will not have the problem that impact pkgdown.

@jayhesselberth
Copy link
Collaborator

jayhesselberth commented Dec 10, 2024

Thanks @cderv for the quick fix! Will leave this open until we confirm it's fixed in the next patch release of quarto.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants