Skip to content

feat(theming): add theme mode system (Classic, Minimal, E-Ink) - #2423

Open
MarvNC wants to merge 22 commits into
yomidevs:masterfrom
MarvNC:theme-revamp
Open

feat(theming): add theme mode system (Classic, Minimal, E-Ink)#2423
MarvNC wants to merge 22 commits into
yomidevs:masterfrom
MarvNC:theme-revamp

Conversation

@MarvNC

@MarvNC MarvNC commented May 24, 2026

Copy link
Copy Markdown
Member

Summary

This PR introduces a theme mode system for Yomitan, allowing users to choose between three visual styles for popups and the search page:

  • Classic — The existing Yomitan appearance (default for existing users)
  • Minimal — A cleaner, more subdued aesthetic inspired by Hoshi Reader
  • E-Ink — High-contrast black/white with no animations, optimized for e-ink displays

Screenshots

Minimal Classic E-Ink
chrome_遠征_-_Yomitan_Search_-_Google_Chrome_2026-05-24_19-09-01 chrome_遠征_-_Yomitan_Search_-_Google_Chrome_2026-05-24_19-09-05 chrome_遠征_-_Yomitan_Search_-_Google_Chrome_2026-05-24_19-09-10

Architecture

Additive CSS with data-theme-mode

Themes are implemented as additive CSS overrides on top of the base Classic theme:

  • Base styles live in display.css, material.css, etc. (Classic is the default)
  • Each theme has its own CSS file (theme-minimal.css, theme-eink.css)
  • Theme CSS uses :root[data-theme-mode='...'] selectors — only the active theme's rules match
  • All theme files are loaded statically via <link> tags; the data-theme-mode attribute on <html> controls activation

Dual-context loading

Each theme CSS file contains selectors for both rendering contexts:

  • Inner (:root[data-theme-mode='...']) — styles content inside the popup iframe, loaded statically
  • Outer (iframe.yomitan-popup[data-theme-mode='...']) — styles the iframe element itself (borders, shadows, radius), injected dynamically by popup.js into the parent page's shadow DOM

This means one file per theme, loaded in both contexts. Selectors that don't match are harmless no-ops.

Registry-driven

ext/js/data/theme-registry.js is the single source of truth:

export const themes = [
    { id: 'classic', label: 'Classic', css: null },
    { id: 'minimal', label: 'Minimal', css: '/css/theme-minimal.css' },
    { id: 'eink', label: 'E-Ink', css: '/css/theme-eink.css' },
];

The registry drives:

  • Dropdown <option> generation in Settings and Welcome pages
  • Outer chrome CSS injection in popup.js

Adding a new theme requires only 4 steps:

  1. Create theme-new.css (inner + outer selectors)
  2. Add <link> to popup.html, search.html, popup-preview.html
  3. Add entry to theme-registry.js
  4. Add to schema enum + TypeScript union type

Theme Details

Minimal

  • Clean white/black backgrounds with subtle gray accents
  • Dictionary name tags converted to text labels
  • Inflection chains shown as compact gray pills (no icons)
  • Frequency tags use translucent blue accents
  • Reduced spacing and visual noise
  • No popup shadow, subtle 8em border radius

E-Ink

  • Pure black/white (no grays)
  • All transitions and animations stripped
  • Tags shown as outlined boxes with no rounding
  • No shadows anywhere
  • 1px solid borders
  • Progress bar animations also disabled

Settings

New "Mode" selector in Appearance settings (and Welcome page):

  • Classic / Minimal / E-Ink
  • When E-Ink is selected, the Shadow dropdown is disabled (e-ink forces no shadow)

Migration

Existing users are automatically migrated to Classic via options v77 to preserve their current appearance. New installations default to Minimal.

Documentation

Added docs/theming.md — a comprehensive guide for creating custom themes.

Testing

  • npm run test:fast — 4,136 tests passed
  • npm run test:css — stylelint clean
  • ✅ Manual visual verification of all three themes in popup preview
  • ✅ Verified theme switching works correctly
  • ✅ Verified dropdowns populate correctly from registry

Files Changed

Core:

  • ext/js/data/theme-registry.js — Theme definitions
  • ext/js/app/theme-controller.jsdata-theme-mode attribute management
  • ext/js/app/popup.js — Outer chrome injection
  • ext/js/display/display.js — Inner content theme application

CSS:

  • ext/css/theme-minimal.css — Minimal theme overrides
  • ext/css/theme-eink.css — E-ink theme overrides
  • ext/css/display.css — CSS variable additions for theming

Settings:

  • ext/settings.html — Theme mode selector
  • ext/welcome.html — Welcome page theme selector
  • ext/js/pages/settings/settings-main.js — Dropdown generation, e-ink handling
  • ext/js/pages/welcome-main.js — Welcome page dropdown generation

Schema/Types:

  • ext/data/schemas/options-schema.jsonpopupThemeMode field
  • types/ext/settings.d.tsPopupThemeMode type
  • ext/js/data/options-util.js — v77 migration (default to classic)

Docs:

  • docs/theming.md — Theme creation guide
  • README.md — Link to theming docs

MarvNC added 20 commits May 23, 2026 20:51
…nk tags

Remove dynamic theme stylesheet loading from display.js and popup.js.
Theme CSS files are now loaded statically via <link> tags in popup.html,
search.html, and popup-preview.html. The active theme is controlled by
the data-theme-mode attribute on <html>, set by theme-controller.js.

This simplifies adding new themes to three steps:
1. Create CSS file with :root[data-theme-mode='id'] selectors
2. Add <link> tag to HTML pages
3. Register in theme-registry.js

No JavaScript changes required per theme.
Add comprehensive documentation for creating Yomitan themes,
covering the additive CSS architecture, 5-step creation process,
CSS patterns, best practices, and testing. Link from README
under Developer Documentation → Advanced Features.
…text CSS loading

Fix review findings: restore theme-registry.js with css property, add
outer chrome injection to popup.js, generate dropdowns dynamically from
registry in settings and welcome pages. Theme CSS files now contain both
inner (:root) and outer (iframe) selectors — loaded statically via <link>
for inner content, injected dynamically for outer chrome.

- Add css property back to theme-registry.js
- Add _injectThemeStylesheet() to popup.js for outer DOM injection
- Generate theme dropdowns from registry in settings-main.js + welcome-main.js
- Move iframe rules back into theme-minimal.css and theme-eink.css
- Fix .menu → .popup-menu in e-ink border-radius block
- Add .progress-bar-indeterminant to e-ink animation reset
- Update docs/theming.md with correct dual-context architecture
…r injection

Fix critical issues found in code review:

- Populate theme dropdown options BEFORE GenericSettingController init,
  so saved values are applied correctly (was populating after, causing
  empty select to default to first option)

- Replace loadStyle with manual DOM injection in popup.js outer chrome.
  loadStyle's WebExtension API tracking throws on repeated injection
  with the same ID, crashing theme switching in non-shadow mode.

- Update theme-minimal.css tag variable names to match display.css refactor:
  --tag-muted-background-color and --tag-accent-background-color
  (was overriding dead --tag-default-background-color etc.)

- Remove unnecessary async from Display._setTheme (no await inside)

- Update docs/theming.md schema default to minimal (matching actual schema)
@MarvNC

MarvNC commented May 25, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fa6e60622

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ext/js/app/popup.js Outdated
Comment thread ext/css/popup-outer.css
- Inject outer theme CSS after shadow root exists (P1)
  _injectStyles() now calls _injectThemeStylesheet() after _setUpContainer
  creates the shadow root, ensuring Minimal/E-Ink outer chrome styles apply.

- Make dark outer-theme shadow variable-driven (P2)
  Changed hard-coded white glow to var(--popup-box-shadow, ...) so themes
  can override it while preserving the Classic default fallback.
@MarvNC
MarvNC marked this pull request as ready for review May 25, 2026 01:35
@MarvNC
MarvNC requested a review from a team as a code owner May 25, 2026 01:35
@MarvNC

MarvNC commented May 25, 2026

Copy link
Copy Markdown
Member Author

