-
Notifications
You must be signed in to change notification settings - Fork 32
Updated font to Satoshi #350
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces Nacelle fonts with Satoshi variable font. Updates Docusaurus head preload tags accordingly. Adjusts CSS: font-family variables, @font-face, new weight variables, letter-spacing, and minor typography tweaks (menu link weight via font-variation-settings and a sidebar sublist font-size increase). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/css/custom.css (3)
76-82: Drop unnecessary !important on sidebar sublist font-sizeIf not required, avoid increasing specificity debt.
- font-size: 14px !important; + font-size: 14px;
532-536: Scope global letter-spacing to avoid affecting code/monospaceGlobal 0.02em can harm code alignment. Reset for code.
Add (outside this block):
code, pre, kbd, samp { letter-spacing: normal; }
879-888: Add robust fallbacks to font-family variablesImproves resilience if Satoshi fails to load and aligns with system stacks.
- --ifm-font-family-base: 'Satoshi'; - --ifm-heading-font-family: 'Satoshi'; - --ifm-font-family-monospace: 'IBM Mono'; + --ifm-font-family-base: 'Satoshi', Inter, system-ui, -apple-system, "Segoe UI", Roboto, Helvetica, Arial, "Apple Color Emoji", "Segoe UI Emoji", sans-serif; + --ifm-heading-font-family: 'Satoshi', Inter, system-ui, -apple-system, "Segoe UI", Roboto, Helvetica, Arial, "Apple Color Emoji", "Segoe UI Emoji", sans-serif; + --ifm-font-family-monospace: 'IBM Mono', 'IBM Plex Mono', ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
static/fonts/Nacelle/Nacelle-Bold.otfis excluded by!**/*.otfstatic/fonts/Nacelle/Nacelle-Light.otfis excluded by!**/*.otfstatic/fonts/Nacelle/Nacelle-Regular.otfis excluded by!**/*.otfstatic/fonts/Nacelle/Nacelle-SemiBold.otfis excluded by!**/*.otfstatic/fonts/Satoshi/Satoshi-Variable.ttfis excluded by!**/*.ttf
📒 Files selected for processing (2)
docusaurus.config.ts(1 hunks)src/css/custom.css(5 hunks)
🔇 Additional comments (2)
docusaurus.config.ts (1)
101-104: Add WOFF2 asset and update preload
The filestatic/fonts/Satoshi/Satoshi-Variable.woff2is missing; add it and then update the preload link as shown below:- href: "/fonts/Satoshi/Satoshi-Variable.ttf", + href: "/fonts/Satoshi/Satoshi-Variable.woff2", as: "font", - type: "font/ttf", + type: "font/woff2",No
Nacellereferences detected.src/css/custom.css (1)
840-845: Fix @font-face: invalid font-style, add WOFF2 with TTF fallback, split faces, and verify weight range@font-face { font-family: 'Satoshi'; - src: url('/fonts/Satoshi/Satoshi-Variable.ttf') format('truetype'); + src: + url('/fonts/Satoshi/Satoshi-Variable.woff2') format('woff2-variations'), + url('/fonts/Satoshi/Satoshi-Variable.ttf') format('truetype'); - font-weight: 400 680; + font-weight: [axis-min] [axis-max]; /* replace with the variable font’s actual weight‐axis min/max */ font-style: normal; font-display: swap; } + +@font-face { + font-family: 'Satoshi'; + src: + url('/fonts/Satoshi/Satoshi-Variable.woff2') format('woff2-variations'), + url('/fonts/Satoshi/Satoshi-Variable.ttf') format('truetype'); + font-weight: [axis-min] [axis-max]; + font-style: italic; + font-display: swap; +}Manually verify the Satoshi variable font’s weight‐axis range (min/max) and replace
[axis-min] [axis-max]accordingly.
| .menu__link { | ||
| font-family: var(--ifm-font-family-base); | ||
| font-variation-settings: 'wght' 600; | ||
| } |
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.
🛠️ Refactor suggestion
Don’t force weight via font-variation-settings on all menu links
This overrides font-weight (500/600) states and breaks fallbacks. Use font-weight only.
- font-variation-settings: 'wght' 600;
+ /* rely on font-weight declarations above (500) and in active/hover states */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .menu__link { | |
| font-family: var(--ifm-font-family-base); | |
| font-variation-settings: 'wght' 600; | |
| } | |
| .menu__link { | |
| font-family: var(--ifm-font-family-base); | |
| /* rely on font-weight declarations above (500) and in active/hover states */ | |
| } |
🤖 Prompt for AI Agents
In src/css/custom.css around lines 545 to 548, the .menu__link rule is forcing
weight via font-variation-settings which overrides font-weight states and breaks
fallbacks; remove the font-variation-settings declaration and replace it with an
explicit font-weight (e.g. font-weight: 600;) so that normal font-weight
fallbacks and non-variable fonts work correctly, or limit
font-variation-settings to a selector that only applies when you know a variable
font is loaded.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
Style
Performance