Skip to content

Flourish fixes #80

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 3 commits into from
May 7, 2025
Merged

Flourish fixes #80

merged 3 commits into from
May 7, 2025

Conversation

adgad
Copy link
Collaborator

@adgad adgad commented May 7, 2025

Does a few things to make the flourish transformation work (tested with 1902feb4-9c1c-4c90-ba6b-235e89fda784)

  • Adds id and removes children from the flourish transformer
  • fallbackImage was previously optional, but it doesn't really make sense in the transit-tree because it will never be transitted. I've jigged the type-making script to allow us to define an optional external. This will not exist in the transit-tree schema, and will exist in the full schema as optional. Hopefully this makes it a bit more explicit.
  • (unrelated) removed embedded from video transformer, after Remove embedded property from videos #77

@adgad adgad requested review from chee and a team as code owners May 7, 2025 12:40
@chee
Copy link
Member

chee commented May 7, 2025

fallbackImage was previously optional,

probably an accident from before external type

flourishType: content.attributes["data-flourish-type"] || "",
layoutWidth: toValidLayoutWidth(
content.attributes["data-layout-width"] || ""
),
description: content.attributes["alt"] || "",
timestamp: content.attributes["data-time-stamp"] || "",
// fallbackImage -- TODO should this be external in content-tree?
Copy link
Member

Choose a reason for hiding this comment

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

lol

@adgad adgad merged commit f4d650c into main May 7, 2025
2 checks passed
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