@codex review all commits

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fa6e60622

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ext/js/app/popup.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a theme mode system layered on top of the existing theme settings, letting users choose between three CSS-driven presentations of popup and search content (Classic, Minimal, E-Ink). The feature is implemented as additive CSS files keyed on a new data-theme-mode attribute set by ThemeController, with a registry that drives both the settings/welcome dropdowns and dynamic outer-chrome stylesheet injection in popup.js. A new general.popupThemeMode option is added to the schema, types, and migrated via options v77 (existing users → classic, new installs → minimal).

Changes:

  • Add popupThemeMode option (schema + types + v77 migration) and propagate it through ThemeController, Display._setTheme, and Popup._setOptionsContext (including dynamic outer theme stylesheet injection).
  • Add theme registry (ext/js/data/theme-registry.js), two new CSS theme files (theme-minimal.css, theme-eink.css), corresponding <link> tags in popup.html / search.html / popup-preview.html, and CSS variable additions to display.css / popup-outer.css.
  • Add Mode dropdown (registry-populated) in Settings and Welcome pages, with logic that forces Shadow → none and disables the Shadow select when E-Ink is selected; add docs/theming.md and README link.

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
types/ext/settings.d.ts Adds PopupThemeMode union and popupThemeMode field on GeneralOptions.
ext/data/schemas/options-schema.json Adds popupThemeMode enum/required field (default minimal).
ext/js/data/options-util.js Adds v77 migration setting existing profiles to popupThemeMode='classic'.
test/options-util.test.js Updates test fixtures to expect v77 and popupThemeMode: 'classic'.
ext/js/data/theme-registry.js New registry of themes plus getThemeById / populateThemeModeSelect.
ext/js/app/theme-controller.js Adds themeMode getter/setter and writes data-theme-mode to target element.
ext/js/app/popup.js Reads popupThemeMode and dynamically injects outer theme CSS into popup chrome.
ext/js/display/display.js Passes popupThemeMode to ThemeController for inner content theming.
ext/js/pages/settings/settings-main.js Populates theme-mode dropdown and manages E-Ink → Shadow disable logic.
ext/js/pages/welcome-main.js Populates theme-mode dropdown on the welcome page.
ext/settings.html / ext/welcome.html Adds Mode dropdown UI and id hooks.
ext/popup.html / ext/search.html / ext/popup-preview.html Statically links the new theme CSS files.
ext/css/theme-minimal.css / theme-eink.css New theme override stylesheets (inner + outer selectors).
ext/css/display.css Adds --tag-muted-background-color / --tag-accent-background-color variables and uses them.
ext/css/popup-outer.css Switches hard-coded popup chrome values to CSS variables.
docs/theming.md / README.md New theming guide and README link.
package.json Adds --ignore-pattern '**/*.jsonc' to test:js.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ext/js/app/popup.js Outdated
Comment thread ext/js/app/popup.js Outdated
Comment thread ext/js/display/display.js Outdated
Comment thread ext/js/pages/settings/settings-main.js Outdated
Comment thread ext/css/theme-eink.css
Comment thread ext/js/pages/settings/settings-main.js
MarvNC added a commit to MarvNC/yomitan that referenced this pull request May 25, 2026
- Defer theme stylesheet injection until popup container is ready
  (only inject in _setOptionsContext when _injectPromiseComplete)
- Enforce E-Ink outerTheme='none' at runtime to prevent shadows
- Remove redundant manual stylesheet removal; let loadStyle manage cache
- Remove unnecessary void operator before synchronous _setTheme
- Prevent settings UI from overwriting saved popupOuterTheme on load
  (add dispatchChange parameter to updateThemeModeUI)

Refs: yomidevs#2423
- Defer theme stylesheet injection until popup container is ready
  (only inject in _setOptionsContext when _injectPromiseComplete)
- Enforce E-Ink outerTheme='none' at runtime to prevent shadows
- Remove redundant manual stylesheet removal; let loadStyle manage cache
- Remove unnecessary void operator before synchronous _setTheme
- Prevent settings UI from overwriting saved popupOuterTheme on load
  (add dispatchChange parameter to updateThemeModeUI)

Refs: yomidevs#2423
@Kuuuube Kuuuube added kind/enhancement The issue or PR is a new feature or request area/ui-ux The issue or PR is related to UI/UX/Design labels May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ui-ux The issue or PR is related to UI/UX/Design kind/enhancement The issue or PR is a new feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants