Skip to content

WEBDEV-9038: Migrate the topnav into elements - #95

Open
jbuckner wants to merge 8 commits into
mainfrom
WEBDEV-9038-migrate-topnav
Open

jbuckner wants to merge 8 commits into
mainfrom
WEBDEV-9038-migrate-topnav

Conversation

@jbuckner

@jbuckner jbuckner commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Ports ia-topnav out of the iaux monorepo. Every sub-element is namespaced ia-topnav-* so it can't collide with the copies petabox and offshoot still load, and element-names.test.ts holds that line: it checks the new names register and that all 17 generic names the old package claims stay free. Both packages load during the migration and a duplicate define throws rather than degrading.

Ported from the WEBDEV-9036 branch rather than waiting on it, so the uploader and biblio admin sections are here from the start. That work has since merged as ia-topnav 2.2.0, and this branch matches it (the only difference between the port source and what landed is the version bump itself).

No dependencies added and two dropped. ia-wayback-search only supplied a base class, and importing it registered the generic ia-wayback-search tag as a side effect, so it's one flattened element now. ia-styles only supplied the sr-only block, now a shared style in themes/.

Per-component style modules became inline styles like everything else here. The user menu and signed-out dropdown returned exactly the base class's styles, which Lit already inherits, so those overrides are gone.

Dropped as dead: the ia-icon element (nothing imported or rendered it, and it squatted a generic name) and a write-only state field on the login button.

Several bugs fell out of the move, all pre-existing.

The test suite was in worse shape than it looked. Two tests had every assertion commented out. Four more asserted to.not.be.undefined on a querySelector result, which returns null, so they passed whether or not they found anything. Tightening those broke two tests that had never actually run: the wayback one looked for the search directly inside the web subnav when it sits a level deeper inside the slider, and the login button one expected the button to open itself when the parent owns that state. All are rewritten against real behavior, taking the component from 52 tests to 95.

dropdown-menu.test.ts also registered the base class under a name element-names.test.ts reserves. They only coexisted because vitest isolates each file in its own page, so isolate: false would have failed one of them by load order.

The desktop wayback search icon was painting black. The topnav's :host declares every other color knob but had dropped --desktopSearchIconFill, so the nested search read an undefined variable and the fill was invalid at computed-value time. The README still documents it as var(--grey20), which is what it goes back to. That is a deliberate visual change, and the only one in this PR.

Offshoot and petabox swap over in follow-ups. Petabox is on a v1 of the package while offshoot is on v2, so this follows v2 and nothing here changes what petabox serves.

Two things I left alone rather than pile onto this diff, both worth their own ticket. The 41 theme variables are hardcoded on the host with generic names like --activeColor and --grey13, so consumers can't override them; converting those to the public-plus-private-alias shape deserves a reviewable diff of its own. And this repo ships icons as SVG files behind a CSS mask, while topnav carries 19 inline icon modules.

QA

Everything runs on the PR preview, no login needed.

Preview: https://internetarchive.github.io/elements/pr/pr-95/#elem-ia-topnav

🤖 Generated with Claude Code

https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY

Ports ia-topnav out of the iaux monorepo as ia-topnav, with every
sub-element namespaced ia-topnav-* so it can't collide with the copies
petabox and offshoot still load. element-names.test.ts holds that line:
it checks the new names register and that all 17 generic names the old
package claims stay free, since both packages load during the migration
and a duplicate define throws rather than degrading.

Ported from the WEBDEV-9036 branch rather than iaux master, so the
uploader and biblio admin sections are here from the start. That PR
touches most of what this move carries, so porting master would have
meant doing the same work twice.

No dependencies added, two dropped. ia-wayback-search only supplied a
base class, and importing it registered the generic ia-wayback-search
tag as a side effect, so it's one flattened element now. ia-styles only
supplied the sr-only block, which is now a shared style in themes.

Per-component style modules became inline styles, which is what every
other element here does. The shared subnav fragment keeps a plain name
at the component root. The user menu and signed-out dropdown returned
exactly the base class's styles, and Lit already inherits those, so the
overrides are gone.

Three things dropped as dead: the ia-icon element, which nothing
imported or rendered and which squatted a generic name; a write-only
state field on the login button; and the redundant style overrides
above.

Two bugs came out of the move. Two tests had every assertion commented
out, so they passed while checking nothing; both are rewritten against
real behavior, taking the component from 52 tests to 94. And the
wayback search's desktop icon read a variable nothing sets, with no
fallback, so its fill resolved to invalid and painted black against the
dark nav. It follows the topnav's own icon color now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
PR Preview Action v1.8.1

🚀 View preview at
https://internetarchive.github.io/elements/pr/pr-95/

Built to branch ghpages at 2026-09-24 18:33 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov-commenter

codecov-commenter commented Sep 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.84663% with 123 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.56%. Comparing base (76db2d8) to head (30b0dac).

Files with missing lines Patch % Lines
src/elements/ia-topnav/lib/keyboard-navigation.ts 39.62% 30 Missing and 2 partials ⚠️
src/elements/ia-topnav/ia-topnav-primary-nav.ts 63.38% 24 Missing and 2 partials ⚠️
src/elements/ia-topnav/ia-topnav.ts 82.79% 10 Missing and 6 partials ⚠️
src/elements/ia-topnav/ia-topnav-media-menu.ts 61.53% 9 Missing and 1 partial ⚠️
src/elements/ia-topnav/dropdown-menu.ts 80.48% 2 Missing and 6 partials ⚠️
src/elements/ia-topnav/ia-topnav-wayback-search.ts 42.85% 8 Missing ⚠️
src/elements/ia-topnav/ia-topnav-media-slider.ts 82.14% 1 Missing and 4 partials ⚠️
src/elements/ia-topnav/lib/make-boolean-string.ts 16.66% 5 Missing ⚠️
src/elements/ia-topnav/ia-topnav-media-subnav.ts 84.00% 1 Missing and 3 partials ⚠️
src/elements/ia-topnav/data/menus.ts 83.33% 1 Missing and 1 partial ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #95      +/-   ##
==========================================
- Coverage   91.22%   88.56%   -2.67%     
==========================================
  Files          60       87      +27     
  Lines        2518     3007     +489     
  Branches      582      709     +127     
==========================================
+ Hits         2297     2663     +366     
- Misses         77      171      +94     
- Partials      144      173      +29     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jbuckner and others added 7 commits September 10, 2026 11:30
Four assertions used `to.not.be.undefined` on a querySelector result.
querySelector returns null, so they passed whether or not the element
was found. Switching them to `to.exist` failed two tests that had never
actually run:

The wayback test looked for the search inside the web subnav, but the
subnav renders the slider and the search sits one level inside that.
The selector never matched.

The login button test clicked the toggle and expected an active class.
The button owns no open state; it reports the click and the parent sets
openMenu. Standalone it can never go active. It now checks both halves
of that contract, the event on click and the class once openMenu is set.

The dropdown-menu test also registered the base class as `dropdown-menu`
at module scope, a name element-names.test.ts asserts stays free. Both
only passed because vitest isolates each file in its own page, so
turning isolation off would have failed one of them depending on load
order. The base class is registered nowhere in the source, so the test
now mounts it under a tag inside the topnav's own namespace.

Also drops a comment that explained the icon fill by contrast with its
old package, and moves the wayback search to the getter form of styles
that every other element here uses.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY
The topnav's :host declares every other color knob, but this one had
gone missing, so the nested wayback search read an undefined variable
and the desktop glyph painted black. The package README still documents
it as var(--grey20), which is what it goes back to. The search keeps a
fallback to the topnav's general icon color for the case where it is
mounted on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY
The fallback only fires when the search is mounted outside a topnav,
and in that case none of the topnav's greys are declared either, so a
var() fallback is as unresolved as the value it was covering for. fill
inherits, so that paints black rather than falling through. A literal
is the only fallback that actually does anything there.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpyQxVSkZFrWxxGamCcNhY
…opnav

* origin/main:
  v0.3.0 (#96)
  WEBDEV-8969: Migrate radio-player into elements (#86)
  WEBDEV-8974: Migrate the radio player search handler into elements (#85)
  WEBDEV-8968: Migrate transcript-view into elements (#84)
  WEBDEV-8967: Migrate expandable-search-bar into elements (#83)
  WEBDEV-8966: Migrate scrubber-bar into elements (#82)
  WEBDEV-8965: Migrate playback-controls into elements (#80)
  WEBDEV-8964: Migrate waveform-progress into elements (#78)
  WEBDEV-8963: Migrate audio-element into elements (#77)

# Conflicts:
#	src/elements/index.ts
Vitest writes these under .vitest-attachments when a test attaches an
image, so they are run output, not source. main already ignores the
directory, but .gitignore only applies to untracked files and these had
already been committed here, so the rule never caught them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaEr5YT1xpjK5ydNZPvjwx
…opnav

* origin/main:
  1.1.1 (#118)
  WEBDEV-9132: Fix Safari image-viewer wide-layout breakpoint (#115)
  1.1.0 (#116)
  WEBDEV-9129: Exclude story files from coverage reporting (#113)
  WEBDEV-9121: Migrate histogram-date-range into elements (#112)
  WEBDEV-9130: Exclude nested dist and worktree tests from local runs (#114)
  v1.0.0 (#111)
  WEBDEV-9025: Add a CLAUDE.md covering the publish flow (#94)
  WEBDEV-9063: Typecheck in CI (#107)
  v0.4.1 (#108)
  WEBDEV-9066: Guard custom element registration against duplicate defines (#106)
  v0.4.0 (#99)
  WEBDEV-9022: Migrate the image viewer into elements (#93)

This branch has not been deployed

No deployments
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.

2 participants