Skip to content

Split the stylesheet up, and stop asking Google for the fonts - #121

Open
openipc-ai wants to merge 1 commit into
masterfrom
feat/design-foundation
Open

Split the stylesheet up, and stop asking Google for the fonts#121
openipc-ai wants to merge 1 commit into
masterfrom
feat/design-foundation

Conversation

@openipc-ai

Copy link
Copy Markdown
Collaborator

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 !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 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-primary and
--bs-btn-bg both compile to #4c60d8.

This is the one intended visual change in this PR: primary buttons and links
go from #0d6efd to the brand indigo, and the navbar from indigo to the ink
#0f1422 the 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 than
sitting in the tree as dead CSS.

pages/_legacy.scss holds the pre-relaunch rules verbatim so the cutover can
delete a file rather than pick rules apart. Two are worth naming:

  • The CSS that hides the Russian integrators from non-Russian visitors on
    /introduction. The replacement is a Ruby helper (PR 3), but until that page
    goes this rule is the only thing enforcing it — dropping it early would
    show that wall to everyone, which is specifically not wanted.
  • The h2 page-title style. _base.scss gives article headings the new
    treatment; 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 to
fonts.googleapis.com, one to cdn.jsdelivr.net. Every visitor announced
themselves 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 before
assets:precompile in the image and before CI renders a page, for no benefit.
tools/copy-fonts.mjs (yarn build:fonts) regenerates them and fails loudly if
a file the stylesheet asks for is missing.

JavaScript

One window.onload handler becomes modules under app/javascript/src/. Two
real bugs went with it:

  • It 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 entire gallery — before anything was
    interactive. DOMContentLoaded now.
  • The zoom initialiser called new bootstrap.Modal(...) unconditionally. Most
    pages have 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.

@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 and must never be rewritten by a
v1 yarn, which produces a format the Dockerfile's yarn install --immutable
cannot 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

check result
bin/rails test 231 runs, 873 assertions, 0 failures
compiled CSS selector diff vs master 1 difference: label.required:after::after
--bs-primary / --bs-btn-bg both #4c60d8
@font-face rules 19 (18 IBM Plex + bootstrap-icons), 0 googleapis/jsdelivr references
table.binaries in output 0 — stays retired

The 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

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
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Modularize frontend assets and self-host design fonts

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Split styles into ordered theme, component, page, and legacy partials.
• Self-host IBM Plex and Bootstrap Icons, removing render-blocking third-party requests.
• Modularize JavaScript initialization, fixing load timing and missing-modal failures.
Diagram

graph TD
  PKG["Font packages"] --> COPY["Font copier"] --> FONT["Public fonts"] --> CSS["Style entry"] --> WEB["Browser"]
  VAR["Theme variables"] --> CSS
  JS["JavaScript entry"] --> INIT["Feature modules"] --> WEB
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Pipeline-managed font assets
  • ➕ Adds fingerprinted URLs and standard asset cache invalidation.
  • ➕ Avoids storing generated font copies under public.
  • ➖ Requires integrating package fonts with Sprockets and generated CSS URLs.
  • ➖ Complicates Bootstrap Icons path overrides and asset compilation.
2. Generate fonts during every build
  • ➕ Keeps copied binary assets out of source control.
  • ➕ Automatically reflects dependency upgrades.
  • ➖ Adds ordering requirements before Rails asset compilation and test rendering.
  • ➖ Makes image and CI builds depend on the copying tool and package layout.

Recommendation: Keep the PR's committed public-font approach for this application: it removes third-party requests while preserving deterministic Rails, CI, and container builds. The regeneration script provides an explicit upgrade path and fails on stylesheet/package mismatches; pipeline integration would add complexity without enough benefit for rarely changed files.

Files changed (20) +601 / -263

Enhancement (3) +165 / -0
_fonts.scssDefine self-hosted IBM Plex font faces +44/-0

Define self-hosted IBM Plex font faces

• Declares Sans and Mono faces across Latin, Latin Extended, and Cyrillic subsets with swap rendering. Font URLs now resolve to local files under public/fonts instead of third-party services.

app/assets/stylesheets/_fonts.scss

_variables.scssEstablish Bootstrap theme variables before compilation +88/-0

Establish Bootstrap theme variables before compilation

• Defines the brand palette, typography, spacing, shapes, component defaults, and project-stage colors. Setting both $primary and $blue to brand indigo eliminates Bootstrap's conflicting default blue.

app/assets/stylesheets/_variables.scss

_navbar.scssExtract navbar styling and apply ink theme +33/-0

Extract navbar styling and apply ink theme

• Moves shared navbar, admin-header, GitHub icon, and Telegram image rules into a component partial. The standard navbar now uses the design's dark ink color and themed dropdown state.

app/assets/stylesheets/components/_navbar.scss

Bug fix (5) +106 / -260
application.bootstrap.scssConvert stylesheet entry point into an ordered import manifest +25/-190

Convert stylesheet entry point into an ordered import manifest

• Loads fonts, Bootstrap functions, theme overrides, Bootstrap, icons, and scoped partials in the required order. This removes remote CSS imports and ensures Bootstrap compiles with the intended theme values.

app/assets/stylesheets/application.bootstrap.scss

application.jsInitialize modular frontend behavior on DOM readiness +33/-70

Initialize modular frontend behavior on DOM readiness

• Imports only the Bootstrap components used by current markup and delegates site behavior to focused modules. Replacing window.onload with DOMContentLoaded avoids image-dependent delays and prevents handler replacement.

