-
Notifications
You must be signed in to change notification settings - Fork 65
Add Colors to DocC Documentation Navigation #973
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,10 @@ | |
| <template> | ||
| <div | ||
| class="TopicTypeIcon" | ||
| :class="{ 'navigator-icon': true, 'colorful-navigator-icon': shouldUseColorfulIcons && hasSymbolText }" | ||
| :style="styles" | ||
| :data-color-variant="colorVariant" | ||
| :data-symbol-text="symbolText" | ||
| > | ||
| <OverridableAsset | ||
| v-if="imageOverride" | ||
|
|
@@ -110,6 +113,10 @@ export default { | |
| type: Boolean, | ||
| default: false, | ||
| }, | ||
| enableColorfulIcons: { | ||
| type: Boolean, | ||
| default: true, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the default should probably be
|
||
| }, | ||
| imageOverride: { | ||
| type: Object, | ||
| default: null, | ||
|
|
@@ -124,6 +131,57 @@ export default { | |
| icon: ({ normalisedType }) => TopicTypeIcons[normalisedType] || CollectionIcon, | ||
| iconProps: ({ normalisedType }) => TopicTypeProps[normalisedType] || {}, | ||
| color: ({ normalisedType }) => HeroColorsMap[normalisedType], | ||
| shouldUseColorfulIcons() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this computed property needed since it always just mirrors the same value as the prop? |
||
| return this.enableColorfulIcons; | ||
| }, | ||
| /** | ||
| * Returns the symbol text to display in colorful icon mode. | ||
| * Only symbols with explicit text definitions (from TopicTypeProps) are colorized. | ||
| * Decorative icons (articles, collections, etc.) remain unchanged. | ||
| */ | ||
| symbolText() { | ||
| if (!this.shouldUseColorfulIcons) return null; | ||
|
|
||
| const props = this.iconProps; | ||
| if (props.symbol) { | ||
| return props.symbol; | ||
| } | ||
| if (props.first && props.second) { | ||
| return props.first + props.second; | ||
| } | ||
|
|
||
| return ''; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems a little odd to sometimes return |
||
| }, | ||
| hasSymbolText() { | ||
| return this.symbolText && this.symbolText.length > 0; | ||
| }, | ||
| /** | ||
| * Determines the background color variant based on the symbol text. | ||
| * Color mapping follows Apple's SF Symbols conventions. | ||
| */ | ||
| colorVariant() { | ||
| if (!this.shouldUseColorfulIcons || !this.hasSymbolText) return null; | ||
|
|
||
| const text = this.symbolText.toUpperCase(); | ||
|
|
||
| // Purple: Structures, Classes, Protocols | ||
| if (['S', 'C', 'PR'].includes(text)) return 'purple'; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? (Might be useful to have a constant for the color names to help avoid mistakes with typing them differently in different places) |
||
|
|
||
| // Blue: Methods | ||
| if (text === 'M') return 'blue'; | ||
|
|
||
| // Mint: Properties | ||
| if (text === 'P') return 'mint'; | ||
|
|
||
| // Orange: Types, Enums | ||
| if (['T', 'E'].includes(text)) return 'orange'; | ||
|
|
||
| // Red: Macros | ||
| if (text === '#') return 'red'; | ||
|
|
||
| // Green: Default | ||
| return 'green'; | ||
| }, | ||
| styles: ({ | ||
| color, | ||
| withColors, | ||
|
|
@@ -132,14 +190,77 @@ export default { | |
| }; | ||
| </script> | ||
|
|
||
| <style scoped lang='scss'> | ||
| <style lang='scss'> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since none of this styling is meant to globally affect other places of the codebase, I think we should keep these styles scoped. |
||
| @import 'docc-render/styles/_core.scss'; | ||
|
|
||
| /** | ||
| * Colorful Navigator Icons | ||
| * | ||
| * Provides colorful, rounded icon badges for symbol types in the navigator. | ||
| * Inspired by Xcode's documentation rendering and SF Symbols design language. | ||
| * | ||
| * Features: | ||
| * - System font fallback for SF Pro Rounded | ||
| * - Color variants based on symbol type (purple, blue, mint, orange, red, green) | ||
| * - Automatic light/dark mode color adjustment | ||
| * - Only applied to symbols with text (classes, structs, methods, etc.) | ||
| * - Decorative icons (articles, collections) remain unchanged | ||
| */ | ||
|
|
||
| // System font with SF Pro Rounded fallback | ||
| @font-face { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (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) |
||
| font-family: 'SF Pro Rounded'; | ||
| src: local('SF Pro Rounded'), local('SF Pro Display'); | ||
| font-weight: bold; | ||
| font-style: normal; | ||
| } | ||
|
|
||
| // Color palette for colorful icons | ||
| :root { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)? |
||
| --systemMint-light: #00c1d9; | ||
| --systemRed-light: #ec4425; | ||
| --systemGreen-light: #34c759; | ||
|
|
||
| // Dark mode colors | ||
| --systemOrange-dark: #ff9f0a; | ||
| --systemPurple-dark: #bf5af2; | ||
| --systemBlue-dark: #0a84ff; | ||
| --systemMint-dark: #27c7d6; | ||
| --systemRed-dark: #ff453a; | ||
| --systemGreen-dark: #30d158; | ||
|
|
||
| // Active colors (default to light mode) | ||
| --systemOrange: var(--systemOrange-light); | ||
| --systemPurple: var(--systemPurple-light); | ||
| --systemBlue: var(--systemBlue-light); | ||
| --systemMint: var(--systemMint-light); | ||
| --systemRed: var(--systemRed-light); | ||
| --systemGreen: var(--systemGreen-light); | ||
| } | ||
|
|
||
| // Switch to dark mode colors | ||
| @media (prefers-color-scheme: dark) { | ||
| :root { | ||
| --systemOrange: var(--systemOrange-dark); | ||
| --systemPurple: var(--systemPurple-dark); | ||
| --systemBlue: var(--systemBlue-dark); | ||
| --systemMint: var(--systemMint-dark); | ||
| --systemRed: var(--systemRed-dark); | ||
| --systemGreen: var(--systemGreen-dark); | ||
| } | ||
| } | ||
| </style> | ||
|
|
||
| <style scoped lang='scss'> | ||
|
|
||
| .TopicTypeIcon { | ||
| width: 1em; | ||
| height: 1em; | ||
| flex: 0 0 auto; | ||
| // use the `--icon-color` if `withColors` is true, otherwise just use gray. | ||
| color: var(--icon-color, var(--color-figure-gray-secondary)); | ||
|
|
||
| :deep(picture) { | ||
|
|
@@ -151,5 +272,48 @@ export default { | |
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| /** | ||
| * Colorful icon badge styling | ||
| * Applied only to symbols with text (classes, structs, methods, etc.) | ||
| */ | ||
| &.colorful-navigator-icon { | ||
| width: 18px; | ||
| min-width: 18px; | ||
| height: 18px; | ||
| padding: 2px 4px; | ||
| border-radius: 4px; | ||
| background-color: var(--systemGreen); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| display: inline-flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same note from above about the SF Pro fonts. |
||
| font-weight: 700; | ||
| font-size: 12px; | ||
| line-height: 1; | ||
| color: white; | ||
|
|
||
| // Hide the original SVG icon | ||
| svg { | ||
| display: none; | ||
| } | ||
|
|
||
| // Display symbol text via pseudo-element | ||
| &::after { | ||
| content: attr(data-symbol-text); | ||
| } | ||
|
|
||
| // Color variants based on symbol type | ||
| &[data-color-variant="purple"] { background-color: var(--systemPurple); } | ||
| &[data-color-variant="blue"] { background-color: var(--systemBlue); } | ||
| &[data-color-variant="mint"] { background-color: var(--systemMint); } | ||
| &[data-color-variant="orange"] { background-color: var(--systemOrange); } | ||
| &[data-color-variant="red"] { background-color: var(--systemRed); } | ||
| &[data-color-variant="green"] { background-color: var(--systemGreen); } | ||
| } | ||
| } | ||
| </style> | ||
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 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.