Skip to content

fix: make form errors and has_many headings readable - #79

Merged
senid231 merged 2 commits into
activeadmin-plugins:masterfrom
Limeone:theme-fixes
Nov 5, 2026
Merged

senid231 merged 2 commits into
activeadmin-plugins:masterfrom
Limeone:theme-fixes

Conversation

@Limeone

@Limeone Limeone commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Form errors in dark mode. The theme never coloured validation messages, so
ActiveAdmin's #932419 text and #fae6e4 summary block carried over into dark
mode: the message under a field was dark red on the dark panel, and the summary
was a pale block above the form.

has_many headings. ActiveAdmin renders the container as an li when
f.has_many is inside f.inputs and as a div otherwise. The theme's rule
matched only the li, so the div kept ActiveAdmin's #5e6469, which is dim on
the dark page. The li heading's fixed #8494a8 was pale on the light page.

Fix

  • Add $skinErrorTextColor and $skinErrorBgColor with Dark variants, exposed as
    --aa-error and --aa-error-bg. The light defaults are the values ActiveAdmin
    already paints, so light forms keep their look.
  • Colour both has_many containers with --aa-text-muted.
  • Give these pairs the contrast guard status tags already have in
    test/css_check.rb, reject invalid values for the new variables, and add them
    to the README table.

Visible changes in light mode

Light forms are unchanged, but has_many headings are not: the li heading gets
darker, and the div heading gets slightly lighter.

Measured

test/dummy at 1200px, computed colours, contrast against the fill in brackets:

Element Mode Before After
Inline error on panel #25292f dark #932419 (1.74) #f49b9b (6.97)
Error summary dark #932419 on #fae6e4 #f49b9b on #3a2224 (6.98)
has_many heading, div, on #161a1e dark #5e6469 (2.92) #b0b8c2 (8.73)
has_many heading, li, on #161a1e dark #8494a8 (5.65) #b0b8c2 (8.73)
has_many heading, div, on #f7f9fb light #5e6469 (5.68) #6b7177 (4.68)
has_many heading, li, on #f7f9fb light #8494a8 (2.93) #6b7177 (4.68)

The has_many markup on the test page was written by hand from HasManyBuilder.

Form errors: the theme never coloured validation messages, so ActiveAdmin's
#932419 text and #fae6e4 summary block carried over into dark mode: dark red
messages on the dark panel and a pale block above the form. Add
$skinErrorTextColor and $skinErrorBgColor with Dark variants. The light
defaults are the values ActiveAdmin already paints, so light forms keep
their look.

Breadcrumbs: the colour was derived from $skinMainFirstColor, but the text
sits on the title bar. Mix it from $skinTitleBarColor, the fill it is on.

has_many headings: ActiveAdmin renders the container as an li when f.has_many
is inside f.inputs and as a div otherwise. The old rule matched only the li,
so the div kept ActiveAdmin's #5e6469. Both now use --aa-text-muted.

css_check gives these pairs the contrast guard status tags already have, and
rejects invalid values for the new variables.

Claude-Session: https://claude.ai/code/session_01TXyspr287VnxtQEFUec2t3
@senid231 senid231 self-assigned this Nov 5, 2026
@senid231
senid231 requested a balanced review from Copilot November 5, 2026 09:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The contrast check can falsely pass invisible transparent text, and the breadcrumb styling produces unreadable white-on-white text with the documented light title-bar override.

2 open issues
What changed in this PR

This PR improves readability in the ActiveAdmin theme by adding mode-aware validation colours and updating breadcrumbs and nested-form headings.

Changes:

  • Add light and dark error colours, CSS custom properties, and input validation.
  • Adjust breadcrumb colours and style both has_many container variants.
  • Add contrast checks and document the new variables with colour swatches.
File Description
test/​css_check.rb Adds invalid-colour cases and message contrast checks.
README.md Documents validation text and summary colours.
docs/​swatches/​fae6e4.svg Adds the light error-summary background swatch.
docs/​swatches/​932419.svg Adds the light validation-text swatch.
docs/​swatches/​3a2224.svg Adds the dark error-summary background swatch.
app/​assets/​stylesheets/​wigu/​active_admin_theme.scss Defines error colours and updates validation, breadcrumb, and heading styles.

Review effort: Balanced

Comment thread test/css_check.rb Resolved
Mixing the breadcrumb from $skinTitleBarColor only toward white made it white
on a white title bar. Go back to the colour master ships, and drop the
breadcrumb from the contrast guard, which that colour does not pass.

messages_are_readable dropped alpha, so a transparent colour scored as black
and could pass. Read it and report translucent pairs, as
status_tag_labels_are_readable does.

Claude-Session: https://claude.ai/code/session_01TXyspr287VnxtQEFUec2t3
@Limeone Limeone changed the title fix: make form errors, breadcrumbs and has_many headings readable fix: make form errors and has_many headings readable Nov 5, 2026
@Limeone
Limeone requested a balanced review from Copilot November 5, 2026 09:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain for the validation-message and has_many heading styling changes.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

@senid231
senid231 merged commit 26e072e into activeadmin-plugins:master Nov 5, 2026
3 checks passed
@senid231 senid231 mentioned this pull request Nov 5, 2026
senid231 added a commit that referenced this pull request Nov 5, 2026
Minor: #79 adds public SCSS variables for validation errors, so this is new
API, not only a fix. Light forms keep their look.

Co-Authored-By: Clanker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants