Split the stylesheet up, and stop asking Google for the fonts - #121
Split the stylesheet up, and stop asking Google for the fonts#121openipc-ai wants to merge 1 commit into
Conversation
The site's CSS was one 200-line file appended to Bootstrap, and the order it was written in hid a bug. Our theme values landed *after* the Bootstrap import, where Bootstrap's own !default declarations had already won, so $primary stayed at Bootstrap's #0d6efd while :root separately declared --bs-blue: #4c60d8. The page carried two blues: buttons and links in one, headings and the navbar in the other. Nobody had written the wrong colour anywhere -- the file just could not express the intent in that order. So the entry point is now only an ordering statement: fonts, Bootstrap's functions, our variables, Bootstrap, then our own partials. --bs-primary and --bs-btn-bg both compile to #4c60d8 now. $blue is set alongside $primary because --bs-blue is what the pre-relaunch rules colour headings with, and leaving it at the default would have reproduced the same split from the other side. The partials that arrive here are the ones that style pages which exist today. Components and page styles belonging to the new pages come with those pages, rather than sitting in the tree as dead CSS in the meantime. pages/_legacy.scss holds the rules that belong to the pre-relaunch pages, kept verbatim and kept together so the cutover can delete a file instead of picking rules apart. One of them is the CSS that hides the Russian integrators from non-Russian visitors on /introduction: the replacement is a Ruby helper, but until that page goes this rule is the only thing enforcing it, and dropping it early would show that logo wall to everyone. The other legacy rule worth naming is the h2 title style. _base.scss gives article headings the new treatment; the legacy selector is longer and therefore wins, and it applies exactly to pages rendered inside the .container wrapper -- which is every page that is not full-bleed. The two rules split along the same line the layout's fullwidth switch will. FONTS. The stylesheet opened with three @import url() lines: two to fonts.googleapis.com and one to cdn.jsdelivr.net. Every visitor announced themselves to two third parties before the page could paint, and first paint sat behind DNS lookups we do not control. This is the same argument that moved the legacy logo off a maintainer's personal CDN in d60729a. IBM Plex and bootstrap-icons are now served from public/fonts, subset to latin, latin-ext and cyrillic; Chinese falls back to the system stack, because IBM Plex has no CJK glyphs and the families that do are megabytes. The files are committed, since public/ bypasses Sprockets and a generated file would have to exist before assets:precompile in the image and before CI renders a page, for no benefit. tools/copy-fonts.mjs regenerates them from the @fontsource packages and fails loudly if a file the stylesheet asks for is missing. JAVASCRIPT. One window.onload handler becomes modules under app/javascript/src. That handler assigned window.onload rather than adding a listener, so any second assignment would have silently replaced all of it, and it waited on every image -- on the Open Wall, the whole gallery -- before anything became interactive. It is DOMContentLoaded now. The zoom initialiser called `new bootstrap.Modal(document.getElementById(...))` unconditionally. Most pages have no zoomable image and so no #modalZoom, the constructor threw on null, and because every initialiser shared the one handler, the throw took external links, timestamps and confirmations down with it on those pages. It returns early now. @hotwired/turbo-rails and @hotwired/stimulus are removed: neither was ever imported. The layout's data-turbo-track goes with them, having been inert. The lockfile is regenerated with Yarn 4 -- it must never be rewritten by a v1 yarn, which produces a format `yarn install --immutable` in the Dockerfile cannot read. Bootstrap is imported per component rather than wholesale. 189.4 KB to 181.8 KB unminified, which is a small win; the point is the import list, which now states what the markup actually depends on. Audited against the views first: dropdown, collapse, offcanvas, carousel and Modal are used, and nothing uses tooltip, popover, tab, alert or scrollspy. Verified: 231 runs, 873 assertions, 0 failures. Diffing the compiled selector set against master's leaves exactly one difference -- label.required:after, which is now spelled ::after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hvzXBErEjEGMyRme2K3hi
PR Summary by QodoModularize frontend assets and self-host design fonts
AI Description
Diagram
High-Level Assessment
Files changed (20)
|
Code Review by Qodo
1. Fonts bypass asset pipeline
|
| font-style: normal; | ||
| font-display: swap; | ||
| font-weight: $weight; | ||
| src: url('/fonts/ibm-plex-sans-#{$subset}-#{$weight}-normal.woff2') format('woff2'); |
There was a problem hiding this comment.
1. Fonts bypass asset pipeline 📘 Rule violation ☼ Reliability
The new @font-face rules reference /fonts/*.woff2 directly, and the fonts are manually committed under public/ rather than produced or resolved by the configured asset build pipeline. This violates the required asset-routing mechanism and makes delivery depend on separate static-file-server behavior.
Agent Prompt
## Issue description
The new font URLs bypass the Rails asset pipeline and refer directly to manually committed files under `public/fonts`.
## Issue Context
Production disables runtime asset compilation, and compliance rule 2 requires application assets to use Rails asset helpers or be produced by the configured CSS/JavaScript build pipeline. Move the font files into a pipeline-managed source location or add a build step that emits them as configured build outputs, then generate resolved URLs from SCSS rather than hardcoding `/fonts`.
## Fix Focus Areas
- app/assets/stylesheets/_fonts.scss[29-40]
- app/assets/stylesheets/application.bootstrap.scss[15-19]
- tools/copy-fonts.mjs[23-60]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Second of five PRs rebasing the June 11 relaunch onto master. Depends on #120
for the container the numbers below were measured in, but does not conflict with
it. No new pages and no IA change — this is the design foundation the later
PRs build on.
The two blues
The CSS was one 200-line file appended to Bootstrap, and the order it was
written in hid a bug: our theme values landed after the Bootstrap import,
where Bootstrap's
!defaultdeclarations had already won. So$primarystayedat Bootstrap's
#0d6efdwhile:rootseparately declared--bs-blue: #4c60d8. The page carried two blues — buttons and links in one,headings and navbar in the other. Nobody had written the wrong colour anywhere;
the file just could not express the intent in that order.
The entry point is now only an ordering statement.
--bs-primaryand--bs-btn-bgboth compile to#4c60d8.This is the one intended visual change in this PR: primary buttons and links
go from
#0d6efdto the brand indigo, and the navbar from indigo to the ink#0f1422the new design uses.Scope discipline
Only partials that style pages which exist today land here. Components for the
new pages (
sections,terminal,stage-badge,logo-wall,browser-frame,the pillar/project cards,
pages/_home) come with those pages, rather thansitting in the tree as dead CSS.
pages/_legacy.scssholds the pre-relaunch rules verbatim so the cutover candelete a file rather than pick rules apart. Two are worth naming:
/introduction. The replacement is a Ruby helper (PR 3), but until that pagegoes this rule is the only thing enforcing it — dropping it early would
show that wall to everyone, which is specifically not wanted.
h2page-title style._base.scssgives article headings the newtreatment; the legacy selector is longer and wins, and applies to exactly the
pages rendered inside
.container— i.e. every page that is not full-bleed.The two rules split along the same line the layout's fullwidth switch will.
Fonts
The stylesheet opened with three
@import url()lines — two tofonts.googleapis.com, one tocdn.jsdelivr.net. Every visitor announcedthemselves to two third parties before the page could paint, and first paint sat
behind DNS we do not control. Same argument that moved the legacy logo off a
maintainer's personal CDN in d60729a.
IBM Plex and bootstrap-icons now come from
public/fonts, subset to latin,latin-ext and cyrillic. Chinese falls back to the system stack — IBM Plex has no
CJK glyphs and the families that do are megabytes. Files are committed:
public/bypasses Sprockets, so a generated file would have to exist beforeassets:precompilein the image and before CI renders a page, for no benefit.tools/copy-fonts.mjs(yarn build:fonts) regenerates them and fails loudly ifa file the stylesheet asks for is missing.
JavaScript
One
window.onloadhandler becomes modules underapp/javascript/src/. Tworeal bugs went with it:
window.onloadrather than adding a listener, so any secondassignment would have silently replaced all of it. And it waited on every
image — on the Open Wall, the entire gallery — before anything was
interactive.
DOMContentLoadednow.new bootstrap.Modal(...)unconditionally. Mostpages have no
#modalZoom, the constructor threw on null, and because everyinitialiser shared the one handler, the throw took external links,
timestamps and confirmations down with it on those pages.
@hotwired/turbo-railsand@hotwired/stimulusare removed — neither was everimported. The layout's
data-turbo-trackgoes with them, having been inert.v1 yarn, which produces a format the Dockerfile's
yarn install --immutablecannot read.
Bootstrap is imported per component. 189.4 KB → 181.8 KB unminified, a small
win; the point is the import list, which now states what the markup depends on.
Audited against the views first — dropdown, collapse, offcanvas, carousel and
Modal are used; tooltip, popover, tab, alert and scrollspy are not.
Verification
bin/rails testlabel.required:after→::after--bs-primary/--bs-btn-bg#4c60d8@font-facerulestable.binariesin outputThe selector diff is the check worth repeating on review: every rule master
emitted is still emitted, so the restructure dropped nothing.
🤖 Generated with Claude Code
https://claude.ai/code/session_015hvzXBErEjEGMyRme2K3hi