Add Colors to DocC Documentation Navigation #973
Conversation
|
@swift-ci please test |
|
Sorry, I've been busy with other things lately, but I'm hoping to take a look at this PR in the near future—bringing color to the sidebar icons sounds exciting! |
| <template> | ||
| <div | ||
| class="TopicTypeIcon" | ||
| :class="{ 'navigator-icon': true, 'colorful-navigator-icon': shouldUseColorfulIcons && hasSymbolText }" |
There was a problem hiding this comment.
I don't think we should always add the "navigator-icon" class here since this component isn't only ever used in the navigator. In fact, I believe the navigator component where this component is utilized is already conditionally applying this exact class, so this is actually getting duplicated in that scenario.
| }, | ||
| enableColorfulIcons: { | ||
| type: Boolean, | ||
| default: true, |
There was a problem hiding this comment.
I think the default should probably be false for now instead of true, for a couple of reasons.
- We likely only want to apply the color when these icons are rendered in the navigator, and there are a few other places where this component is rendered—even if the symbol variants aren't rendered there today.
- More importantly, I wonder if we can introduce these colored icons as an experimental feature first by way of creating a new feature flag in
theme-settingsto opt-in to this new styling. While I feel that the colored icons look better overall, it might be good to introduce this more gradually since there isn't much color used in other places throughout the app yet.
| icon: ({ normalisedType }) => TopicTypeIcons[normalisedType] || CollectionIcon, | ||
| iconProps: ({ normalisedType }) => TopicTypeProps[normalisedType] || {}, | ||
| color: ({ normalisedType }) => HeroColorsMap[normalisedType], | ||
| shouldUseColorfulIcons() { |
There was a problem hiding this comment.
Is this computed property needed since it always just mirrors the same value as the prop?
| return props.first + props.second; | ||
| } | ||
| return ''; |
There was a problem hiding this comment.
It seems a little odd to sometimes return null and sometimes return ''—should we just return null here?
| const text = this.symbolText.toUpperCase(); | ||
| // Purple: Structures, Classes, Protocols | ||
| if (['S', 'C', 'PR'].includes(text)) return 'purple'; |
There was a problem hiding this comment.
Although the performance difference is very minimal, should we just map all of these explicitly to avoid searching through arrays? Something like this pseudocode maybe?
{
[TopicTypes.struct]: Color.purple,
[TopicTypes.protocol]: Color.mint,
...
}[type] ?? Color.default
(Might be useful to have a constant for the color names to help avoid mistakes with typing them differently in different places)
| */ | ||
| // System font with SF Pro Rounded fallback | ||
| @font-face { |
There was a problem hiding this comment.
I'm not sure we should look for SF Pro fonts first if we aren't providing them in the app itself.
There was a problem hiding this comment.
(The system fonts are actually the fallbacks to these fonts in the family rule from the "colorful-navigator-icon" class below and the ones more likely to be actually displayed)
| } | ||
| // Color palette for colorful icons | ||
| :root { |
There was a problem hiding this comment.
If we want these colors to be available to other places within the app, we should probably just move these to the existing colors in src/styles/core/colors/_[light|dark].scss
However, since we aren't using these elsewhere yet, I would probably just define these CSS properties within the scoped styles for this component itself.
| // Light mode colors | ||
| --systemOrange-light: #ff9500; | ||
| --systemPurple-light: #cc54da; | ||
| --systemBlue-light: #0f7dfa; |
There was a problem hiding this comment.
I'm curious how you decided on the values for these. Are you using the System colors section from the Human Interface Guidelines maybe (doesn't seem like it)?
| height: 18px; | ||
| padding: 2px 4px; | ||
| border-radius: 4px; | ||
| background-color: var(--systemGreen); |
There was a problem hiding this comment.
Maybe we should use the default fill color for the app instead of green? Not super important—green just seems a little arbitrary to me.
| box-sizing: border-box; | ||
| vertical-align: middle; | ||
| font-family: 'SF Pro Rounded', 'SF Pro Display', -apple-system, BlinkMacSystemFont, system-ui, sans-serif; |
There was a problem hiding this comment.
Same note from above about the SF Pro fonts.
|
Sorry I forgot to include the below disclosure before taking your valuable time to review this PR. @mportiz08 This draft PR is mainly intended to demonstrate the idea of integrating the colorful-apple-documents plugin directly into docc-render, and it’s currently not ready for a full review. (That said, I have already tested and used the generated artifact in several places.) Most of the code was assisted or translated by AI from ktiays/colorful-apple-documents, and it’s not meant to be merged upstream directly. I originally planned to refine and optimize the code before removing the draft status and mark it ready for review, but I haven’t had much time lately. I’ll make changes based on your comments and try to remove the draft status as soon as possible. Of course, if anyone would like to help with these improvements based on my current branch (I have enabled Allow edits by maintainers), that would be very welcome since I’m not very familiar with frontend code. |
|
No rush @Kyle-Ye, and I appreciate the disclosure. Thanks for exploring this! |
4 participants
From: https://github.com/ktiays/colorful-apple-documents
Bug/issue #, if applicable:
Summary
Enhances Swift DocC-generated documentation sites with color-coded tags and improved visual styling for better navigation and readability.
It does the same as the Browser extension. But this is enabled on DocC render so that the reader do not need to install it if the documentation is built with this feature.
Dependencies
None
Checklist
Make sure you check off the following items. If they cannot be completed, provide a reason.
npm test, and it succeededOther
https://forums.swift.org/t/browser-extension-add-colors-to-docc-documentation-navigation/81789
Prebuilt dist download:
https://github.com/Kyle-Ye/swift-docc-render/releases/tag/0.0.1-colorful