Skip to content

Fix Markdown rendering inside templates. #1219

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

Merged
merged 2 commits into from
Sep 12, 2022
Merged

Conversation

carloskelly13
Copy link
Contributor

Description

Fixes a bug where a Markdown component inside a Deck template would break Spectacle. The previous implementation of Markdown always creates a Notes node tree, even with undefined children if there are no notes, which cannot exist on a template as it's not a slide. This verifies there is note content before building the Notes node tree.

Type of Change

Please delete options that are not relevant (including this descriptive text).

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Add a <Markdown /> component with text content to a Deck template before this change and see Spectacle break and fail to render. Check it after and see it correctly render the Markdown component.

Example below showing a Markdown component on the template and notes inside a Markdown slide verifying no regressions introduced.

Screen Shot 2022-09-10 at 4 10 19 PM

Copy link
Contributor

@gksander gksander left a comment

Choose a reason for hiding this comment

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

LGTM

@carloskelly13 carloskelly13 merged commit 055d9c6 into main Sep 12, 2022
@carloskelly13 carloskelly13 deleted the fix/markdown-template-bug branch September 12, 2022 18:00
@github-actions github-actions bot mentioned this pull request Sep 12, 2022
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.

2 participants