Skip to content

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Dec 22, 2023

fixes #8012

Doing a title snipping when engine: jupyter is a bit too much for revealjs format because this will create a new title slide whereas when no title is provided, it should be considered custom first slide per our doc: https://quarto.org/docs/presentations/revealjs/index.html#title-slide

So I suggest we do not strip in Revealjs format.

Just asking for review because I don't fully understand why we do this stripping.

Does it sounds good ?

We also use the function for stripping at

// partition markdown into yaml, the first heading, and the rest of the markdown text
export function partitionMarkdown(markdown: string): PartitionedMarkdown {
// partition out yaml
const partitioned = partitionYamlFrontMatter(markdown);
markdown = partitioned ? partitioned.markdown : markdown;
// extract heading
const { lines, headingText, headingAttr } = markdownWithExtractedHeading(
markdown,
);

So I need to check if we need also some support there... 🤔

it seems used for special cases like embed feature and some unused function

@cderv cderv requested a review from dragonstyle December 22, 2023 14:56
// if we have front matter and a title then we are done
yaml?.title ||
// if we have front matter and it has revealjs as a format then we are done too
(yaml?.format !== null &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea of this is that a python user with existing notebooks may want to render them using quarto render foo.ipynb --to html or similar commands and get a reasonable experience. Since it is common to place a heading as the 'title' of the notebook in a markdown cell at the top of the notebook, we're trying to find the best way to promote that to the document title in Quarto.

It's pretty tempting to just that if the user creates front matter at all, they should place a title there (not just for revealJS - just that once they're making front matter we expect them to handle the title)...

We could start with this if you're feeling conservative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revealjs specific case is that

  • Using title will trigger the use of a specific title-slide sectionf from the revealjs template
  • To opt out this title slide from the template, no providing title in YAML is for now the way to go.

Currently it seems you can't do that with engine: jupyter, or at least that we promote headers like ## without considering the slide-level, meaning that a non section slide could be promoted to section slide.

This is something quite specific to revealjs here related to this heading promoting feature. 🤔

@cderv
Copy link
Collaborator Author

cderv commented Dec 22, 2023

BY the way the partitionMarkdown where we do use this splitting of heading, seems to have a bug regarding indexFile for caching. I'll report elsewhere for this

@cderv
Copy link
Collaborator Author

cderv commented Dec 22, 2023

OK so it seems this is more complex that it seems. This markdown.headingText is used in a lot of places.

I am even not sure that the new addition in 24672a4 about not stripping header if content before heading (contentBeforeHeading) is really respected in the partitionMarkdown() function used in part of our code base

I'll leave this as draft for further work together on this, and open smaller PR for immediate issues found

Don't perform notebook title fixup with `format: revealjs` as this would create a new undesired title slide.

Follow up on 24672a4
@cderv cderv force-pushed the revealjs/extract-heading-jupyter branch from 73083f5 to 962c77e Compare September 3, 2024 15:16
@cderv
Copy link
Collaborator Author

cderv commented Oct 16, 2024

Not sure why,but it does not seem to be needed anymore. Initial issue #8012 is no more reproducible.

So I am closing this.

@cderv cderv closed this Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RevealJS: Specifiyng engine introduces spurious title slide.

2 participants