Read the list-marker color from MarkdownRenderConfig - #164
Open
lukebradford wants to merge 1 commit into
Open
Conversation
`OrderedListView` colors its numerals with `config.orderedListStyle.textColor`, but `UnorderedListView` drew its bullet and its task-list checkbox with a literal `Color.Theme.Foreground.Primary.Primary450`. A host that themes the config therefore gets correctly-colored numerals and bundled-palette bullets in the same document, with no way to reach the latter — an `Image`'s own `foregroundStyle` can't be overridden by an ancestor. Both now read the same config property the ordered list already uses. This is a no-op for anyone on the default config: `defaultOrderedListStyle`'s `textColor` *is* `Primary450`, the exact literal being replaced. Only a host that has already overridden `orderedListStyle` sees a change, and that is the change it asked for. Verified by running the package's own `UnorderedListViewTests` before and after: byte-identical results. (Heads up for CI: on my machine those snapshot tests already fail at unmodified `main` — 10 reference mismatches under `macOS-standard-*` — so the comparison above is "same failures before and after", not "green". Looks like the references were recorded on different hardware.)
There was a problem hiding this comment.
Pull request overview
This PR makes unordered list markers (bullets and task-list checkboxes) respect the host-provided MarkdownRenderConfig color, aligning their appearance with ordered-list numerals and preventing mixed “themed text + bundled-palette markers” rendering in the same document.
Changes:
- Inject
MarkdownRenderConfigvia@Environment(\.markdownConfig)inUnorderedListView. - Replace hard-coded marker
foregroundStylecolor withconfig.orderedListStyle.textColorfor bullets and task checkboxes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| .resizable() | ||
| .frame(width: 12, height: 12) | ||
| .foregroundStyle( Color.Theme.Foreground.Primary.Primary450) | ||
| .foregroundStyle(config.orderedListStyle.textColor) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OrderedListViewcolors its numerals withconfig.orderedListStyle.textColor, butUnorderedListViewdraws its bullet and its task-list checkbox with a literalColor.Theme.Foreground.Primary.Primary450.A host that themes
MarkdownRenderConfigtherefore gets correctly-colored numerals and bundled-palette bullets in the same document, with no way to reach the latter — anImage's ownforegroundStylecan't be overridden from an ancestor. In our case (a warm, near-black/near-white palette) the bullets came out mid-gray in light and blue-gray in dark, next to text that was neither.This routes the bullet and the checkbox through the same config property the ordered list already uses.
Why this is safe
It's a no-op on the default config:
defaultOrderedListStyle'stextColorisPrimary450, the exact literal being replaced. Only a host that has already overriddenorderedListStylesees any change — and that's the change it asked for.I verified by running
UnorderedListViewTestsbefore and after the patch: byte-identical results, same failures, same snapshot names.Note for CI
Those snapshot tests already fail for me at unmodified
main— 10 reference mismatches undermacOS-standard-light/macOS-standard-dark, acrossUnorderedListViewTestsandTableViewSnapshotTests.testTableCellWithOnlyCitation. They look recorded on different hardware than mine. So my verification above is "identical before and after", not "green", and it may be worth a look independently of this PR.Alternative
If you'd rather not overload
orderedListStylefor unordered markers, I'm happy to add a dedicatedunorderedListStyle(or a sharedlistMarkerStyle) instead — that's a public API addition, which is why I didn't reach for it first. Say the word and I'll rework it.🤖 Generated with Claude Code