refactor: extract banners.js from content.js#93
Merged
Conversation
content.js was 1,280 lines after the 3.5.6 hotfix. Banner UI is one of
the cleanest extraction boundaries — pure DOM, only depends on
window._sb.t and window._sb.escapeHtml plus existing label constants
already declared as ESLint globals.
Moves out of content.js (~120 lines):
- showOfflineBanner / hideOfflineBanner
- skillbridge:bridgeunavailable listener (persistent banner)
- skillbridge:storagequota listener (auto-dismiss banner)
- showExamBanner
- showTranslationProgress / updateTranslationProgress / hideTranslationProgress
Stays in content.js because of state coupling:
- online/offline event handlers (intertwined with translation pipeline)
- showTermPreview / _renderTermPreview (uses translator,
FLASHCARD_COURSE_SLUGS_SORTED, _termPreviewShown closure)
Functions attach back onto window._sb so call sites only change shape,
never semantics. Optional chaining (?.) on each call site protects
against banners.js failing to load — a missing banner is a soft
degradation, not a content.js crash.
manifest.json content_scripts list updated to load banners.js
immediately after content.js. Bundle and Firefox build paths read the
manifest list directly, so no other config changes needed.
Net change: -114 lines in content.js (1318 → 1204), +161 lines in new
banners.js, +1 manifest entry. Bundle output grows ~2 KB pre-minify
(unchanged after minify, 114.4 KB total — same as before).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three findings from /simplify on the just-pushed extraction:
1. CRITICAL — listener-leak guard.
Without an idempotency marker the IIFE re-runs on extension
auto-update / dev reload and adds duplicate listeners for
skillbridge:bridgeunavailable and skillbridge:storagequota each
time, so after N reloads N banners would render per fire. Mirrors
the history.pushState __sb_wrapped__ guard from the v3.5.6 hotfix.
Added an `sb.__bannersLoaded` flag.
2. Unified the five small banners (offline, bridge-unavailable,
storage-quota, exam, plus offline's hide cousin) onto a single
showSimpleBanner({...}) helper. Each call site is now 6-8 lines
describing only the banner's identity rather than re-doing the
create/setAttribute/append/raf-class dance. Translation progress
stays separate — its two coordinated elements with dynamic content
don't fit the helper shape.
3. Trimmed the breadcrumb comment from content.js that pointed at
banners.js — the optional-chaining call sites already document the
dependency, and the docblock list inside banners.js was a
maintenance hazard (could go stale if banners are added).
Reuse review came back clean (textContent-only, namespace pattern
matches header-controls/text-selection). Tests 309/309 pass; lint,
prettier, bundle build all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
content.js was 1,280 lines after the v3.5.6 hotfix. Banner UI is the cleanest extraction boundary — pure DOM, only depends on `window._sb.t` and the label constants that are already ESLint globals.
This is the first of three refactor candidates flagged by the production audit; the other two (`flashcards.js`, `chat-history.js` from `sidebar-chat.js`) are larger and stay for a separate PR.
What moved
Out of content.js into the new src/content/banners.js (~120 lines):
What deliberately stayed in content.js
How call sites change
Bare function calls become `window._sb.X?.()` — optional chaining on each site so a missing banners.js degrades softly (no banner UI) rather than crashing content.js. Same shape as the other extracted modules (`header-controls.js`, `text-selection.js`).
manifest.json gets one new entry between `content.js` and `code-comments.js`. Both build pipelines (bundle, Firefox) read the manifest list directly so no other config changed.
Net change
```
content.js 1,280 → 1,166 (-114)
banners.js — → 161 (+161)
manifest.json +1 array entry
```
Bundle output: 114.4 KB minified — same as before extraction (esbuild dedupe handles the ~2 KB of pre-minify overhead from the new IIFE).
Verification (local)
Test plan
🤖 Generated with Claude Code