app/javascript/application.js

confirms.jsExtract defensive destructive-action confirmations +12/-0

Extract defensive destructive-action confirmations

• Encapsulates confirmation listeners for destructive buttons and links. Parent-form traversal now safely handles controls that are not contained in a form.

app/javascript/src/confirms.js

external-links.jsExtract secure external-link initialization +8/-0

Extract secure external-link initialization

• Marks external links to open in a new tab, adds the visual class, and sets rel=noopener to protect the opener context.

app/javascript/src/external-links.js

zoom.jsGuard and modularize image zoom initialization +28/-0

Guard and modularize image zoom initialization

• Initializes Bootstrap Modal only when zoom markup exists, preventing a null-element exception from disabling unrelated behaviors. Delegated clicks continue to support dynamically added zoomable images.

app/javascript/src/zoom.js

Refactor (9) +255 / -0
_base.scssExtract global typography and document defaults +37/-0

Extract global typography and document defaults

• Adds shared body, page-header, link, breadcrumb, required-label, and bidirectional-data rules. The required marker uses the modern ::after syntax and theme danger color.

app/assets/stylesheets/_base.scss

_utilities.scssExtract site utilities and breakpoint debugging +36/-0

Extract site utilities and breakpoint debugging

• Moves sizing, zoom cursor, ratio overflow, offcanvas width, and responsive debug-overlay rules into a dedicated utility partial.

app/assets/stylesheets/_utilities.scss

_cards.scssExtract team card social-icon styling +23/-0

Extract team card social-icon styling

• Moves card positioning and overlaid social-icon presentation into a component-specific partial without changing the existing layout behavior.

app/assets/stylesheets/components/_cards.scss

_calculator.scssIsolate existing calculator page styles +46/-0

Isolate existing calculator page styles

• Preserves calculator form, numeric field, readonly state, and progress-color rules in a page-specific partial. Monospace and brand colors now reference shared theme variables.

app/assets/stylesheets/pages/_calculator.scss

_hardware.scssIsolate supported-hardware page styles +23/-0

Isolate supported-hardware page styles

• Preserves hardware navigation, SoC table alignment, icon sizing, anchors, and featured-row styling. The retired binaries table styles remain excluded.

app/assets/stylesheets/pages/_hardware.scss

_legacy.scssQuarantine pre-relaunch compatibility rules +39/-0

Quarantine pre-relaunch compatibility rules

• Keeps legacy heading presentation and Russian integrator visibility gating together for current pages. The isolated partial can be removed atomically when the relaunch replaces those pages.

app/assets/stylesheets/pages/_legacy.scss

_openwall.scssIsolate Open Wall carousel captions +12/-0

Isolate Open Wall carousel captions

• Moves the gallery caption transparency, hover contrast, and link color behavior into a page-specific partial.

app/assets/stylesheets/pages/_openwall.scss

heif-viewer.jsExtract delegated HEIF viewer behavior +31/-0

Extract delegated HEIF viewer behavior

• Moves client-side HEIF fetching, decoding, status updates, and fallback handling into a dedicated initializer while retaining delegated click handling.

app/javascript/src/heif-viewer.js

timestamps.jsExtract localized timestamp rendering +8/-0

Extract localized timestamp rendering

• Moves Unix timestamp conversion into a focused initializer that renders values using each visitor's locale and timezone.

app/javascript/src/timestamps.js

Other (3) +75 / -3
application.html.erbRemove obsolete Turbo tracking metadata +1/-1

Remove obsolete Turbo tracking metadata

• Drops the inert data-turbo-track attribute from the deferred application JavaScript include after removing unused Turbo dependencies.

app/views/layouts/application.html.erb

package.jsonReplace unused Hotwired packages with local font sources +3/-2

Replace unused Hotwired packages with local font sources

• Removes unused Turbo and Stimulus dependencies, adds IBM Plex Fontsource packages, and exposes a build:fonts regeneration command.

package.json

copy-fonts.mjsAdd deterministic self-hosted font regeneration +71/-0

Add deterministic self-hosted font regeneration

• Copies the exact IBM Plex subsets, weights, and Bootstrap Icons formats required by the stylesheets into public/fonts. The command reports all missing package files and exits unsuccessfully to prevent silent font fallbacks.

tools/copy-fonts.mjs

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Fonts bypass asset pipeline 📘 Rule violation ☼ Reliability
Description
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.
Code

app/assets/stylesheets/_fonts.scss[29]

+      src: url('/fonts/ibm-plex-sans-#{$subset}-#{$weight}-normal.woff2') format('woff2');
Evidence
Rule 2 requires all application asset references to use the Rails asset pipeline or configured build
outputs. The stylesheet hardcodes /fonts/..., while tools/copy-fonts.mjs explicitly copies those
files to public/fonts and states that they do not pass through Sprockets or run as part of any
build.

CLAUDE.md: Route Asset References Through the Rails Asset Pipeline
app/assets/stylesheets/_fonts.scss[22-40]
tools/copy-fonts.mjs[1-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## 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


Grey Divider

Context sources

Grey Divider

Tip of the day
💡 Did you know, you can hide the parts of a finding you never read, like the evidence or the agent prompt

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

font-style: normal;
font-display: swap;
font-weight: $weight;
src: url('/fonts/ibm-plex-sans-#{$subset}-#{$weight}-normal.woff2') format('woff2');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant