Dark mode - #49
Merged
Merged
Dark mode#49
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Palette inconsistencies, ineffective datepicker overrides, and insufficient status-tag contrast need correction.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
Adds configurable light/dark theming to the ActiveAdmin stylesheet.
Changes:
- Introduces semantic CSS variables and automatic/explicit dark-mode selection.
- Reworks component colors, controls, tables, menus, and status tags.
- Adds extensive sizing and layout customization variables.
| File | Description |
|---|---|
app/assets/stylesheets/wigu/active_admin_theme.scss |
Implements the dark-mode palette and component styling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1364
to
+1371
| .ui-datepicker.ui-widget { | ||
| background: var(--aa-surface); | ||
| border-color: var(--aa-border); | ||
| color: var(--aa-text); | ||
| .ui-datepicker-header { background: var(--aa-surface-2); border-color: var(--aa-border); } | ||
| a, td span, .ui-datepicker-title { color: var(--aa-text); } | ||
| .ui-state-hover, .ui-state-active { background: var(--aa-surface-hover); } | ||
| } |
This was referenced Oct 1, 2026
master has moved three commits ahead: the gem file whitelist (activeadmin-plugins#48), the compile check and variable type guards (activeadmin-plugins#50), and the header menu work with its review fixes (activeadmin-plugins#51). activeadmin-plugins#51 matters here. Its first two commits are this branch's first two, cherry-picked, with thirteen defects fixed on top — and it was squash-merged, so git cannot tell they are already upstream. A rebase would have replayed them and quietly reverted every one of those fixes; merging keeps them. The stylesheet conflicted in eight places. Resolution: * Variable header: both sides kept. This branch's palette and the per-mode *Dark values stay; the eight menu variables the two sides share take master's definitions, because master's are the corrected ones — $skinMenuPillTextColor and $skinMenuItemHoverTextColor now default to $skinMenuTextColor instead of #ffffff independently, $skinMenuItemPaddingY is 8px, $skinHeaderPaddingY is split back into Top/Bottom so the header does not shift, and $skinMenuPanelMaxWidth comes across. The type guards from activeadmin-plugins#50 follow the variables. * Menu rules: master's throughout — the width ceiling, the squared pill and panel corners, the 5px bridge, the currentColor marker, :focus. * #utility_nav and ul.tabs > li font-size: this branch's, master has nothing there. * The dead #title_bar batch-actions block stays deleted. Verified after the merge: the stylesheet compiles, and every fix from activeadmin-plugins#51 is still present in it.
`$skinBorderRadius - 1px` is evaluated by Sass at compile time, so a project that sets the radius in any unit other than px does not get a wrong corner — it gets `Incompatible units: 'px' and 'rem'` and no stylesheet at all. Six table-corner rules did this, and `rake css` now covers that override, which is why CI went red on this branch. Wrap it in a small function instead: calc() defers the subtraction to the browser, so the unit no longer has to match, and a unitless 0 is returned as 0 because calc(0 - 1px) is not valid CSS either. Output is unchanged for the default: calc(4px - 1px) renders as 3px.
Brings in activeadmin-plugins#53: the flat button rule now covers form input[type=button] and form button, not only the submit input. No conflict — this branch does not touch that selector.
The merge with master restored `ul.tabs > li { font-size: $skinMenuFontSize }`
alongside master's `ul.tabs > li > a` rule, so the file carried both — and the
comment on the anchor rule, three lines below, says in so many words that it
sits on the anchor precisely so the dropdown subtree does not inherit it.
font-size is inherited and `li.has_nested` contains the submenu `ul`, so the
li rule put the size back on the whole subtree: a project enlarging the
top-level tabs got its dropdown items enlarged too, with no way to separate
them. Dropping the li rule leaves the anchor rule doing what it documents.
The variable was introduced on this branch and then orphaned by the merge: master's menu rules set no text colour on the hovered or current dropdown item at all — activeadmin-plugins#51 deleted the hard-coded white rather than varying it — so the variable existed, documented a behaviour, and changed nothing. Apply it where its own comment promises. It defaults to $skinMenuTextColor, so a project that light-themes the menu still sets one variable and the rendered output is unchanged; a project that wants the hovered item to read differently now has the knob the name implies.
ActiveAdmin paints a ticked row with `table.index_table tr.selected td`, which scores (0,2,2). The zebra and hover rules added here are `#wrapper #active_admin_content table.index_table tbody tr… > td` at (2,2,4), so they win and a ticked row renders exactly like an unticked one. Tick three rows out of fifty and nothing on screen says so — in either mode. Add the missing rule at the same depth as the ones that displaced it, with its own palette entry so dark mode gets a selection colour rather than the light blue. The light default is ActiveAdmin's own $table-selected-color. The `:hover` variant is there so the selection stays visible while the pointer is over the row, instead of being repainted by the hover fill.
`div.batch_actions_selector { a { … } }` is a descendant selector, and
ActiveAdmin nests the open menu in the same element:
`div.batch_actions_selector > a.dropdown_menu_button` is the button, but
`div.dropdown_menu_list_wrapper > ul > li > a` are the entries, and both match.
So every entry in the open menu also got `height: 30px; line-height: 30px;
border: 1px; padding: 0 12px; box-sizing: border-box` — a fixed-height boxed
row with a doubled 2px border against its neighbour, and any batch action
whose translated label wraps to two lines clipped at 30px.
Scope both copies to `> a`. The more specific `.dropdown_menu_list li a` rule
already styles the entries.
The hovered page link keeps its accent fill from the base pagination block, but the generic content-link rule added here repaints every anchor with var(--aa-link) — and the accent fill and the link colour are the same value. Measured on a hovered page number with stock defaults: #5ea3d3 on #5ea3d3, contrast 1.00. The digit is simply not there. Dark mode gives 1.16. The generic rule scores (2,6,1) — two ids plus six :not() clauses — so it cannot be out-specified by anything worth reading. Set the colour explicitly with !important, alongside the accent fill one line over that already uses it. This restores what master renders (white on the accent, 2.74). That is still short of 4.5 for small text, but it is the theme's long-standing look and a separate question from this regression.
The README described three variables. The file declares seventy-four, fifty-two of them added on this branch, and the only place they were written down was the comments in the stylesheet — which a consumer installing the gem never reads. Tables grouped by area, generated from the source so the defaults match what actually ships, with the light and dark defaults side by side. Also documents two things that were nowhere: how dark mode is selected (prefers-color-scheme, overridable per page with data-theme), and that variables are type-checked, so a wrong-typed override fails the build with a readable message rather than emitting CSS the browser throws away.
--aa-accent was written as the literal #5ea3d3 in both palettes, so a project that rebrands $skinMainSecondColor got its whole UI in the new colour and a stray default-blue ring on every focused input, select and textarea. It also replaced three pre-existing focus rules that did follow the accent (`border-color: lighten($skinMainSecondColor, 20%)`), so this is a regression in behaviour, not merely an omission. Give it a variable of its own, defaulting to the accent, with a dark twin defaulting to the light value — the same shape as the rest of the palette, so a project can point the focus ring somewhere else if it wants to.
The theme reads prefers-color-scheme and honours data-theme on <html>, but nothing in the gem ever sets that attribute and the README never said so — a dark mode a user cannot turn off is not much of a feature. No JavaScript ships: the gem is a stylesheet, and where a theme control belongs differs per application. What ships is the twenty lines that do it, verified against a real ActiveAdmin admin rather than written from memory — one click switches without a reload and the label follows. Also notes the two things easy to get wrong: applying the stored choice before DOMContentLoaded so the page does not flash the other theme, and falling back to prefers-color-scheme when no attribute is set so the label is honest.
Red is deliberate — a destructive action should not look like every other link, and this is the one place the theme keeps a second hue. But lifting the dark surfaces left #eb7b7b at 3.53:1 on a hovered row and 3.66:1 on a selected one, and those are exactly the rows it is read from: you hover a row to reach its Delete. #f49b9b clears 4.5 on every surface in the ladder, worst case 4.61 on hover. Same class of miss as the muted text two commits ago — the surfaces moved and the foregrounds sitting on them were not all rechecked.
Dark mode a user cannot turn off is not much of a feature, and ActiveAdmin 3 offers nothing to turn it off with — the switch is built into ActiveAdmin 4, which this theme does not support and never will. A hundred lines of vanilla JavaScript, no jQuery and no ujs. Two things taken from ActiveAdmin 4's implementation because they are better than what I wrote first: binding by delegation to .dark-mode-toggle so the control can live anywhere and survive a re-render, and a `storage` listener so switching in one tab applies in the others. One thing deliberately not taken. Theirs has two states and writes light or dark on the first click, so a user who touches it once can never return to following the system without clearing localStorage by hand. This cycles auto → light → dark → auto, where auto removes the attribute entirely and the media query decides again. Verified by driving the control four times in a real admin: the attribute and the stored key both clear on the third click. The stylesheet still works with none of this required.
One image, the same page a click apart, with the label in both states. The shot clears localStorage first and then clicks: the modes are reached through the control, not forced by the harness, so the picture shows what a user gets rather than what the stylesheet can be told to do.
`input:focus` sat next to the rule that paints the resting border, and that rule carries six `:not([type=…])` tests — six class-level points against the one `:focus` is worth. Same ids, same element, so the resting border won on every text input, select and textarea: focusing a field changed nothing at all, in either mode. Nesting `&:focus` inside that selector gives it the same `:not()` chain plus the pseudo-class, so it wins by construction and cannot drift apart from the selector it has to beat. Measured on an ActiveAdmin 3.5 edit form, sampling the focused field's top border: `#c9ced4` (the resting colour) before, `#5ea3d3` after, both modes.
The two overview shots scale the controls to the point where the fill and the border are a guess. This is the same admin cropped to the Account fieldset, a nested has_many row and the filter sidebar, 1:1 in both modes, with one field focused in each. $skinInputBgColorDark was also listed as #2c3137 in the variables table; the default is #1e2227.
Both collages ended on a hard crop: the table under the open batch-actions menu stopped halfway through a row, and the show/edit pair stopped halfway through the second token. The batch menu and the datepicker also sat in the same frame, where the picker covers the filter buttons it opened under. Rows are now laid out from whole frames — index, then show and the has_many form at half scale, then the batch menu beside the datepicker — so each ends where the page does.
ImageMagick applies -background in the order it is read, and it came after the smush that opened the gutters, so they were left the default white — a white column down the middle of the dark collage.
ActiveAdmin paints the hovered item in a dropdown menu with a `linear-gradient`, and a background image sits on top of a background colour. The theme set only `background-color`, so all three dropdown panels — title bar, table tools, batch actions — kept ActiveAdmin's stock #75a1c2→#608cb4 blue with white text, no matter what the theme or a project configured. Resetting `background-image` makes the colour reachable again. The hover is $skinSelectedRowColor, the same fill a checked table row already uses, so a pointer over a batch action and a selected row read as the same state.
ActiveAdmin 3 has none, and the stylesheet's dark mode is otherwise reachable only by changing the operating system. The script is opt-in, carries no jQuery or ujs, and the stylesheet works unchanged without it. One icon for the state you are in — half circle for auto, sun for light, moon for dark — and a click moves to the next. auto removes html[data-theme] so the media query decides and the page follows the system live; the other two pin the choice in localStorage under `aa-theme`. ActiveAdmin 4's own toggle writes light or dark on the first click and never writes auto back, so a user there cannot return to following the system without clearing storage by hand — hence the third state. The script writes nothing to the page but data-mode; the stylesheet draws the glyph from it, the way yeti-web already does with its own icon font. The glyphs are inline SVG used as a CSS mask, so the gem ships no image files, there is nothing for a host CSP to allow, and the icon takes currentColor — $skinMenuTextColor at rest, the bar's hover colour on hover. $theme-icon-auto, -light and -dark are !default, so a project can point them at its own icons. Binds by delegation to #theme_toggle or .dark-mode-toggle, so the control survives a re-render. A gem cannot add a menu item — ActiveAdmin builds the utility navigation from the host initializer — so with nothing declared the script injects its own li; the README documents declaring it instead, with yeti-web's config, and says why that is the better of the two.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Functional fallback, packaging, CSS specificity, validation, accessibility, and documentation issues remain unresolved.
Review effort: Balanced
Findings: 6
Open (9)
Comment on lines
+75
to
+79
| function cycle() { | ||
| var next = ORDER[mode()]; | ||
| store(next === "auto" ? null : next); | ||
| apply(); | ||
| refresh(); |
Comment on lines
+49
to
+51
| $skinHeaderPaddingY: null!default; | ||
| $skinHeaderPaddingTop: 4.5px!default; | ||
| $skinHeaderPaddingBottom: 4.5px!default; |
| --aa-label-text: #{$skinLabelColorDark}; | ||
| --aa-button: #{$skinButtonColorDark}; | ||
| --aa-button-text: #{$skinButtonTextColorDark}; | ||
| --aa-button-hover: #{lighten($skinButtonColorDark, 5%)}; |
Comment on lines
+261
to
+266
| skinSelectedRowColor: $skinSelectedRowColor, | ||
| skinSelectedRowColorDark: $skinSelectedRowColorDark, | ||
| skinAccentColor: $skinAccentColor, | ||
| skinAccentColorDark: $skinAccentColorDark, | ||
| skinButtonTextColor: $skinButtonTextColor, | ||
| skinButtonTextColorDark: $skinButtonTextColorDark |
| // (.table_tools_button), jQuery-UI tab anchors (.ui-tabs-anchor) — and delete | ||
| // links (handled by their own rule). Excluding them means those rules win on | ||
| // their own, with no need for !important. | ||
| a:not(.button):not(.action_item):not(.table_tools_button):not(.dropdown_menu_button):not(.ui-tabs-anchor):not(.delete_link):not([data-method="delete"]) { color: var(--aa-link); } |
Comment on lines
+123
to
+125
| ```js | ||
| // or, as an npm module | ||
| import "@activeadmin-plugins/active_admin_theme/app/assets/javascripts/wigu/theme_toggle"; |
Comment on lines
+131
to
+132
| `li#theme_toggle` into `#utility_nav` on load. That works, but the item is | ||
| appended after the server-rendered ones and is not yours to order or hide. |
|
|
||
| #### Core | ||
|
|
||
| | Variable | Default (light / dark) | | |
It was declared, documented as the way to set one value for top and bottom, and then never read: the header always took $skinHeaderPaddingTop and $skinHeaderPaddingBottom, which carried their own hard-coded defaults. The shorthand exists for yeti-web, which sets it. The halves now default to it when it is set, and still win when set themselves, since they are read after. Compiled: default 4.5px/4.5px, $skinHeaderPaddingY: 7px gives 7px/7px, and adding $skinHeaderPaddingTop: 2px on top gives 2px/7px. The comment also claimed the defaults were asymmetric. They have not been since the palette moved to yeti-web's configuration.
The generic content-link rule carries seven :not() exclusions, and each is worth a class-level point. At (2 ids, 7 classes, 1 element) it beat every component rule written later, so the exclusions filtered which elements the rule matched without stopping it winning on the ones it did. Measured in a browser on an ActiveAdmin 3.5 index, computed color: pagination page link rgb(56,103,139) -> rgb(50,53,55) Clear Filters button rgb(56,103,139) -> rgb(50,53,55) ordinary table link rgb(56,103,139) unchanged That is --aa-link where --aa-text was meant. :where() wraps the exclusions so they filter at zero weight; the last line is the check that nothing else moved, since lowering specificity can let something unintended win.
The label is white, so lightening the fill walks the contrast down on the state the pointer is on. Measured against white: light rest 2.74:1 hover was 2.37:1, now 3.27:1 dark rest 5.35:1 hover was 4.41:1, now 6.85:1 Darkening is also the conventional feedback for a filled button, so this costs nothing in appearance. Note the light resting value: 2.74:1 is what the theme has always rendered — $skinButtonColor defaults to $skinMainSecondColor, as on master — so it is not this branch's to change without changing every project's buttons.
store() swallowed the failure and mode() read the value straight back, so in private mode — where some browsers make localStorage throw — a click wrote nothing, read the old value, and the theme never changed. The control looked dead. The choice is now held in the module and storage is best-effort persistence. Checked in Node against the real file with a localStorage that throws on every call: four clicks give light -> dark -> auto -> light, where before they gave auto four times. The storage event re-syncs from event.newValue so another tab still wins.
package.json publishes src/**/* and prepublishOnly copied only the stylesheet directory into src, so theme_toggle.js was absent from the tarball — the import path the README gave could not resolve for anyone installing from npm. The gem was unaffected; its gemspec takes all of app. prepublishOnly now copies the script in too, and the documented path is @activeadmin-plugins/active_admin_theme/src/theme_toggle. Simulated the copy: the tarball gets src/active_admin_theme.scss and src/theme_toggle.js. The README also said the injected fallback is appended after the server-rendered items; insertBefore(nav.firstChild) puts it first.
The README says variables are typed, but the colour guard covered 26 of 63: $skinPageBgColor: 10px and $skinTextColor: none compiled without a word and emitted custom properties the browser drops, which is the exact failure the guards exist to catch. All 37 remaining colour variables are in. The four panel-header ones get their own guard: they default to var(--aa-page-bg) and are documented as accepting a custom property, so a plain type-of != color would reject the shipped value. rake css grew four rejection cases and three that must still compile, among them $skinPanelHeaderColor: var(--aa-surface) and $skinHeaderPaddingY: 7px. The switch icon variables also moved back above the comment describing the guards; they had been inserted between the two.
$skinActiveTabTextColor was the literal #5ea3d3, the same value $skinMainSecondColor carries, so a project that rebranded the accent kept a blue active tab and had to find a second variable to fix it. Same default, compiled: #5ea3d3 by default, #0066cc once the accent is set to #0066cc — where before it stayed #5ea3d3. This is the September review finding about hard-coded accents; it survived in this one place.
30 of the 52 documented defaults were wrong. The table is what people configure against, so it was telling them, among other things, that $skinMenuTextColor is #ffffff (it is #dfe2e6), that $skinMenuPillColor follows $skinMainSecondColor (it is #2e3236), that $skinTitleBarBorderWidth is 3px (it is 0) and that $skinLinkColor is #1f5f8d (it is #38678b). The drift came in when the defaults moved to yeti-web's configuration and the table did not. Every row is regenerated from the !default declarations. rake css now compares the two on every run and fails with the offending rows, so this cannot drift again unnoticed — checked by putting the old $skinLinkColor value back and watching it fail. The worked example also still said $skinHeaderPaddingY: null gives 5px/9px. Those defaults have not been asymmetric since the same move.
Pagination and the Clear Filters button are --aa-text now, and the hovered button darkens instead of lightening. 0.06% of the overview shots, 0.62% of the switch sheet.
$skinButtonColor was the accent itself, and white on #5ea3d3 is 2.74:1 against the 4.5:1 small text needs. Dark mode was already carrying the accent darkened 20%, at 5.35:1 — so the fix is to use that tone in both modes, which also stops the button being the one element that changes colour between them for no reason. light #5ea3d3 -> #2c709f white 2.74:1 -> 5.35:1 dark #2c709f unchanged white 5.35:1 hover #23597f white 6.84:1 The fill against its panel is untouched in dark mode (2.73:1, as before) — that is the surface contrast, not the label, and changing it would move the whole palette. This changes the default look for projects that never set $skinButtonColor, so Upgrading carries the one line that restores it. Screenshots reshot.
Merged
Fivell
added a commit
that referenced
this pull request
Oct 2, 2026
Major, not minor: #49 changed the default look for every project that does not set variables — anthracite header instead of blue, darker links and buttons, dropdown panels on the surface palette. README's Upgrading section lists the one-line override that restores each. Both artefacts checked with the theme switch in them: gem app/assets/javascripts/wigu/theme_toggle.js ships beside the stylesheet npm prepublishOnly + pack puts src/theme_toggle.js beside src/active_admin_theme.scss The npm install snippet in the README moves to ^3.0.0. Also drops the two references to ActiveAdmin 4. The gemspec pins activeadmin >= 3.0, < 4.0, so describing this theme against a version it does not support only invites the question of whether it does.
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
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.
No description provided.