-
Notifications
You must be signed in to change notification settings - Fork 0
Port Cathedral color theme into RecursionLab site #5
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
Conversation
…ace a few hard-coded colors with variables, and keep SCSS colors concrete to pass build
…-based base overrides; fix YAML; prep for PR
Reviewer's GuideThis PR ports the Cathedral design system into the Jekyll site by introducing HSL-based CSS custom properties (with dark mode), adding sacred animations and UI enhancements, applying the new tokens site-wide, remapping syntax highlighting to use these variables, and modernizing Sass variables to eliminate hard-coded colors. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- To automatically respect user preferences, consider adding @media (prefers-color-scheme: dark) rules alongside the .dark class for dark mode.
- Centralize your CSS custom property lists into a dedicated theme partial or data map and import/iterate in SCSS to keep your root and .dark overrides DRY.
- The syntax highlighting token mapping is very repetitive—consider consolidating duplicate assignments or generating the mappings programmatically to ease future updates.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- To automatically respect user preferences, consider adding @media (prefers-color-scheme: dark) rules alongside the .dark class for dark mode.
- Centralize your CSS custom property lists into a dedicated theme partial or data map and import/iterate in SCSS to keep your root and .dark overrides DRY.
- The syntax highlighting token mapping is very repetitive—consider consolidating duplicate assignments or generating the mappings programmatically to ease future updates.
## Individual Comments
### Comment 1
<location> `_sass/_base.scss:36` </location>
<code_context>
font-size: .8em;
}
-table td {
- border-bottom: solid 2px #e6e6e6;
-}
+table td { border-bottom: solid 2px mix(#fff, $border-color, 60%); }
.button-dark {
</code_context>
<issue_to_address>
**suggestion:** Mixing #fff with $border-color for table borders may reduce contrast in dark themes.
Consider using a variable or conditional logic to maintain adequate border contrast in both light and dark themes for accessibility.
Suggested implementation:
```
$table-border-color: mix(#fff, $border-color, 60%);
// If you have a dark theme, override $table-border-color for accessibility
// Example using a .dark-theme class (adjust to your theme logic)
.dark-theme {
$table-border-color: mix(#000, $border-color, 60%);
}
table td { border-bottom: solid 2px $table-border-color; }
```
If your theme switching uses CSS custom properties (variables) or a different mechanism, you should adapt the variable assignment accordingly. For example, you might use `var(--table-border-color)` and set that property in your theme root or body selector. If you use SCSS maps or functions for theme management, update those instead.
</issue_to_address>
### Comment 2
<location> `_sass/_base.scss:167-171` </location>
<code_context>
left: -0.2em;
top: -0.2em;
- color: #7a7a7a;
+ color: mix(#fff, $text-color, 50%);
}
blockquote cite {
- color: #999999;
+ color: mix(#fff, $text-color, 60%);
font-size: 0.9em;
display: block;
</code_context>
<issue_to_address>
**suggestion:** Mixing #fff with $text-color for blockquote elements may not adapt well to dark mode.
Consider using theme variables or adjusting the mix ratio to ensure sufficient contrast and readability in dark mode.
Suggested implementation:
```
blockquote cite {
color: var(--blockquote-cite-color, mix(#fff, $text-color, 80%));
font-size: 0.9em;
display: block;
margin-top: 5px;
```
To fully implement this, you should:
1. Define `--blockquote-cite-color` in your CSS root or theme files for both light and dark modes, for example:
```css
:root {
--blockquote-cite-color: #666; /* Light mode */
}
[data-theme="dark"] {
--blockquote-cite-color: #bbb; /* Dark mode */
}
```
2. Adjust the fallback mix ratio (`80%`) if you find it does not provide enough contrast in your design.
</issue_to_address>
### Comment 3
<location> `_sass/_reset.scss:36` </location>
<code_context>
}
&.current {
- color: #fff;
+ color: hsl(var(--primary-foreground));
background: $theme-color;
</code_context>
<issue_to_address>
**issue:** Selection color and background may be confusing if $background-color is light.
Swapping the selection colors or choosing a more distinct highlight will improve legibility, especially with light backgrounds and dark text.
</issue_to_address>
### Comment 4
<location> `assets/css/main.scss:179` </location>
<code_context>
+::-webkit-scrollbar { width: 8px; }
+::-webkit-scrollbar-track { background: hsl(var(--muted)); }
+::-webkit-scrollbar-thumb { background: hsl(var(--primary)); border-radius: 4px; }
+::-webkit-scrollbar-thumb:hover { background: hsl(var(--primary) / 0.8); }
+
+/* Theme overrides to adopt Cathedral palette */
</code_context>
<issue_to_address>
**issue:** The use of hsl(var(--primary) / 0.8) may not be supported in all browsers.
Consider using rgba or providing a fallback to ensure compatibility with browsers that do not support this hsl alpha syntax.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
font-size: .8em; | ||
} | ||
|
||
table td { |
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.
suggestion: Mixing #fff with $border-color for table borders may reduce contrast in dark themes.
Consider using a variable or conditional logic to maintain adequate border contrast in both light and dark themes for accessibility.
Suggested implementation:
$table-border-color: mix(#fff, $border-color, 60%);
// If you have a dark theme, override $table-border-color for accessibility
// Example using a .dark-theme class (adjust to your theme logic)
.dark-theme {
$table-border-color: mix(#000, $border-color, 60%);
}
table td { border-bottom: solid 2px $table-border-color; }
If your theme switching uses CSS custom properties (variables) or a different mechanism, you should adapt the variable assignment accordingly. For example, you might use var(--table-border-color)
and set that property in your theme root or body selector. If you use SCSS maps or functions for theme management, update those instead.
color: mix(#fff, $text-color, 50%); | ||
} | ||
|
||
blockquote cite { | ||
color: #999999; | ||
color: mix(#fff, $text-color, 60%); |
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.
suggestion: Mixing #fff with $text-color for blockquote elements may not adapt well to dark mode.
Consider using theme variables or adjusting the mix ratio to ensure sufficient contrast and readability in dark mode.
Suggested implementation:
blockquote cite {
color: var(--blockquote-cite-color, mix(#fff, $text-color, 80%));
font-size: 0.9em;
display: block;
margin-top: 5px;
To fully implement this, you should:
- Define
--blockquote-cite-color
in your CSS root or theme files for both light and dark modes, for example:
:root {
--blockquote-cite-color: #666; /* Light mode */
}
[data-theme="dark"] {
--blockquote-cite-color: #bbb; /* Dark mode */
}
- Adjust the fallback mix ratio (
80%
) if you find it does not provide enough contrast in your design.
/* Selected elements */ | ||
|
||
::-moz-selection { | ||
color: #fff; |
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.
issue: Selection color and background may be confusing if $background-color is light.
Swapping the selection colors or choosing a more distinct highlight will improve legibility, especially with light backgrounds and dark text.
::-webkit-scrollbar { width: 8px; } | ||
::-webkit-scrollbar-track { background: hsl(var(--muted)); } | ||
::-webkit-scrollbar-thumb { background: hsl(var(--primary)); border-radius: 4px; } | ||
::-webkit-scrollbar-thumb:hover { background: hsl(var(--primary) / 0.8); } |
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.
issue: The use of hsl(var(--primary) / 0.8) may not be supported in all browsers.
Consider using rgba or providing a fallback to ensure compatibility with browsers that do not support this hsl alpha syntax.
This PR ports the Cathedral design system to the Jekyll site.
What’s included
assets/css/main.scss
(:root + .dark).dark
class)_sass/_syntax.scss
$brand-color
mapped to Cathedral primary (HSL), preserving Sass color math compatibility_sass/_base.scss
and_sass/_reset.scss
to reduce hard-coded colorsNotes
Verification
bundle exec jekyll build
PASS.Follow-ups (optional)
Summary by Sourcery
Port the Cathedral design system into the Jekyll site by introducing HSL-based CSS tokens, dark mode support, sacred geometry animations, custom scrollbar, and theme-wide style overrides, while refactoring Sass variables and syntax highlighting to adopt the new palette.
New Features:
.dark
class to override design tokensfadeIn
,sacredPulse
), smooth scroll behavior, and custom scrollbar stylingEnhancements:
_sass/_syntax.scss
$brand-color
Sass variable to the Cathedral primary hue to maintain legacy Sass color mathChores: