-
Notifications
You must be signed in to change notification settings - Fork 362
feature: light and dark logo #12996
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
feature: light and dark logo #12996
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
There is one bug I don't see how to fix in any elegant way. In order to allow #12981 dark theme enables dark brand, this PR changes the signature of This is because we don't know until At that point we have combined the project metadata, document metadata, and crucially, we've also combined things like
Since the sidebar and navbar are initialized before any documents, they still have to use This manifests as an invisible |
@@ -738,41 +738,11 @@ function render_dashboard() | |||
end | |||
end | |||
}, { | |||
-- todo: dark mode | |||
Meta = function(meta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seemed weird to me - I'm using a Lua filter just to copy a filter param (with fully-resolved logo) to Pandoc metadata. Is this a normal thing to do, or should I have changed the document metadata that goes into Pandoc? I couldn't figure out how to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quoting your comment here:
weird that i need to forward this here.
i'd prefer to directly change the pandoc metadata in ts
but i couldn't figure it out
What you did is fine, works, and it's relatively efficient (we don't run an entirely new Lua filter call if the filter is only a Meta
entry, there's special-casing for that). We limit information that comes into Pandoc via metadata because Pandoc does automatic Markdown parsing of string values, and generally prefer to send "side-channel" information through the param()
mechanism. That's all set up filterParamsJson in src/command/render/filters.ts:filterParamsJson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, great, I'll fix the comment then. For context, there is also a Pandoc
handler here. I added the Meta
handler in 1.6 to get basic brand.logo
functionality and I was more comfortable with Lua back then. Now I prefer to have all the normalization/resolution logic in Typescript (which helps a LOT with all the cases here!), and I was surprised I couldn't remove the Meta handler entirely.
I was joking about metadata provenance, but more reasonably I think a diagram could be drawn of all the metadatas and how they feed into each other, like our big process diagram. I'm starting to get it, but figuring out how to get information from point A to point B in the code is always the slowest thing for me. (getStack("ansi")
helps a lot!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more reasonably I think a diagram could be drawn of all the metadatas and how they feed into each other, like our big process diagram
Yes, we direly need it. Earlier today, I made a separate comment about the precise location where our Markdown envelope gets created (and how it impacts shortcode resolution).
We need a similar thing about how metadata is assembled and handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm starting to get it, but figuring out how to get information from point A to point B in the code is always the slowest thing for me. (getStack("ansi") helps a lot!)
Side note: I would love a quick demo of this usage of getStack("ansi") because it is still not in my habit to use it despite Carlos having demoed it in the past. When time permits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cderv I'll add a note to our eng meetings
Any objections to filing the above bug separately, and merging this? It should be harmless, a spurious |
@gordonwoodhull That sounds great to me. |
rename functions unified -> having dark use Zod to reify enumerations unified and simple brand logo brand metadata now only contains one polarity of logo since we split the unified logo immediately unfortunate consequence of dark sidebar logo: the dreaded !important css bootstrap started it normalize and resolve all document- and project- provided logos to LightDarkLogo
to ensureHtmlElements it's more robust and i saw this matter with at least one of the others
normalizeLogo + findLogo = resolveLogo we were not able to handle the case where document overrides only dark or light logo under the previous structure, because it needs to look at size order also fold in brand and document.logo undefined cases
Claude got the syntax immediately, but not always the semantics, so I fixed those and we iterated. Claude and came up with many interesting edge cases, some fixed in adjacent commits. "Propose questions and answers, I'll choose the tests." Preserving all "learnings" here. This is not slop, testing light & dark logo is an ideal task for Claude! This commit adds tests for dark/light logo variants in Quarto brand configurations: Key learnings: 1. The dashboard format selects the medium size logo by default, not large 2. Direct path behavior: - Strings without path/alt structure are treated as direct paths - Alt text is empty when using direct path format without alt specification - Named resources and direct paths can be mixed within a configuration 3. Fallback behavior: - When a size is missing a dark variant, it doesn't fall back to its light variant - Instead, it falls back to another size's dark variant (large.dark) - This cross-size fallback behavior may need further specification Tests include: - dark-logo.qmd: Basic light/dark logo configuration - override-dark-logo.qmd: Top-level logo overriding brand.logo - explicit-dark-light-paths.qmd: Direct path specifications - dark-logo-only.qmd: Fallback behavior testing - mixed-logo-path.qmd: Mixed logo definitions (named resources and paths) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> claude: Add comprehensive logo behavior tests This commit adds tests exploring Quarto's logo behavior for dark/light modes: Key learnings: 1. Small is the default logo size for dashboard navbar (not medium) 2. Logo resolution follows a mode-first search algorithm: - Light mode searches small.light → medium.light → large.light - Dark mode searches small.dark → medium.dark → large.dark - It does NOT prioritize staying within same size and switching modes 3. When partial document-level overrides are used (e.g., only light or dark), the non-overridden mode inherits from brand configuration 4. Direct path strings (without .path property) have empty alt text unless explicitly provided via object syntax 5. Document-level logo settings take precedence over brand-level settings Tests include: - document-brand-precedence.qmd: Tests precedence between document and brand logos - partial-document-override.qmd: Tests overriding only one light/dark variant - mixed-specification-formats.qmd: Tests different logo specification formats - cross-size-fallbacks.qmd: Tests fallback behavior when a specific mode is missing - size-inheritance.qmd: Tests the mode-first search algorithm 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> claude: Add logo edge case tests This commit adds tests for edge case logo behaviors: Key learnings: 1. No recursive resolution of logo references is supported - Quarto does not follow chains of references between logo resources 2. Format-specific logo configuration: - Tests format.dashboard.logo taking precedence over document-level and brand logos - Allows format-specific customization while maintaining generic defaults 3. Legacy logo syntax: - A simple string logo value at document level applies to both light and dark modes - Demonstrates backward compatibility with pre-dark-mode syntax 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> claude: Rename logo tests for better clarity Renamed test files to more accurately reflect the behaviors they test: - cross-size-fallbacks.qmd → mode-first-search-algorithm.qmd Tests how light/dark modes independently search through size hierarchy - size-inheritance.qmd → dark-mode-search-priority.qmd Tests dark mode's search priority and lack of fallback to light mode - dark-logo-only.qmd → missing-medium-dark-fallback.qmd Tests specific fallback behavior when medium.dark is missing - mixed-logo-path.qmd → mixed-resource-direct-path.qmd Better describes the mix of resource references and direct paths - mixed-specification-formats.qmd → path-vs-object-specification.qmd Clarifies test comparing string paths vs object specifications with alt text 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> remove pngs it's not necessary to have the files for the tests to run claude: Add sidebar logo tests for size preference and mode-first search This commit adds tests for sidebar logo behavior: 1. Size preference order test (sidebar/size-preference-order) - Tests that sidebar logos follow the preference order: medium → small → large - Verifies that when medium.dark is missing but small.dark and large.dark exist, it correctly selects small.dark due to the preference order 2. Mode-first search algorithm test (sidebar/mode-first-search) - Tests that light/dark modes independently search through all sizes before considering the other mode - Verifies that it searches through all sizes for dark mode before considering any light mode variants These tests confirm that sidebar logo behavior follows the same mode-first search principles as dashboard logos, but with a different size preference order (medium → small → large for sidebars vs medium → large → small for dashboard). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> claude: Add navbar light/dark logo tests This commit adds tests for navbar logo behavior with light/dark variants: 1. Size Preference Test: - Tests navbar size preference order (small → medium → large) - Verifies the mode-first search algorithm in a scenario with no small logos - Confirms medium.light is selected for light mode when small is missing - Confirms large.dark is selected for dark mode when small.dark is missing 2. Alt Text Consistency Test: - Tests that alt text is properly inherited from brand definitions - Confirms that missing resources get empty alt text - Helps ensure accessibility attributes are maintained 3. Missing Logo Handling Test: - Tests behavior when logo paths point to non-existent files - Confirms img tags are generated with correct src attributes - Ensures robust rendering even with missing resources Key learnings: - Navbar DOES NOT support document-level logo overrides unlike sidebars/dashboards - Navbar follows the size preference order: small → medium → large (different from sidebar: medium → small → large and dashboard: medium → large → small) - Mode-first search applies to navbars: it exhaustively searches for a specific mode (light/dark) across all sizes before falling back to other modes 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> claude: Add cross-mode logo fallback tests This commit adds tests for how Quarto handles edge cases when logos are missing for one theme mode but both light and dark modes are enabled: 1. dark-mode-no-dark-logo.qmd: Tests what happens when dark mode is enabled but only light logo exists 2. dark-mode-no-light-logo.qmd: Tests what happens when dark mode is enabled but only dark logo exists 3. document-light-brand-light-only.qmd: Tests document-level light-only logo + brand with light-only logo 4. document-dark-brand-dark-only.qmd: Tests document-level dark-only logo + brand with dark-only logo Key learnings: - Quarto implements cross-mode fallbacks where missing logos for one mode automatically use logos from the other mode - Implementation in brand.ts shows the fallback logic applies in both directions: light → dark and dark → light - Document-level logo settings take precedence over brand-level settings - The fallback system ensures logos always appear regardless of which theme mode is active, providing a consistent user experience these tests do not support revealjs default to other mode of logo
and enablesDarkMode flag until merging with theme, when we know whether we need dark mode a bit messy to have this extra flag, but seems the be the only correct way
96a99e4
to
eb5c38a
Compare
eb5c38a
to
39d6b3e
Compare
Windows, |
(I'm going to merge this independently of the test failures. @cderv I think this is similar to what we had seen in that other PR...) |
yes it seems similar. Really odd issue. xml2 seems to be correctly installed. |
We allow the all logo specifications for document- and project- level logos in sidebar, navbar, and dashboard navbar, from string to object to
{light: {...}, dark: {...}
We normalize to the final, most general form.
Fixes #12341 dark logo
Fixes #12981 brand applied to dark mode if theme enables dark mode
Posting draft for CI; reviews also welcome!
I'm going to put off the following until after vacation. Not hard, just running out of time and I have other stuff to work on.
{path, alt}
object form, Typst #12180