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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve title snipping in notebooks for revealjs #8016

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

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

// 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 9f0c42f to 42cd1a8 Compare December 22, 2023 20:44
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