Header menu colors, with the review fixes applied - #51
Merged
Merged
Conversation
Review of the two commits above surfaced 13 defects. The evidence for each is in docs/pr-47-fixes/ — one folder per defect, with before/after screenshots taken by compiling the theme against ActiveAdmin's own stylesheets and driving the menu with a real cursor in Chromium. Ships to every project, configured or not: * `width: auto; max-width: none` plus `white-space: nowrap` removed every bound on dropdown width, so a long label pushed the panel out of the viewport. Scrolling to reach it breaks the :hover chain that holds the menu open, so the item was unreachable by mouse. Measured at a 1280px viewport: 132.5px off-screen, scrollWidth 1420. Now content-sized with a ceiling ($skinMenuPanelMaxWidth) and wrapping past it. * Narrowing the bridge border to `> a` also stripped it from every dropdown anchor, where it had been acting as 7px of row separation. Rows went 34.3px to 27.3px. Restored as padding instead of a side effect. * $skinHeaderPaddingY collapsed the header's asymmetric 5px/9px into 7px each. Same total height, but every header element moved 2px down. Split into $skinHeaderPaddingTop/Bottom keeping the original values. Only reachable through the new variables: * A hard-coded `color: #ffffff` out-specified $skinMenuTextColor, so the variable had no effect on the hovered or the current item — and `.current` is not transient, so the current page's entry was invisible permanently. * The top-level pill was themable but its text was not; added $skinMenuPillTextColor. * $skinMenuFontSize sat on the `li`, so it rescaled the whole dropdown subtree rather than the header menu its comment describes. * The marker arrow kept a magic `top: 12px` while item height became configurable, and was pinned to a fixed-palette PNG that does not follow $skinMenuPanelColor. Now centred, and drawn in currentColor so it tracks the text colour. * The pill/panel junction showed the header colour through four rounded corners, and the bridge was 7px against ActiveAdmin's 5px gap. * `li.has_nested a` gave every leaf item a 20px arrow gutter it does not need. * `:focus` was unstyled, so keyboard navigation had no indicator. * $skinTitleBarButtonPadding* reached one of the two buttons in the title bar; the batch-actions button beside it stayed on the literal. * Wrong-typed overrides now hit the type guards added in #50, which this commit extends to all thirteen new variables. `rake css` covers the first and last of these directly: 8 override configurations compile, 6 wrong-typed ones are rejected, and no hard-coded white is left in the header menu rules. The same check reports 4 problems against the two commits below. Known limit, commented in place: a depth-2 flyout under a right-edge tab can still leave a 1280px viewport, down from 132.5px to 69.5px. CSS cannot see the distance to the viewport edge, and ActiveAdmin's own 175px panels already land within 5px of it.
$skinMenuPillTextColor defaulted to #ffffff independently of $skinMenuTextColor, so a project that light-themes the menu had to discover and set two variables instead of one: setting the panel and the dropdown text left the top-level pill label white on a light fill. Found by shooting the menu against a real ActiveAdmin admin rather than by reading — it is the same mistake this branch fixes elsewhere, made one variable over.
The previous commit claimed to fix "only one of eight identical `padding: 10px 20px` rules was extracted" by applying the new variables to `#title_bar #titlebar_right div.batch_actions_selector` as well. That rule matches nothing: ActiveAdmin renders the batch-actions selector only inside `div.table_tools` — `build_table_tools` in lib/active_admin/views/pages/index.rb, unchanged across 3.0 through 3.5 — so the title bar never contains one. The claim came from a hand-written screenshot harness whose markup I had invented, with a batch-actions button sitting inside `#titlebar_right`. Shooting the same scenario against a generated ActiveAdmin admin showed the real markup and the mistake with it. So there is no second button beside the title-bar action button to disagree with, `$skinTitleBarButtonPadding*` on `.action_item a` is complete as it stands, and the block this removes has been dead CSS for the whole supported ActiveAdmin range. The two live copies under `div.table_tools` keep their literal padding: they are a different row, and giving them a variable named for the title bar would be worse than leaving them alone.
It was a point-in-time record of reviewing these commits, and it does not belong in the gem: the before/after pairs are reproducible on demand against a generated ActiveAdmin admin, so carrying ~450 KB of PNGs and a static harness in the repository buys nothing that regenerating them does not. One of its cases was actively wrong. 09-titlebar-button-padding was shot against markup I had hand-written, with a batch-actions button inside #titlebar_right — ActiveAdmin puts it in div.table_tools and never there. That invented markup is where the withdrawn finding came from.
Closed
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 2, 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.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 2, 2026
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.
Fivell
added a commit
to yeti-switch/active_admin_theme
that referenced
this pull request
Oct 2, 2026
activeadmin-plugins#51 split it into $skinHeaderPaddingTop and $skinHeaderPaddingBottom to stop a symmetric default shifting the whole header, and dropped the old name. That is already on master — and yeti-web sets `$skinHeaderPaddingY: 4.5px`, so on the first release carrying activeadmin-plugins#51 its header silently reverts to 5px/9px with nothing in the build to say why. Keep both forms: the shorthand sets the two halves, the halves can still be set individually, and the asymmetric defaults are unchanged when neither is given. The shorthand defaults to null rather than a length, which is why it stays out of the length guard.
Fivell
added a commit
that referenced
this pull request
Oct 2, 2026
* add menu colors variables
* more variables
* expose more variables. dark mode
* scope buttons outline and configurable height
* fix menu overlapping
* login form respect variables
* add active menu item text color variable
* refactor table tools buttons
* disable pill higligting in #utility_nav and fix .col-selectable aligning
* pagination bar fixes
* pagination bar fixes
* skinTabPaddingX variable for tab headers padding
* fix table
* fix status tag centering
* fix comments
* Stop subtracting px from a radius the project chooses
`$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.
* Keep the menu font size off the li, where it leaks into the dropdown
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.
* Apply $skinMenuItemHoverTextColor, which was declared and never used
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 — #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.
* Make a checked table row look checked again
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.
* Give the batch-actions button its geometry, not the menu inside it
`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.
* Keep the hovered page number readable
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.
* Document every variable the theme exposes
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.
* Derive the focus accent from the theme accent
--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.
* Paint the dialog's primary button from the button palette it hovers to
Only its hover state was moved onto the new palette; the resting state kept
$skinMainSecondColor. With $skinButtonColor set to anything other than the
accent, the button therefore sat in one colour and jumped to an unrelated one
under the pointer — unlike every other primary button, and unlike its own
sibling, which was already converted.
* Give buttons a label colour to go with their fill
$skinButtonColor was added here so a project can give buttons a colour of
their own, and the comment above it says as much. The label stayed hard-coded
white, so the knob is only usable with a dark fill: $skinButtonColor #f0c14b
renders white on amber at 1.69, with nothing in the theme to fix it short of
writing CSS over the gem.
Pair it, light and dark, defaulting to white so nothing changes by default.
* Make status tag labels readable on their own fills
The five fills this branch introduces are all mid-tone, and the label was
white on every one of them: 3.21 on the neutral grey, 2.53 on green, 3.23 on
blue, 2.35 on amber, 3.78 on red. WCAG AA asks 4.5 for text this small, and
status tag text is both small and uppercase, which is the harder case.
Black clears it everywhere — 5.56 at worst, on the red — so the label is
black. #1a1d21 was the obvious softer choice and misses by 0.02 on that same
red, which is not a margin worth defending.
The fills are literals in the mixin calls rather than variables, so a single
ink is safe here; if they ever become configurable the label has to follow.
* Give the index view switcher the segmented geometry it shares with scopes
ActiveAdmin renders it as ul.indexes.table_tools_segmented_control with the
same .table_tools_button anchors as the scope bar, but every fix-up here --
the zeroed radius, the collapsed shared borders, the re-rounded first and
last segment, the selected fill -- was scoped to ul.scopes. A resource with
more than one `index as:` view therefore got three separate 4px pills with
doubled borders where they meet, and no way to tell which view is active.
* Drop fourteen variables nobody needs before they become API
This branch takes the theme from five public variables to seventy-eight. Once
released, every one of them is a promise: it cannot be withdrawn without a
major version. These fourteen are promises worth not making, and removing them
now is free.
* $skinTableToolButtonColor, $skinTableToolScopeColor and
$skinTableToolBatchActionsColor — three knobs for three groups of buttons in
one toolbar, all three defaulting to the surface colour. Their three custom
properties go with them; the rules read var(--aa-surface) directly.
* $skinTabInactiveColor, $skinActiveTabTextColor, $skinInactiveTabTextColor —
the tab bar already follows the panel header and the link colour, which is
where these defaulted anyway.
* $skinTableHeaderTextColor — the value stays, as a value.
The --aa-* custom properties remain: they are how the stylesheet carries a
colour between the two palettes, not something a project sets.
One deliberate behaviour change: the active tab label in dark mode now takes
the dark link colour rather than the light accent, 6.35 against the dark
surface instead of 5.46. That is what the removed variable's own default would
have given had it had a dark value of its own.
* Keep $skinBorderWindowColor working for index table headers
The README tells projects to set $skinBorderWindowColor, and some have for
years. The new content-scoped rule
`#wrapper #active_admin_content table thead th { background: var(--aa-surface-2) }`
scores (2,0,3) and outranks the theme's own `#wrapper table.index_table th`
at (1,1,2), so the setting stopped reaching the headers entirely — and the
default fill moved from #e6e9ee to #f0f2f5 for everyone at the same time.
Give index-table headers their own property: the documented variable in light
mode, the dark surface in dark mode. Plain content tables keep the surface
scale, which is what it is for.
* Let a.delete_link keep the delete colour without data-method
The generic content-link rule excludes `[data-method="delete"]` but not
`.delete_link`, and scores (2,6,1) against the delete rule's (2,1,1) — so a
destroy link rendered without that attribute came out in the link colour
while the one beside it carrying the attribute came out red.
Apps hit this with button_to, with Turbo's data-turbo-method, or with their
own .delete_link class. Add the class to the exclusion list rather than
relying on source order, which the specificity gap makes irrelevant anyway.
* Darken the default link colour until it passes on every light surface
$skinLinkColor was set to the theme accent, #5ea3d3. As a fill that is fine;
as body text it is 2.74 on white, against the 4.5 WCAG AA asks for text this
size — and it had replaced ActiveAdmin's own #38678b, which was 6.03. Every
record link, member action, filter and breadcrumb in the admin regressed.
Links also sit on zebra rows, table headers and selected rows, so white is
not the test that matters. Measured against all five light surfaces, the
worst case is the selected row at #d9e4ec; #1f5f8d clears it at 5.28 and
keeps the hue. Dark mode was already fine at 6.35 and is unchanged.
* Put back the four tab and table-header variables — they are in use
I removed these as speculative two commits ago. They are not: yeti-web, the
project this branch's dark mode was developed in, sets all four pairs in
app/assets/stylesheets/themes/variables.scss — inactive tabs recede to the
page background there, the active tab label takes the accent, and the index
table header text is pinned to one colour instead of a mix of blue and grey.
I judged "no use case" without looking at the one consumer that exists. The
three table-tool fills stay removed; those really are unused.
Their defaults are rewired to follow the variables they were duplicating
anyway — tab fill from the panel header, tab labels from the link and the
panel header text — so the defaults say what the relationship is, and a
project that wants them apart still can.
Verified by compiling the branch against yeti-web's variables.scss in full.
* Accept $skinHeaderPaddingY again, as a shorthand for both halves
#51 split it into $skinHeaderPaddingTop and $skinHeaderPaddingBottom to stop
a symmetric default shifting the whole header, and dropped the old name. That
is already on master — and yeti-web sets `$skinHeaderPaddingY: 4.5px`, so on
the first release carrying #51 its header silently reverts to 5px/9px with
nothing in the build to say why.
Keep both forms: the shorthand sets the two halves, the halves can still be
set individually, and the asymmetric defaults are unchanged when neither is
given. The shorthand defaults to null rather than a length, which is why it
stays out of the length guard.
* Repaint the dropdown list and its dividers for dark mode
The three dropdown blocks here restyle the wrapper and the item anchors, but
never the `ul.dropdown_menu_list` between them or its `li` dividers.
ActiveAdmin sets those to #FFF and a #ebebeb hairline, so in dark mode an open
batch-actions or title-bar menu is a white list bleeding through the panel's
rounded corners, with a bright line under every entry.
One rule each, at the same selector depth as the base so source order decides.
* Keep the title-bar action button's hover off its own dropdown entries
`.action_item a` is a descendant selector, and `action_item :x do dropdown_menu`
is a documented ActiveAdmin pattern this file styles ten lines further down.
So every entry in an open action-item menu took the button fill, the button
text colour and — with !important — a hover the entry rules could not undo:
measured in dark mode, a hovered entry came out at 1.72:1.
Scope it to the button itself and to the dropdown's own trigger.
* Fix the record count line everywhere, not only under .comments
The dark-mode colour for .pagination_information was scoped to .comments, but
ActiveAdmin sets it globally in _pagination.scss — so the index footer kept
#5c6469 on the #1a1d21 page background, 1.9:1. In "Displaying all 247 Users"
the bold record count is exactly the part that disappears, on every index page
in the admin.
* Style the plain dropdown button in the tool row like its neighbours
$skinTableToolsHeight is documented as the height of every table-tools button
including custom tools, and .dropdown_menu_button is listed in the font-size
reset right above it — but only a.table_tools_button and the batch-actions
anchor got the fill, height and border. A "Visible columns"-style tool
therefore kept ActiveAdmin's white gradient pill with grey text at 22px, 8px
shorter than the buttons beside it, and unreadable in dark mode.
* Address the datepicker by id, the way everything else addresses it
The dark-mode block used .ui-datepicker.ui-widget — two classes. ActiveAdmin
and this file's own datepicker block both use #ui-datepicker-div, and an id
outranks any number of classes, so none of it applied: the calendar kept its
light fills and #666 day numbers on the dark surface, 2.61:1. Repointing the
panel background in the same breath made that worse than it was, not better.
Same block addressed by id, with the header arrow, the weekday headings and
the day numbers repointed explicitly — each carries its own base rule.
* Write down what changes shape for projects already setting variables
Dropdown panels moved from $skinMainFirstColor/$skinMainSecondColor to the
surface palette — deliberate, it is what makes them work in both modes, but a
project that branded them through the two main colours loses that silently.
Same for the darker default link colour. Both have a one-line way back; say so
rather than leaving people to find out from a screenshot.
* Show a real configuration, not a three-line stub
The README's only example set three colours. A table of fifty-odd variables
does not tell anyone how they go together; yeti-web's configuration does, and
it is the project this theme's dark mode was built in. It also demonstrates
something not obvious: a CSS custom property works as a variable value, so
`var(--aa-page-bg)` covers both modes in one line.
* Outline the dropdown nipple instead of filling it flat
ActiveAdmin stacks three triangles to draw it: the element is the outline,
:before the inner shadow, :after the fill. All three were painted the same
colour, which collapses the arrow into one solid shape — fine against the old
dark panel, invisible once the panel became a light surface sitting on a light
page. Give the outline the border colour and the inner pair the fill, the same
two colours the panel itself is drawn with.
* Leave the datepicker header alone — it was already right in both modes
My previous commit repointed the month title and every anchor inside the
picker to var(--aa-text). The theme paints that header with the accent and a
white label, above and in both modes, so in light mode the title came out dark
on blue. A blanket `a` also caught the month arrows.
Only the calendar body needs the palette: the grid background, the weekday
headings, the day numbers and disabled days. The header keeps the accent fill
and its white label, which reads on either background.
* Lift tool buttons and dropdown panels off the dark page
Everything that floats above the page — the table-tools buttons, the
batch-actions button, the dropdown panels and the nipple fill — was painted
var(--aa-surface). In light that is white on a near-white page and reads fine;
in dark it is #24272c on #1a1d21, 1.13:1, so the batch-actions button looks
like a hole in the page and the nipple is not there at all.
One variable for the whole idea: light keeps the content surface, dark gets
#30353b, 1.37:1 against the page. Three separate knobs for three groups of
tool buttons were removed earlier in this branch; this is one knob for the
concept they were all circling.
* Show the theme as it actually looks now, in both modes
The README's single screenshot predates dark mode and most of the variables,
and shows an admin that no longer resembles what the gem renders. Replace it
with four: light and dark on the defaults, and light and dark with the
configuration from the worked example, so the variables table has something to
point at.
Shot against a generated ActiveAdmin 3.5 admin rather than a hand-made page.
img/ is outside the gemspec whitelist, so none of this ships in the gem.
* Adopt the configuration this theme is actually used with as the defaults
yeti-web is where this branch's dark mode was built, and its variables.scss
carries the design the theme is really run with: an anthracite menu instead of
the blue one, a dark title bar with no accent rule under it, panel headers and
inactive tabs receding to the page background so the active tab reads as
raised, and a darker link colour. Out of the box the gem still shipped the
2014 blue header that nobody keeps.
Thirty defaults move. Two settings from that file are deliberately not taken:
* $skinHeaderLogoMaxHeight — depends on the project's own logo.
* $skinDeleteLinkColorDark — yeti pins it to the link colour; the theme keeps
red, because losing the delete signal is a choice each project should make.
ActiveAdmin swaps in its dark submenu arrow for the current and hovered items,
assuming the highlight is lighter than the bar. With an anthracite pill it is
not, so the light arrow is pinned there.
This changes how every existing installation looks and wants a major version.
* Show the whole admin in one image per mode
One screenshot of an index table does not say much about a theme. Each mode now
gets a single composite: index with filters, show page, nested has_many form,
an open batch-actions menu and the datepicker — the parts a project actually
looks at, and the ones most likely to be wrong.
The worked example changes with them: yeti-web's configuration is the default
now, so the example that earns its place is how to get the old blue header
back.
* Tick two rows in the screenshot, not all of them
The batch-actions shot selected every row, so the whole table rendered in the
selection colour and read as a blue table rather than as a selection. Two rows
ticked shows what the state actually looks like next to unselected ones.
* Give the dark table header its own step again
I pointed --aa-table-header at $skinSurface2ColorDark a few commits ago while
restoring $skinBorderWindowColor for light mode. That variable is also the
zebra fill, so in dark mode the header band and every even row came out the
same colour — ΔL* 0.0, the header simply stopped existing. Light mode was
unaffected because it feeds from $skinBorderWindowColor, which is a step of
its own.
* Rebuild the dark palette as a ladder of distinguishable steps
The complaint was that dark mode looks flat and something is off with the
contrast. Measured, the text contrast was never the problem — body text sat at
10.8:1, AAA. The surfaces were: seven roles painted on five distinct
lightnesses, so the structure disappeared while the text shouted.
Measured in CIE L* rather than WCAG ratio, which is the wrong instrument for
dark-on-dark — its flare constant swamps the differences.
before after
header vs zebra ΔL* 0.0 → 4.9 the header band did not exist
hover vs zebra 2.2 → 9.0 half the rows gave no hover feedback
elevated vs hover 0.0 → 4.0 a tool button matched a hovered row
panel vs page 4.9 → 7.4
zebra vs panel 4.2 → 3.6 deliberately weaker: it is decoration,
not a state, and states need the budget
The selected row is separated by chroma instead of lightness — 45% saturation
against 20% on everything around it — which costs nothing from the lightness
budget and fixes the one real standards failure: a link on a selected row was
4.43:1, now 5.08:1.
Muted text goes further than the proposal I started from: lifting the hover
surface to #3f454d put #9ba3ad at 3.80:1 against it, and that pair is
reachable — an empty status tag is muted text inside a table cell, and the row
hovers. #b0b8c2 clears 4.5 on every surface in the ladder.
The button colour is left alone; which hue it should be is a separate question
from contrast. Light mode is untouched.
* Keep dark-mode buttons in the theme's own accent
The teal came across with yeti-web's configuration, and left dark mode with
three unrelated hues: teal buttons, blue links, a blue accent. Plain
$skinMainSecondColor cannot be used as the fill — a white label on it is
2.74:1 — but darkening it by 20% gives #2c709f at 5.4:1 and keeps one accent.
Derived rather than pinned, so a project that rebrands $skinMainSecondColor
gets a dark button that follows instead of a teal one that does not.
* Document how to switch themes, since nothing here does it
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.
* Reshoot both modes on the finished palette
* Keep the delete link readable on the rows it is read from
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.
* Ship a theme switch, since ActiveAdmin 3 has none
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.
* Show the switch rather than only describing it
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.
* Make the input focus indicator actually win
`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.
* Document the form and filter controls at full size
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.
* Rebuild the overview shots so nothing is cut mid-row
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.
* Fill the gutters with the page colour
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.
* Let the dropdown hover colour actually show
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.
* Ship an optional theme switch
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.
* Make $skinHeaderPaddingY do what it documents
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.
* Stop the link rule out-weighing the components it excludes
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.
* Darken the button on hover instead of lightening it
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.
* Keep the switch working where localStorage throws
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.
* Ship the theme switch in the npm package
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.
* Type-guard the whole palette, not a quarter of it
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.
* Derive the active tab label from the accent instead of repeating it
$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.
* Rewrite the variables table from the declarations, and keep it there
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.
* Reshoot on the fixed palette
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.
* Darken the primary button so its white label reads
$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.
---------
Co-authored-by: Igor Fedoronchuk <fedoronchuk@gmail.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
2 participants
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.
Supersedes #47. @dmitry-s's two commits are cherry-picked unchanged with authorship preserved; the commits on top fix what a review of them turned up.
Ships to every project, configured or not
width: auto; max-width: nonewithwhite-space: nowrapremoved every ceiling, and a long label pushed the panel past the window edge — where it cannot be reached, because scrolling breaks the:hoverchain that holds the menu open. Measured at 1280px: panel right edge 1286 vs master's 1158, with a horizontal scrollbar appearing. Nowwidth: max-contentwith a$skinMenuPanelMaxWidthceiling (260px) and wrapping past it: right edge 1223, 57px of room left.> aalso stripped the 7px border from every anchor in the open panel, where it was acting as row separation. Panel 157 → 136px. Restored with padding instead: 154px.$skinHeaderPaddingY: 7pxreplaced the previous asymmetric 5px/9px. Same total height, every element shifted. Split into$skinHeaderPaddingTop/Bottomwith the original values.Only reachable through the new variables
color: #ffffffout-specified$skinMenuTextColor, so the variable this branch exists to add had no effect on the hovered item or the current one — and.currentis not transient, so the current page's entry was invisible permanently. The top-level pill had the same problem;$skinMenuPillTextColorwas added, defaulting to$skinMenuTextColorso a light menu is one variable, not two.$skinMenuFontSizesat on theli, so it rescaled the whole dropdown rather than the header menu its comment describes.top: 12pxwhile row height became configurable, and was a fixed-palette PNG that ignores$skinMenuPanelColor(1.77:1 on a dark panel). Now centred, and drawn as a CSS triangle incurrentColor.:focuswas unstyled, so keyboard navigation had no indicator of the theme's own.Checked
rake css— 8 override configurations compile, 6 wrong-typed ones are rejected, and no hard-coded white survives in the header menu rules. The same check reports 4 problems against the two cherry-picked commits alone.Every case was also shot against a generated ActiveAdmin 3.5 admin, one screenshot per commit. That is what caught
$skinMenuPillTextColordefaulting to white independently of$skinMenuTextColor, which reading the diff did not.One review finding withdrawn
An earlier commit here claimed to fix "only one of eight identical
padding: 10px 20pxrules was extracted" by also applying the variables to#title_bar #titlebar_right div.batch_actions_selector. That selector matches nothing: ActiveAdmin renders the batch-actions selector only insidediv.table_tools, in every version from 3.0 to 3.5. The finding came from a hand-written screenshot harness whose markup I had invented. The last commit removes that dead block instead;$skinTitleBarButtonPadding*on.action_item ais complete as it stands.