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 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 was referenced Oct 2, 2026
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 5, 2026
Two reports after 3.0.0, and what verifying them turned up. ## Status tags The label was black on a mid-tone fill. White on its own was the wrong fix — against the five shipped fills it lands between 2.35:1 and 3.78:1, under the 4.5:1 small text needs, which is why it went black during the review of activeadmin-plugins#49. So the fills move down with the label. Measured against white: neutral #8a909a -> #707681 3.21 -> 4.57 ok #8daa92 -> #5e7e63 2.53 -> 4.53 notice #6090db -> #3874d2 3.23 -> 4.57 warn #e29b20 -> #9e6c15 2.35 -> 4.56 error #d45f53 -> #ce483b 3.78 -> 4.55 The five fills join $skinStatusTagTextColor as variables, so the other choice is still one line away. They were literals inside the mixin call before. ## Column headings $skinTableHeaderTextColor was #5e6469, 4.93:1 against the header fill while the rows underneath run at 10.15:1 — passing, but visibly softer than the data it labels, which is what made it look blurry. It takes $skinTextColor now. They also did not line up with their own columns. ActiveAdmin puts the sort arrow inside the heading link, on the left, cleared with `padding-left: 13px`. Cell and heading share the same 12px padding, so the cell is aligned, but the label inside is pushed 13px right while the data below starts at the padding edge. Measured on a block column of status tags, the shape the report came from: cell left 559.9 559.9 heading link left 571.9 571.9 tag left 571.9 571.9 heading TEXT left 584.9 571.9 Moving the image to the right edge is not enough: the link is `display: block`, so the arrow would park at the far side of the column. It is a pseudo-element now, which puts it beside the label, keeps the link full width so the whole cell stays clickable, and lets the marker take currentColor — the stock sprite is a fixed grey PNG that cannot follow the text into dark mode. At 0.6 opacity it is 3.40:1 light and 4.22:1 dark against the header fill; it is the only thing separating a sortable heading from a plain one, so WCAG 1.4.11 asks 3:1. ## The check that was supposed to prevent this rake css compares the README variables table against the declarations. It was doing so in one direction only, and loosely: - a variable with no row in the table passed, while the success line claimed the table matched every declaration; - a blank cell counted as documentation — the name entered the documented set, which exempted it from the undocumented check, while the mismatch check skipped it for having no value, so it passed on both sides; - a name listed twice kept the last row, so the table could agree with the stylesheet while a reader meets the stale row first; - a name declared twice went unnoticed, though Sass keeps the first !default and drops the rest. All four fail now, each reproduced before and after. The advertised count comes from the declarations rather than a hand-maintained constant that said 52 while 81 variables were being compared.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 6, 2026
Two reports after 3.0.0, and what verifying them turned up. ## Status tags The label was black on a mid-tone fill. White on its own was the wrong fix — against the five shipped fills it lands between 2.35:1 and 3.78:1, under the 4.5:1 small text needs, which is why it went black during the review of activeadmin-plugins#49. So the fills move down with the label. Measured against white: neutral #8a909a -> #707681 3.21 -> 4.57 ok #8daa92 -> #5e7e63 2.53 -> 4.53 notice #6090db -> #3874d2 3.23 -> 4.57 warn #e29b20 -> #9e6c15 2.35 -> 4.56 error #d45f53 -> #ce483b 3.78 -> 4.55 The five fills join $skinStatusTagTextColor as variables, so the other choice is still one line away. They were literals inside the mixin call before. The label applies to filled tags; empty, unknown and none have no fill and keep $skinTextMutedColor, which the table now says. ## Column headings $skinTableHeaderTextColor was #5e6469, 4.93:1 against the header fill while the rows underneath run at 10.15:1 — passing, but visibly softer than the data it labels, which is what made it look blurry. It takes $skinTextColor now. They also did not line up with their own columns. ActiveAdmin puts the sort arrow inside the heading link, on the left, cleared with `padding-left: 13px`. Cell and heading share the same 12px padding, so the cell is aligned, but the label inside is pushed 13px right while the data below starts at the padding edge. Measured on a block column of status tags, the shape the report came from: cell left 559.9 559.9 heading link left 571.9 571.9 tag left 571.9 571.9 heading TEXT left 584.9 571.9 Moving the image to the right edge is not enough: the link is `display: block`, so the arrow would park at the far side of the column. It is a pseudo-element now, which puts it beside the label, keeps the link full width so the whole cell stays clickable, and lets the marker take currentColor — the stock sprite is a fixed grey PNG that cannot follow the text into dark mode. At 0.6 opacity it is 3.40:1 light and 4.22:1 dark against the header fill; it is the only thing separating a sortable heading from a plain one, so WCAG 1.4.11 asks 3:1. ## The check that was supposed to prevent this rake css compares the README variables table against the declarations. It was doing so in one direction only, and loosely: - a variable with no row in the table passed, while the success line claimed the table matched every declaration; - a blank cell counted as documentation — the name entered the documented set, which exempted it from the undocumented check, while the mismatch check skipped it for having no value, so it passed on both sides; - skipping blank cells outright then opened a narrower version of the same hole: one blank row and one good row for the same name, where the good row satisfied the lookup and the blank one never reached duplicate detection; - a name listed twice kept the last row, so the table could agree with the stylesheet while a reader meets the stale row first; - a name declared twice went unnoticed, though Sass keeps the first !default and drops the rest. Every row counts towards duplicate detection now, blank or not, and a blank cell fails on its own. Each hole was reproduced before and after. The advertised count comes from the declarations rather than a hand-maintained constant that said 52 while 81 variables were being compared.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 6, 2026
Two reports after 3.0.0, and what verifying them turned up. ## Status tags The label was black on a mid-tone fill. White on its own was the wrong fix — against the five shipped fills it lands between 2.35:1 and 3.78:1, under the 4.5:1 small text needs, which is why it went black during the review of activeadmin-plugins#49. So the fills move down with the label. Measured against white: neutral #8a909a -> #707681 3.21 -> 4.57 ok #8daa92 -> #5e7e63 2.53 -> 4.53 notice #6090db -> #3874d2 3.23 -> 4.57 warn #e29b20 -> #9e6c15 2.35 -> 4.56 error #d45f53 -> #ce483b 3.78 -> 4.55 The five fills join $skinStatusTagTextColor as variables, so the other choice is still one line away. They were literals inside the mixin call before. The label applies to filled tags; empty, unknown and none have no fill and keep $skinTextMutedColor, which the table now says. This pairing is the one thing here that has been got wrong twice — once by shipping white on a mid-tone fill, once by darkening the fill and leaving the label behind — and nothing was checking it. rake css now computes the ratio for every tag rule the theme emits and fails under 4.5:1. ## Column headings $skinTableHeaderTextColor was #5e6469, 4.93:1 against the header fill while the rows underneath run at 10.15:1 — passing, but visibly softer than the data it labels, which is what made it look blurry. It takes $skinTextColor now. They also did not line up with their own columns. ActiveAdmin puts the sort arrow inside the heading link, on the left, cleared with `padding-left: 13px`. Cell and heading share the same 12px padding, so the cell is aligned, but the label inside is pushed 13px right while the data below starts at the padding edge. Measured on a block column of status tags, the shape the report came from: cell left 559.9 559.9 heading link left 571.9 571.9 tag left 571.9 571.9 heading TEXT left 584.9 571.9 Moving the image to the right edge is not enough: the link is `display: block`, so the arrow would park at the far side of the column. It is a pseudo-element now, which puts it beside the label, keeps the link full width so the whole cell stays clickable, and lets the marker take currentColor — the stock sprite is a fixed grey PNG that cannot follow the text into dark mode. At 0.6 opacity it is 3.40:1 light and 4.22:1 dark against the header fill; it is the only thing separating a sortable heading from a plain one, so WCAG 1.4.11 asks 3:1. The unused side of the triangle has no width rather than a transparent one, so the box is as tall as the mark in it and both sort states sit at the same height. Keeping all four sides and nudging the ascending one with a margin put them visibly apart, because `vertical-align: middle` centres the box and the visible half then sits off-centre within it. ## The check that was supposed to prevent this rake css compares the README variables table against the declarations. It was doing so in one direction only, and loosely: - a variable with no row in the table passed, while the success line claimed the table matched every declaration; - a blank cell counted as documentation — the name entered the documented set, which exempted it from the undocumented check, while the mismatch check skipped it for having no value, so it passed on both sides; - skipping blank cells outright then opened a narrower version of the same hole: one blank row and one good row for the same name, where the good row satisfied the lookup and the blank one never reached duplicate detection; - a name listed twice kept the last row, so the table could agree with the stylesheet while a reader meets the stale row first; - a name declared twice went unnoticed, though Sass keeps the first !default and drops the rest. Every row counts towards duplicate detection now, blank or not, and a blank cell fails on its own. Each hole was reproduced before and after. The advertised count comes from the declarations rather than a hand-maintained constant that said 52 while 81 variables were being compared.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 6, 2026
Two reports after 3.0.0, and what verifying them turned up. ## Status tags The label was black on a mid-tone fill. White on its own was the wrong fix — against the five shipped fills it lands between 2.35:1 and 3.78:1, under the 4.5:1 small text needs, which is why it went black during the review of activeadmin-plugins#49. So the fills move down with the label. Measured against white: neutral #8a909a -> #707681 3.21 -> 4.57 ok #8daa92 -> #5e7e63 2.53 -> 4.53 notice #6090db -> #3874d2 3.23 -> 4.57 warn #e29b20 -> #9e6c15 2.35 -> 4.56 error #d45f53 -> #ce483b 3.78 -> 4.55 The five fills join $skinStatusTagTextColor as variables, so the other choice is still one line away. They were literals inside the mixin call before. The label applies to filled tags; empty, unknown and none have no fill and keep $skinTextMutedColor, which the table now says. This pairing is the one thing here that has been got wrong twice — once by shipping white on a mid-tone fill, once by darkening the fill and leaving the label behind — and nothing was checking it. rake css now computes the ratio for every tag rule the theme emits and fails under 4.5:1. ## Column headings $skinTableHeaderTextColor was #5e6469, 4.93:1 against the header fill while the rows underneath run at 10.15:1 — passing, but visibly softer than the data it labels, which is what made it look blurry. It takes $skinTextColor now. They also did not line up with their own columns. ActiveAdmin puts the sort arrow inside the heading link, on the left, cleared with `padding-left: 13px`. Cell and heading share the same 12px padding, so the cell is aligned, but the label inside is pushed 13px right while the data below starts at the padding edge. Measured on a block column of status tags, the shape the report came from: cell left 559.9 559.9 heading link left 571.9 571.9 tag left 571.9 571.9 heading TEXT left 584.9 571.9 Moving the image to the right edge is not enough: the link is `display: block`, so the arrow would park at the far side of the column. It is a pseudo-element now, which puts it beside the label, keeps the link full width so the whole cell stays clickable, and lets the marker take currentColor — the stock sprite is a fixed grey PNG that cannot follow the text into dark mode. At 0.6 opacity it is 3.40:1 light and 4.22:1 dark against the header fill; it is the only thing separating a sortable heading from a plain one, so WCAG 1.4.11 asks 3:1. The unused side of the triangle has no width rather than a transparent one, so the box is as tall as the mark in it and both sort states sit at the same height. Keeping all four sides and nudging the ascending one with a margin put them visibly apart, because `vertical-align: middle` centres the box and the visible half then sits off-centre within it. ## The check that was supposed to prevent this rake css compares the README variables table against the declarations. It was doing so in one direction only, and loosely: - a variable with no row in the table passed, while the success line claimed the table matched every declaration; - a blank cell counted as documentation — the name entered the documented set, which exempted it from the undocumented check, while the mismatch check skipped it for having no value, so it passed on both sides; - skipping blank cells outright then opened a narrower version of the same hole: one blank row and one good row for the same name, where the good row satisfied the lookup and the blank one never reached duplicate detection; - a name listed twice kept the last row, so the table could agree with the stylesheet while a reader meets the stale row first; - a name declared twice went unnoticed, though Sass keeps the first !default and drops the rest. Every row counts towards duplicate detection now, blank or not, and a blank cell fails on its own. Each hole was reproduced before and after. The advertised count comes from the declarations rather than a hand-maintained constant that said 52 while 81 variables were being compared.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 6, 2026
Two reports after 3.0.0, and what verifying them turned up. ## Status tags The label was black on a mid-tone fill. White on its own was the wrong fix — against the five shipped fills it lands between 2.35:1 and 3.78:1, under the 4.5:1 small text needs, which is why it went black during the review of activeadmin-plugins#49. So the fills move down with the label. Measured against white: neutral #8a909a -> #707681 3.21 -> 4.57 ok #8daa92 -> #5e7e63 2.53 -> 4.53 notice #6090db -> #3874d2 3.23 -> 4.57 warn #e29b20 -> #9e6c15 2.35 -> 4.56 error #d45f53 -> #ce483b 3.78 -> 4.55 The five fills join $skinStatusTagTextColor as variables, so the other choice is still one line away. They were literals inside the mixin call before. The label applies to filled tags; empty, unknown and none have no fill and keep $skinTextMutedColor, which the table now says. This pairing is the one thing here that has been got wrong twice — once by shipping white on a mid-tone fill, once by darkening the fill and leaving the label behind — and nothing was checking it. rake css now computes the ratio for every tag rule the theme emits and fails under 4.5:1. ## Column headings $skinTableHeaderTextColor was #5e6469, 4.93:1 against the header fill while the rows underneath run at 10.15:1 — passing, but visibly softer than the data it labels, which is what made it look blurry. It takes $skinTextColor now. They also did not line up with their own columns. ActiveAdmin puts the sort arrow inside the heading link, on the left, cleared with `padding-left: 13px`. Cell and heading share the same 12px padding, so the cell is aligned, but the label inside is pushed 13px right while the data below starts at the padding edge. Measured on a block column of status tags, the shape the report came from: cell left 559.9 559.9 heading link left 571.9 571.9 tag left 571.9 571.9 heading TEXT left 584.9 571.9 Moving the image to the right edge is not enough: the link is `display: block`, so the arrow would park at the far side of the column. It is a pseudo-element now, which puts it beside the label, keeps the link full width so the whole cell stays clickable, and lets the marker take currentColor — the stock sprite is a fixed grey PNG that cannot follow the text into dark mode. At 0.6 opacity it is 3.40:1 light and 4.22:1 dark against the header fill; it is the only thing separating a sortable heading from a plain one, so WCAG 1.4.11 asks 3:1. The unused side of the triangle has no width rather than a transparent one, so the box is as tall as the mark in it and both sort states sit at the same height. Keeping all four sides and nudging the ascending one with a margin put them visibly apart, because `vertical-align: middle` centres the box and the visible half then sits off-centre within it. ## The check that was supposed to prevent this rake css compares the README variables table against the declarations. It was doing so in one direction only, and loosely: - a variable with no row in the table passed, while the success line claimed the table matched every declaration; - a blank cell counted as documentation — the name entered the documented set, which exempted it from the undocumented check, while the mismatch check skipped it for having no value, so it passed on both sides; - skipping blank cells outright then opened a narrower version of the same hole: one blank row and one good row for the same name, where the good row satisfied the lookup and the blank one never reached duplicate detection; - a name listed twice kept the last row, so the table could agree with the stylesheet while a reader meets the stale row first; - a name declared twice went unnoticed, though Sass keeps the first !default and drops the rest. Every row counts towards duplicate detection now, blank or not, and a blank cell fails on its own. Each hole was reproduced before and after. The advertised count comes from the declarations rather than a hand-maintained constant that said 52 while 81 variables were being compared.
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.