Add unified icon system by lokesh · Pull Request #12955 · internetarchive/openlibrary

lokesh · 2026-06-17T18:22:05Z

Part of #13240 — Epic: UI Modernization (Foundation workstream).

Open Library's icons are currently a patchwork: some are PNG images from the 2010s, some are one-off SVG files, some are SVG markup pasted inline into JavaScript. They come from different families, sit at different weights, and each one is its own file to download.

This PR replaces that with a single icon system: 43 icons, one visual family, one small file, and one obvious way to use them. [feature]

Docs, a searchable gallery, and the API reference live at /developers/design/icons.

What patrons get

Icons that look like they belong together. Every glyph is drawn on the same 24×24 grid at the same stroke weight. Today the arrows in the editions table, the header hamburger, and the eye toggle on the login form are three different visual languages; after this they're one.
Crisp icons on modern screens. Nine of the icons being replaced were PNG images — including every sort arrow and pager arrow in the editions table. Raster icons blur on high-DPI phones and laptops, which is most traffic. These are now vectors, sharp at any zoom level.
A slightly faster page. 21 separate icon files (15.4 KB) collapse into one 10.9 KB sprite — 2.0 KB gzipped — that the browser downloads once and reuses on every page for the rest of the session. Server-rendered icons need no JavaScript at all, so they're painted in the first frame and never cause layout shift.
Icons that respect the reader's settings. Sizes are in rem, so icons grow when someone bumps their browser font size instead of staying stuck at 16px next to enlarged text. Every glyph inherits the surrounding text color, so it stays legible wherever it sits.
Better screen-reader behavior. Decorative icons are properly hidden (aria-hidden) instead of announcing a filename; meaningful ones take a real translated label. The old <img alt="..."> pattern got this wrong in both directions.

What developers get

One obvious way to add an icon. $:macros.icon("search"). That's the whole API in a template. Previously you'd hunt for a file path, guess a size, and hand-write <img> with the right classes.
A gallery you can search. /developers/design/icons lists every available icon with a click-to-copy call. No more grepping static/images/ to find out whether a chevron already exists.
It works in Jinja too. {{ icon("search") }} — same macro, no imports, no passing it through as a render kwarg.
One command, no toolchain. make icons rebuilds everything from the source SVGs. It's a single dependency-free Node script, already part of npm run build-assets. No icon font, no npm package, no bundler plugin, no runtime library.
Not locked to a vendor. Most geometry is vendored from Lucide (ISC-licensed), but it's copied in as plain files, not depended on. Source folders record provenance and are auto-discovered — swapping a glyph or adding a new source library changes one file and no call sites.

How it works

One thing you edit, one command, two outputs:

static/icons/src/<source>/*.svg          ← the only thing you hand-edit
        │
        │  make icons   (scripts/build_icon_sprite.mjs — plain Node, zero deps)
        ▼
├─ static/icons/sprite.svg           served content-hashed; what the macro's <use> points at
└─ …/lit/icons.generated.js          glyph templates <ol-icon> renders from

Neither output is committed — both are gitignored. npm run build-assets runs make icons, and make lit-components depends on it, so the Lit bundle can't be built from a stale glyph module.

The sprite URL includes a content hash (sprite.svg?v=abc123) so browsers can cache it forever and still pick up changes when it's rebuilt. The macro emits a plain <svg><use href="…sprite.svg#icon-search"> pointing at that URL, and a <meta name="ol-icon-sprite"> tag in the page head exposes the same URL to client-side JS.

Two ways to draw an icon

Both read the same source files and look identical. You pick based on where the icon lives:

1. The macro — server-rendered templates. This is the default and covers most cases.

$:macros.icon("search")                                  $# Templetor
{{ icon("globe", size="lg", label=_("Language")) }}      {# Jinja #}

2. The <ol-icon> element — anything drawn by client-side JS, including inside a web component's shadow DOM.

<ol-icon name="globe" size="lg" label="Language"></ol-icon>

It's registered site-wide by the Lit bundle, so plain markup and runtime-built markup need no import. Inside a Lit component, import './OlIcon.js' and use it in a template like any other element.

What you control: size (sm/md/lg = 16/20/24px), color, and — where a component wants a non-default — stroke weight via --ol-icon-stroke-width.

Why two and not one

This is the one place the system isn't as simple as I'd like, so it's worth stating plainly: a <use> reference cannot cross a shadow-DOM boundary. That's a browser rule, not a design choice. Without shadow-DOM components, this PR would be just the sprite and the macro.

So <ol-icon> inlines the glyph into its own shadow root rather than pointing at the sprite, which is why the build emits a glyph module alongside it. Both come from the same sources — there's no second set of icons to keep in sync. And the macro stays because an element has to wait for JavaScript, which a server-rendered icon shouldn't.

Earlier revisions of this branch had a third path: components imported a named glyph and hand-wrote the <svg> wrapper around it. That's gone. The wrapper's attributes were duplicated at every call site and free to drift, and <ol-icon> does the same job in one tag.

Also rejected

Inlining the sprite into every page. Simplest possible <use> story, but it repeats the payload on every response. A hashed external file caches once for the whole session.
Using <ol-icon> for server-rendered icons too. One path instead of two, but every icon on the page would then wait on JavaScript and reserve a box that fills in a frame later. The macro covers those without the wait.
Committing the generated outputs. This branch did that for a while: an uncommitted asset means an environment that skips the build renders blank icons while the code looks perfectly correct. But it cost a freshness check in both CI and pre-commit to keep the outputs from drifting from their sources, and a guaranteed merge conflict on every concurrent icon PR. The build is a zero-dependency Node script that runs in well under a second and is wired into build-assets, so paying it is cheaper than policing the outputs. icon_sprite_url() falls back to the unhashed path when the sprite is missing, so a checkout that never ran make icons draws blank icons rather than 500ing every page that has one.

Scope

43 icons, adopted across 17 templates and 12 Lit components — the header, search, book and edition pages, My Books, the markdown editor, dialogs, and the signup/login forms. The editions table also moves off its PNG-era DataTables chrome onto <ol-pagination> and sprite sort arrows.

Remaining surfaces still use their old images and are unaffected; they can be converted incrementally. Nothing in this PR requires a follow-up to be correct.

The 50 now-dead image files — the 21 replaced here, plus 29 that had already been dead for years and were found while auditing — are deleted in a follow-up rather than here. They're the only binary content the branch had, and GitHub's .diff omits binary data, which forced every patch deploy to pass --exclude=static/images/* or abort outright. Unreferenced files on disk cost nothing in the meantime.

How to review this

119 files is misleading — 43 of them are one-line SVG sources, with nothing to read:

	files	lines
New icon sources	43	+43
Design docs (page + `design.py` + its JS)	5	+550 / −117
CSS	19	+339 / −149
Lit components	12	+183 / −78
Template call sites	17	+68 / −37
App JS	4	+173 / −43
Sprite build script	1	+163
Other	18	+161 / −72

The 17 call-site templates are one-line swaps of an <img> or an inline <svg> for a macro call, so they're fine to skim. Two things are worth real attention:

The experience of adding an icon. Drop an SVG into static/icons/src/lucide/, run make icons, call $:macros.icon("name"). If that flow is awkward it stays awkward for everyone after us, so it's the part most worth pushing back on.
How it looks. Icons are on a lot of pages now, and eyes catch what tests can't.

Nothing generated is checked in, so there's nothing that can silently drift from its sources: make icons is the only way to produce the sprite and the glyph module, and both are gitignored. The gallery on the design page reads the source SVG file names directly for the same reason.

One thing here isn't icon-specific, and is the reason the docs shouldn't rot: /developers/design now has three sections whose content is derived rather than hand-written — design tokens parsed out of the token CSS, Lit API tables from the Custom Elements Manifest, and the icon gallery from the icon sources. Icons are the third consumer of that idea, not a new mechanism.

Testing

Have a browse around testing and make sure everything still looks right — icons show up in a lot of places, so general clicking around is more useful here than a checklist. Then take a look at the Icons section of the design docs at /developers/design/icons, which is where the gallery and the usage guidance live.

Pulling the branch locally: the sprite and the glyph module are generated, so run npm run build-assets (or just make icons if that's all you need) once after checkout. If icons come up blank anywhere, that's the build not having run.

Stakeholders

@cdrini
cc: @mekarpeles @jimchamp

One curated set on a 24x24 / 2px / currentColor grid, replacing the ad-hoc mix of PNG sprites, file SVGs, and inline-SVG-in-JS. Source of truth: static/icons/src/{lucide,custom}/. scripts/build_icon_sprite.mjs (make icons) emits three outputs from it: - static/build/icons/sprite.svg - <symbol> sheet for the document/light DOM - static/icons/manifest.json - committed icon-name list (gallery + lint) - openlibrary/components/lit/icons.generated.js - committed Lit svg fragments for shadow-DOM components (which can't reach the document sprite via <use>) Delivery: - $:icon(name, size, label) Templetor macro for server templates - <ol-icon> Lit component (light DOM) for JS/light-DOM contexts Both use a same-document <use href="#name">; the sprite is inlined once per page via the icon_sprite() helper (footer), since external <use href="file.svg#id"> is unreliable on the Safari/iOS versions in our browserslist. Shadow-DOM components inline glyphs imported from icons.generated.js instead; ol-dialog's close icon is migrated as the first example. Also: icon-size tokens (sm/md/lg), shared ol-icon.css, an icon gallery with copy-to-clipboard at /developers/design#icons, and a pre-commit/npm check that the generated sprite/manifest/module stay in sync with the sources.

Replace inline SVG glyphs with imports from the generated icon module: - OLMarkdownEditor: all 19 toolbar glyphs (wrapped in the editor's <svg> shell so toolbar CSS still governs size/stroke-width) - Close (X) icon deduped across OLChip, OlBanner, OlToast, and SearchModal to the canonical `x` glyph (removes four hand-written copies) Geometry is unchanged (the icon set was harvested from these components), so the rendered output is identical. Verified in-browser: editor toolbar, dialog/chip/ banner/toast/search-modal close icons all render as real SVG elements.

Size the ol-icon host element from its size attribute in render-blocking CSS (a plain `ol-icon` selector, so it applies before and after the component upgrades). The box is held from first paint, so no surrounding content reflows when the Lit bundle runs. The glyph still appears on upgrade — server-rendered templates that need instant, JS-free icons should use the $:icon() macro.

…gn page Add a 'which technique to use' comparison table, usage examples for all three paths, and a 'what you control' section spelling out sizing (sm/md/lg tokens + numeric escape hatch) and color (currentColor — set color on any ancestor).

@prop

A const declaration sat between the JSDoc block and the class, so the analyzer attached the @prop descriptions to the const instead of OlIcon — leaving the design-page API table empty. Move it above the doc comment so the table renders name/size/label with their descriptions.

…ns anywhere The icons-generated pre-commit hook ran the generator, which imported svgo — a devDependency absent in CI/containers/fresh checkouts, so the hook failed with ERR_MODULE_NOT_FOUND. The source SVGs are authored clean, so svgo was only merging paths; the generator already keeps just the attributes it needs. Drop svgo from the generator (pure Node built-ins now) and remove the unused sprite SVGO config. Glyph geometry is unchanged (paths are simply no longer merged).

…n-system

…n-system # Conflicts: # static/css/page-design.css

accesslint

Found 1 issue across 1 rule.

accesslint · 2026-07-18T06:55:19Z

+      placeholder="Filter $len(icons) icons…"
+      aria-label="Filter icons"
+    >
+    <ul class="icon-gallery" id="icon-gallery">


WCAG 1.3.1: <ul> contains direct text content. Wrap in <li>.

<ul> and <ol> must only contain <li>, <script>, or <template> as direct children.

Details

Screen readers announce list structure ('list with 5 items') based on proper markup. Placing non-<li> elements directly inside <ul> or <ol> breaks this structure. Wrap content in <li> elements, or if you need wrapper divs for styling, restructure your CSS to style the <li> elements directly.

…ent foundations A broad snapshot of in-flight design-system work on this branch. - Icons: unified <ol-icon> / $:icon() sprite system over static/icons/src, with a build-time sprite plus generated fragment module for shadow DOM. - Design docs: /developers/design rebuilt into Foundations / Components / Icons / Playground, with tokens and component APIs derived rather than hand-maintained. Icon size presets now document their pixel values (sm 16px, md 20px, lg 24px) and that the 2px grid stroke scales with them. - Web components: focus/tabbing utilities, FocusableHostMixin, FormAssociatedMixin, keyboard-nav helpers, new OlMenuPopover and OlSegmentedControl, and a11y tests across the Lit layer. - Tokens/CSS: token contrast tooling and wide stylesheet updates. - Also carries subject-page macros, results-toolbar sort work, and assorted plugin/template changes. Claude-Session: https://claude.ai/code/session_01XfFpmRvFnGA4zTwUpokNzW

accesslint

👏 All fixed. Great work.

…n-system # Conflicts: # openlibrary/components/lit/OlDialog.js # openlibrary/components/lit/OlToast.js # openlibrary/components/lit/custom-elements.json # openlibrary/plugins/openlibrary/design.py # openlibrary/plugins/openlibrary/js/design-system/index.js # openlibrary/templates/design.html # static/css/page-design.css

djLint H021 flagged the Color demo's inline `style="color: ..."`, which was the last lint failure on the icons page. Swap it for a .ds-demo-danger class alongside the other ds-demo-* helpers, and show the reader the CSS rule in the blurb so the snippet teaches the real technique rather than an inline style nobody should copy. Claude-Session: https://claude.ai/code/session_01XfFpmRvFnGA4zTwUpokNzW

…ite early Review follow-ups on the icon system: - Docs and the gallery's copy button taught `$:icon("search")`, which is not a global — Templetor exposes disk macros only as `$:macros.icon(...)`, so every documented snippet raised NameError when pasted into a template. - Sprite <symbol> ids were unprefixed, putting document-global ids like #search and #code on every page. Any colliding id earlier in the document won <use> resolution and the icon silently rendered blank. Namespaced to icon-<name>; the names callers pass and manifest.json are unchanged. - The sprite was inlined just before </body>, so icons above the fold had nothing to resolve against until the response finished streaming. It is now the first thing in the body (~1.5KB gzipped). - <ol-icon size="32"> styled the inner <svg> but not the host, painting a 32px glyph inside a 20px box. Raw pixel sizes are gone; a one-off is a CSS job, which also gets the box right before the component upgrades. - The macro emitted 18 leading newlines, one per comment line above its output; its docs now sit below it. - The drift-check hook matched a path that does not exist and did not guard the generated outputs it exists to protect. - The build script now rejects icon names that are not kebab-case or that collide as JS identifiers, naming the offending source file rather than failing later inside a generated one. Claude-Session: https://claude.ai/code/session_01XnwG8xofUcTkTSyUaz7fu7

Jinja templates are the preferred target for new code but had no way to draw an icon: Infogami exposes disk macros only through Templetor's `macros` namespace, so the design page had to hand `icon=macros.icon` to render_jinja_template as a kwarg, and any other template would have had to do the same. Registers `icon` in the Jinja env instead, wrapping the same macro and returning Markup so it survives autoescape. Templates now write `{{ icon("search") }}` against the same sprite the Templetor macro uses. The design page drops its per-call-site shim and documents both forms. Claude-Session: https://claude.ai/code/session_01XnwG8xofUcTkTSyUaz7fu7

The sprite shipped on every page but nothing outside /developers/design referenced it — $:macros.icon() had no production callers, and the glyphs patrons actually saw were hand-copied paths, <img> icons that could not inherit color, and a second icon vocabulary. Deduped 20 hand-copied glyphs. Six Lit components now import from icons.generated.js rather than carrying their own path data; the chevron-down path alone existed verbatim in four files. Converted 22 <img> icons to sprite references: all 12 right-chevron sites, the password-visibility eye (password-toggle.js now repoints <use href> instead of swapping img.src), signup's success check, the language globe, search-inside, the barcode scanner, the edition nav arrows and the hamburger. As images these could not inherit currentColor — two of them hardcoded internetarchive#666/#8a7f71 and ignored the theme outright. Retired the second vocabulary: PreviewSearchInside drew three Material Symbols glyphs (viewBox="0 -960 960 960", filled, one with a hardcoded fill) next to Lucide stroke icons in the same row on the book page. Supporting changes: - The macro emits a fixed class list, which blocked nearly every call site — they carry load-bearing hooks (.down-arrow's rotation, .sas-avatar__icon's sizing, .search-icon). Added an `extra_class` passthrough, mirrored in the Jinja global, so adopting the system does not mean rewriting CSS at each site. - icon_link() takes an icon name instead of an image src. Migrating only its share icon would have left a stroke glyph between two 700pt filled illustrations, so Review and Notes moved with it. - Four new glyphs, taken from lucide-static rather than redrawn: share-2, scan-barcode, message-square-text, sticky-note. - Dropped three shape-describing alt strings from the catalog ("right chevron", "search inside icon", "Search Inside Icon"). layout_options.html deliberately keeps its inline glyphs, with a comment saying why: ol-segmented-control harvests each segment's innerHTML and re-renders it inside its shadow root, where a same-document <use> resolves to nothing. Glyphs drawn through the macro take the sprite's baked stroke-width of 2, so the browse chevron goes from 2.5 to 2. The Lit components keep their own wrapper stroke-width and are unchanged. Twelve image assets are now unreferenced and left in place for a separate cleanup pass alongside the 30 that were already dead. Claude-Session: https://claude.ai/code/session_01XfFpmRvFnGA4zTwUpokNzW

Rolldown (Vite 8) only honors /*#__PURE__*/ before a call or new expression - before a tagged template it warns INVALID_ANNOTATION and keeps unused glyphs in the bundle. Wrap each glyph in an IIFE so the annotation precedes a real call; terser understands the same form, so the webpack build keeps its tree-shaking hint too. Claude-Session: https://claude.ai/code/session_01V312UXw8oNocbu5SkC1A42

The sprite was inlined into every page body; now it's one external content-hashed file cached sitewide. icon_sprite() becomes icon_sprite_url(), the macro and <ol-icon> reference <use href="{sprite}#icon-name">, and client JS picks the URL up from a <meta name="ol-icon-sprite"> tag in site/head. Also: delete the legacy one-off SVGs now replaced by the sprite, drop the unused chevron-up glyph, thicken the scan-barcode icon and match its header color to the search magnifier, and trim the language-dropdown labels to the native name only. Claude-Session: https://claude.ai/code/session_01RjCpiDb519vwQx4yDAKpra

These 28 files were unreferenced before the icon-system branch — no template, CSS, JS, or Python mentions them, nothing constructs their paths dynamically, and the service worker caches by URL pattern rather than a file list. Claude-Session: https://claude.ai/code/session_01V312UXw8oNocbu5SkC1A42

Replace the last PNG sprite consumers with the icon system: sort state in the editions table header now renders arrow-up-down / arrow-up-narrow-wide / arrow-down-wide-narrow glyphs driven purely by DataTables' sorting classes, and the full_numbers pager is dropped from the `dom` string in favor of an <ol-pagination> synced to the table's paging state. The nine legacy sort/paginate PNGs and their CSS are deleted, along with the hand-rolled arrow-toggling jQuery. Along the way: - Add per-size stroke tokens (--icon-stroke-sm/md/lg); the sprite build now strips stroke-width from each <symbol> so the weight inherits from the referencing <svg>, letting `lg` pull back to 1.75px instead of a full 2px. - Auto-discover source groups under static/icons/src/ instead of hardcoding lucide/custom, and rename LICENSE-lucide.md to ATTRIBUTION.md to match the library-agnostic framing. - Swap the ✏️ emoji on the subject-edit button for the pencil glyph. - Document stroke behavior and glyph provenance on /developers/design. Claude-Session: https://claude.ai/code/session_01RjCpiDb519vwQx4yDAKpra

Pull the how-to out of the provenance blurb into its own numbered section: author in the canonical form, drop the kebab-case file in the folder matching its source, run `make icons`, commit the two generated files alongside it. Claude-Session: https://claude.ai/code/session_01RjCpiDb519vwQx4yDAKpra

Several comment blocks on this branch had grown to explain more than the code needs — the sprite build's module header ran 27 lines, <ol-icon>'s class JSDoc 22. Trim each to the fact you can't recover by reading the code (why stroke-width is stripped from the symbol, why the sprite meta isn't a rel=preload, why the segmented control's glyphs stay inline, why the IIFE wraps the PURE annotation) and drop the restatement around it. Regenerates icons.generated.js, whose header the build script emits. The /developers/design prose is untouched — that's the documentation itself, not commentary on code. Claude-Session: https://claude.ai/code/session_01AzJjBQ55TN5JbSkaCiLFUt

Clicking a glyph used to copy the Templetor macro with no hint that it had. Now it opens a shared top-layer panel (native Popover API) showing all four usage forms — Templetor, Jinja, Lit import, <ol-icon> — each in a recessed well that copies on click anywhere in it, with a copy glyph that flips to a check. Browsers without the Popover API keep the old copy-on-click. Adds Lucide's `copy` icon to the set for the wells. Claude-Session: https://claude.ai/code/session_01V312UXw8oNocbu5SkC1A42

The svg chevron was still carrying two PNG-era pixel nudges (.icon-link__image's top: 3px plus a 6px top margin), landing it ~9px low. Center it with flex on .dropdown instead; the scoped selector is needed because mybooks.css loads later and would re-apply the nudge. Claude-Session: https://claude.ai/code/session_01V312UXw8oNocbu5SkC1A42

The sprite was the only one of the build script's three outputs left uncommitted, so a checkout that hadn't run `make icons` rendered blank icons everywhere. That is what happens on testing.openlibrary.org: patch deploys apply a diff and restart, they never run a build, so the sprite 404s and every glyph draws empty. Committing it removes the state where the code is present but the asset isn't — testing, fresh clones and local dev all work with no build step. The manifest and the Lit glyph module were already committed, so this makes the three outputs consistent rather than introducing a new idea. It moves to static/icons/ because static/build/ is gitignored wholesale and `make clean` does `rm -rf $(BUILD)`, which would delete a committed file. All four icon artifacts now live in one directory. Knock-on simplifications: icon_sprite_url() loses the fallback that existed only for the missing-sprite case, the Makefile target loses the atomic-swap dance that kept a half-written sprite from being served, and --out loses its only caller. Symbols are now one per line so the committed file diffs readably and conflicts can be resolved by rebuild. check:icons and the pre-commit hook both gained the sprite path — with the file committed, drift is now possible and those guards are what catch it. Claude-Session: https://claude.ai/code/session_01Lv9GuFfV3JRG7hmKGmN1t4

These 50 files are the only binary content in the PR, and GitHub's .diff omits binary data — so `git apply` aborts the entire patch unless the deploy passes --exclude=static/images/*. That one flag was the last thing standing between this branch and an ordinary patch deploy to testing. The deletions don't need to ship here. 29 of these have been dead for years; the other 21 stop being referenced as soon as this PR's call sites move to the macro. Unreferenced files on disk hurt nothing, so they move to a follow-up that never has to reach testing. Also drops the PR from 170 files to 120. Claude-Session: https://claude.ai/code/session_01Lv9GuFfV3JRG7hmKGmN1t4

…n-system

…n-system Claude-Session: https://claude.ai/code/session_011N2wcn6DdjDWcGUoHz7RC9

cdrini

Niiice! This is a great cleanup 😊

A few thoughts:

(1) I wonder if we can reduce from 3 ways to 1...
Not sure if this is feasible, but was thinking that we might be able to switch to using <ol-icon name="foo"> everywhere, by:
- For pre-js, we can generate a sprite sheet as a .css file, which sets the background/width/height/etc of the ol-icon[name="foo"]:not(:defined) { background: 'data:...'}
- For post-js, we can have lit components with the SVG in them, which should give us full control over the display.

This would mean we would have exactly two copies of the sprite sheet sent over once as CSS once as JS, but might be worth it if we can have only one way to do icons that works everywhere.

(2) There are a lot of generated files being commited into the repo. Can we remove some of these?

For icons.generated.js, can we potentially have vite plugin to handle importing the svg icons? Something like this according to claude might work:

function svgNamedExport() {
  return {
    name: 'svg-named-export',
    enforce: 'pre',
    transform(code, id) {
      if (!id.endsWith('.svg')) return null;
      const escaped = JSON.stringify(code);
      return {
        code: `export const svg = ${escaped}; export default svg;`,
        map: null,
      };
    },
  };
}

// ....

// vite.config.js
import { defineConfig } from 'vite';

export default defineConfig({
  plugins: [svgNamedExport()],
});

Which will allow us to do:

import { fooIcon } from 'static/icons/src/foo.svg';

But not sure.

For manifest, likewise I think we might be able to replace it with a glob in the code.

And the sprite sheet should no longer be needed once I update the testing file to run the icons step!

cdrini · 2026-08-13T15:08:41Z

 <ul class="locale-options $classes">
  $for lang in get_supported_languages().values():
-    <li><a href="#" lang="$lang['code']" data-lang-id="$lang['code']" title="$lang['localized']">$lang['native'] ($lang['code'])</a></li> $# detect-missing-i18n-skip-line
+    <li><a href="#" lang="$lang['code']" data-lang-id="$lang['code']" title="$lang['localized']">$lang['native']</a></li> $# detect-missing-i18n-skip-line


Was this intentional?

Intentional, but it is a drive-by product change.

Context: the language dropdown's trigger in site/alert.html was English (en) next to language-icon.svg. Swapping that PNG-era icon for $:macros.icon("globe") was in scope, and while I was there I dropped the (en) from the trigger. The code felt redundant next to the name and the lang attribute plus the title (localized name) are still there for anything that needs them.

Say the word, and I'm happy to put the codes back.

Oh just checking since I thought maybe AI did of its own accord 😁 Up to you ; one thing I will note, is that it would be nice to have a way to also display the language names in the current language. E.g as an English user, I might want to see "Français / French" or something, so I can understand what I'm seeing. The language codes were added to provide a bit of that context, but longer term it might be nice to have both labels somehow!

…n-system

The Lit components each imported a named glyph from icons.generated.js and hand-wrote the <svg> wrapper around it — a third icon path alongside the macro and the element, with the wrapper's attributes duplicated at every call site and free to drift. They now use <ol-icon name="..."> instead. OlIcon renders the glyph inside its own shadow root, so it works where <use> can't reach the sprite, and each host sizes it with plain width/height rules. Stroke weight, where a component wanted a non-default, moves to --ol-icon-stroke-width. Drops the lit-import snippet from the design page's icon gallery: with the glyph module no longer imported by hand, only three usage forms remain.

Committing the sprite, the manifest, and the Lit glyph module bought a checkout that renders icons with no build, and cost a freshness check in both CI and pre-commit, plus a merge conflict on every concurrent icon PR. The build is a dependency-free Node script that runs in well under a second, so paying it is cheaper than policing the outputs. `make icons` now writes only the sprite and the glyph module, both gitignored. `make lit-components` depends on it, so the bundle can't be built from a stale module, and a pretest:js hook covers the JS tests that import it. The freshness check and its CI step and pre-commit hook are gone. The sprite is written to a temp file and renamed, since a rebuild under a running server would otherwise serve a half-written one. The manifest existed only to list icon names for the design page's gallery, which now globs static/icons/src/ — one less generated file, and a list that can't drift from the sources. icon_sprite_url() falls back to the unhashed path when the sprite is missing, so a checkout that never ran `make icons` draws blank icons rather than 500ing every page with one.

Every helper now documents its params and return shape, and parseAttrs and collectIcons get real descriptions — collectIcons in particular explains why it enforces both name and identifier uniqueness (the sprite keys on the id, the Lit module on the export name). Claude-Session: https://claude.ai/code/session_01MKQA4KK4i8E3osyV98359T

lokesh · 2026-08-14T23:40:34Z

@cdrini

Live on testing: https://testing.openlibrary.org/developers/design/icons

Worked through your comments. I reduced the ways of rendering icons down from three, to two, and most importantly was able to simplify the usage in our Lit web components. No more fragment importing and constructing svgs, now simply:

import './OlIcon.js';

html`<ol-icon name="search"></ol-icon>`

Only the a11y attributes differed between the labeled and decorative branches; the conditional now sits inside the tag instead of duplicating the whole element. Claude-Session: https://claude.ai/code/session_01LhQSynuMDhyeBxHv96LyCS

The layout switcher and the component's docs hand-wrote their glyphs because the control re-renders each segment inside its shadow root, where a sprite <use> can't resolve. <ol-icon> inlines its glyph, so it works there — which leaves no inline SVG in the segment call sites. Adds lucide/layout-grid for the grid segment, and sizes .segment ol-icon alongside .segment svg (an outer-tree rule beats <ol-icon>'s :host). Claude-Session: https://claude.ai/code/session_01LhQSynuMDhyeBxHv96LyCS

<ol-books-display> renders a titled set of books for a Solr query from /books-display.json, switchable between a covers carousel (on <ol-carousel>) and a list view, with offset paging and a per-user shelf/rating overlay. It renders into the light DOM so the carousel gets real children and sitewide handlers keep working; styles live in ol-books-display.css. <ol-book-actions> is the per-book popover: reading-log shelves, a star rating, and an add-to-list pane (filter, membership, inline create) that slides in from the right. Optimistic updates with toast rollback; fires ol-book-state-change so cards stay in sync. Lucide glyphs are inlined for now (swap for <ol-icon> once internetarchive#12955 lands). Claude-Session: https://claude.ai/code/session_015u26V9Ppiy5NwqnrhtYShC

Resolve conflicts with the shadow-DOM ol-button (internetarchive#13350): dialog/toast close controls keep <ol-button shape="icon"> but draw <ol-icon name="x">, and the icon macro gains a `slot` arg so PreviewSearchInside can slot into icon-start. Claude-Session: https://claude.ai/code/session_01Vmhke1AR5VjcNKP7KnciCJ

…n-system # Conflicts: # package.json

…n-system # Conflicts: # static/css/components/header-bar.css # static/css/legacy-datatables.css # static/css/tokens/colors.css

for more information, see https://pre-commit.ci

cdrini

Niiiiice this looks fantastic!! 😊 All of this looks great to me ; all my comments are non-blockers. One blocking question which it would be good to answer is do we want to store all the icons directly in the repo? There are some great aggregator sites like https://icon-sets.iconify.design/ with I think npm packages that we might be able to use to fetch them during the build instead of committing them to the repo.

cdrini · 2026-08-21T15:49:47Z


+// Toolbar glyphs from the shared icon set, keyed by the editor's local names.
+// .toolbar-btn ol-icon governs size and stroke width.
+const glyph = (name) => html`<ol-icon name="${name}"></ol-icon>`;


I would just inline this method ; introducing a new noun here glyph I think adds a layer of indirection which isn't particularly useful.

cdrini · 2026-08-21T15:51:34Z

I think switching to a css-based sprite sheet might be worth exploring, since then we'll go down to only one way to use an icon! And we can remove this duplicate mapping of the web component attributes to python templetor parameters -- as well as the jinja wrapper.

cdrini · 2026-08-21T15:53:31Z

+    static _backIcon = html`<ol-icon name="arrow-left"></ol-icon>`;

-    static _personIcon = html`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><path d="M20 21v-2a4 4 0 0 0-4-4H8a4 4 0 0 0-4 4v2"/><circle cx="12" cy="7" r="4"/></svg>`;
+    static _personIcon = html`<ol-icon name="user"></ol-icon>`;


Soooo much nicer!!! 🥳 I would also likely inline these ; not sure we're getting much out of having them stored as static constants. Before it made sense since it was a decent junk of SVG code that would make the place it was used difficult to read, but now it's a hyper readable short string!

cdrini · 2026-08-21T15:54:49Z

This could in theory potentially be replaced by a css-based sprite sheet.

cdrini · 2026-08-21T15:57:17Z

 <ul class="locale-options $classes">
  $for lang in get_supported_languages().values():
-    <li><a href="#" lang="$lang['code']" data-lang-id="$lang['code']" title="$lang['localized']">$lang['native'] ($lang['code'])</a></li> $# detect-missing-i18n-skip-line
+    <li><a href="#" lang="$lang['code']" data-lang-id="$lang['code']" title="$lang['localized']">$lang['native']</a></li> $# detect-missing-i18n-skip-line


Oh just checking since I thought maybe AI did of its own accord 😁 Up to you ; one thing I will note, is that it would be nice to have a way to also display the language names in the current language. E.g as an English user, I might want to see "Français / French" or something, so I can understand what I'm seeing. The language codes were added to provide a bit of that context, but longer term it might be nice to have both labels somehow!

Co-authored-by: Drini Cami <[email protected]>

book-icons.js inlined Lucide paths as a stopgap until the shared icon system landed (internetarchive#12955). It has, so the books-display components now render <ol-icon> like every other Lit component. The eight glyphs the sprite lacked are added as Lucide sources, plus two non-Lucide ones: covers-row (custom) and star-filled, since a symbol's fill="none" outranks inherited CSS so the rating stars need a filled variant. Per-use stroke widths move to --ol-icon-stroke-width. Claude-Session: https://claude.ai/code/session_016bwMwkcQzpKTpEqrtCoeaw

21 of these were replaced by sprite icons in internetarchive#12955; the other 29 have had no references in the codebase for years and turned up while auditing. Verified none of the 50 is referenced from any template, JS, CSS or Python on the icon-system branch. Split out of internetarchive#12955 so that PR carries no binary content: GitHub's .diff omits binary data, so a patch deploy of a PR containing binary deletions aborts unless it passes --exclude=static/images/*. This one never needs to reach testing, so the flag stops mattering. Merge after internetarchive#12955. Claude-Session: https://claude.ai/code/session_01Lv9GuFfV3JRG7hmKGmN1t4

* Delete 50 legacy image files replaced by the icon system 21 of these were replaced by sprite icons in #12955; the other 29 have had no references in the codebase for years and turned up while auditing. Verified none of the 50 is referenced from any template, JS, CSS or Python on the icon-system branch. Split out of #12955 so that PR carries no binary content: GitHub's .diff omits binary data, so a patch deploy of a PR containing binary deletions aborts unless it passes --exclude=static/images/*. This one never needs to reach testing, so the flag stops mattering. Merge after #12955. Claude-Session: https://claude.ai/code/session_01Lv9GuFfV3JRG7hmKGmN1t4 * Replace the list delete trashcan link with an ol-button The delete/remove control on list pages was an anchor with a sprite-image trashcan; it now uses ol-button with the trash icon, which lets the last of the legacy image files go. On a list's own page the button moves into the CTA column under the lending button and dropper, so the dropper's ad-hoc margin and its 50px reservation for the absolutely positioned control give way to a single flex gap owning the rhythm. Claude-Session: https://claude.ai/code/session_01NmxEn3FtZunApjLZAW78wo * Move the unreferenced phase-2 icon set out to its own PR The 17 lucide additions had ridden along in the ol-button commit with nothing consuming them; they now land via #13424 instead. Claude-Session: https://claude.ai/code/session_01RtTEd3rAHJYz3zKi3zUWwD * Point the service-worker fixture at a surviving image The matcher test used the now-deleted barcode_scanner.svg as its sample path. Claude-Session: https://claude.ai/code/session_01RtTEd3rAHJYz3zKi3zUWwD * Scope the empty check-in-container rule to list pages The grid variant applied to every .list-books--grid (search results, author pages, reading log), not just list seeds. Claude-Session: https://claude.ai/code/session_01RtTEd3rAHJYz3zKi3zUWwD * Move the ol-button trashcan swap to its own PR Restores the list templates, JS, and CSS to master and keeps back_list-trash.png, whose last reference lives until #13425 lands. This PR is back to being a pure deletion of unreferenced files. Claude-Session: https://claude.ai/code/session_01RtTEd3rAHJYz3zKi3zUWwD * Keep read aloud.svg: buttonCta.css still references it URL-encoded The audit grep missed the %20-encoded url() in buttonCta.css:39, so deleting the file would blank the open-access audiobook button icon. It goes once that button is on the sprite (icons/chore/phase2-icon-set). Claude-Session: https://claude.ai/code/session_01RtTEd3rAHJYz3zKi3zUWwD

lokesh added 7 commits June 17, 2026 10:32

Style icon gallery as a bordered grid

23fc3c1

cdrini mentioned this pull request Jun 19, 2026

Browse menu: redesign as a popover with featured destinations #12917

Merged

lokesh added 2 commits June 24, 2026 23:55

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

2a2d668

…n-system

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

50a9de8

…n-system # Conflicts: # static/css/page-design.css

accesslint Bot reviewed Jul 18, 2026

View reviewed changes

accesslint Bot reviewed Aug 9, 2026

View reviewed changes

lokesh added 3 commits August 8, 2026 22:38

lokesh changed the title ~~Add unified icon system: <ol-icon>, $:icon() macro, build-time sprite~~ Add unified icon system: <ol-icon>, $:macros.icon() macro, build-time sprite Aug 9, 2026

lokesh added 8 commits August 9, 2026 11:44

lokesh mentioned this pull request Aug 11, 2026

Epic: UI Modernization — consistency, mobile, accessibility #13240

Open

50 tasks

lokesh added 2 commits August 10, 2026 22:23

lokesh changed the title ~~Add unified icon system: <ol-icon>, $:macros.icon() macro, build-time sprite~~ Add unified icon system Aug 11, 2026

lokesh assigned cdrini Aug 11, 2026

lokesh added 2 commits August 12, 2026 21:43

lokesh mentioned this pull request Aug 13, 2026

Delete 68 legacy image files no longer referenced anywhere #13329

Merged

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

8df74ff

…n-system

lokesh added the On Testing label Aug 13, 2026

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

d1e77bc

…n-system Claude-Session: https://claude.ai/code/session_011N2wcn6DdjDWcGUoHz7RC9

cdrini reviewed Aug 14, 2026

View reviewed changes

lokesh added 4 commits August 14, 2026 15:05

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

6fdfb93

…n-system

lokesh added 2 commits August 14, 2026 19:19

lokesh mentioned this pull request Aug 17, 2026

Subject-page carousel on ol-books-display: covers/list toggle + per-book actions popover #13346

Closed

lokesh requested a review from cdrini August 19, 2026 17:33

lokesh and others added 4 commits August 19, 2026 10:36

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

12e1cf2

…n-system # Conflicts: # package.json

Merge remote-tracking branch 'upstream/master' into icons/feat/ol-ico…

05f8086

…n-system # Conflicts: # static/css/components/header-bar.css # static/css/legacy-datatables.css # static/css/tokens/colors.css

Merge branch 'master' into icons/feat/ol-icon-system

ee3a465

[pre-commit.ci] auto fixes from pre-commit.com hooks

8100a14

for more information, see https://pre-commit.ci

cdrini approved these changes Aug 21, 2026

View reviewed changes

Update openlibrary/components/lit/OlIcon.js

97c471e

Co-authored-by: Drini Cami <[email protected]>

lokesh merged commit 026d3d4 into internetarchive:master Aug 21, 2026
4 of 5 checks passed

openlibrary-bot mentioned this pull request Aug 23, 2026

Continue Reading Phase 1: Merge 'Active Loans' with 'Loan History' on 'My Books' page #13281

Merged

lokesh mentioned this pull request Aug 25, 2026

Add 10 lucide icons and replace their legacy image consumers #13424

Closed

Uh oh!

Uh oh!

Conversation

lokesh commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What patrons get

What developers get

How it works

Two ways to draw an icon

Why two and not one

Also rejected

Scope

How to review this

Testing

Stakeholders

Uh oh!

accesslint Bot left a comment

Choose a reason for hiding this comment

Uh oh!

accesslint Bot Jul 18, 2026

Choose a reason for hiding this comment

Uh oh!

accesslint Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cdrini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lokesh commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cdrini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lokesh commented Jun 17, 2026 •

edited

Loading

lokesh commented Aug 14, 2026 •

edited

Loading