Fix stub login banner in dark mode, add dark mode toggle to login screen - #18
Merged
Conversation
Fix: the stub login banner was unreadable in dark mode. The banner styled itself with Tailwind utilities (bg-red-50, text-red-900, dark:bg-red-900/30 and friends). Those classes live in a view shipped inside the gem, which the host application's Tailwind build never scans, so the CSS for them was never generated. The banner rendered with no background and no text color -- effectively white on white. Replaced them with inline styles that need no build step: a translucent red tint and border, and no explicit text color so the text inherits the page's, which reads on both a light and a dark card. Applied to the gem view (both the Active Admin 4 and 3 branches) and to both generator templates. Improvement: added Active Admin's dark mode toggle to the login screen. Active Admin only renders the toggle in the signed-in top bar, so a signed out admin had no way to switch themes. The logged out layout already renders active_admin/html_head, which loads the JS that delegates clicks on .dark-mode-toggle, so the button needs no script of its own -- only the markup, copied from Active Admin's own _site_header partial. Added to the Active Admin 4 markup only; Active Admin 3 has no dark mode. The auto margin is an inline style rather than ms-auto for the same reason as above: the gem's views are outside the host's Tailwind content path, so only utilities Active Admin itself already emits are safe to use here. Co-Authored-By: Clanker
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The toggle lacks a visible keyboard-focus indicator, and the new behavior needs regression coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves login-page dark-mode support and fixes the stub-login warning’s visibility.
Changes:
- Adds an AA4 login-page dark-mode toggle.
- Replaces unavailable Tailwind banner styles with inline styling.
- Bumps the gem version to 2.2.1.
File summaries
| File | Description |
|---|---|
app/views/active_admin/devise/sessions/new.html.erb |
Updates bundled AA3/AA4 login views. |
lib/generators/active_admin/oidc/install/templates/sessions_new.html.erb |
Fixes generated AA3 banner styling. |
lib/generators/active_admin/oidc/install/templates/sessions_new_v4.html.erb |
Adds the AA4 toggle and fixes banner styling. |
lib/activeadmin/oidc/version.rb |
Bumps the patch version. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Only Tailwind utilities that Active Admin itself emits are compiled, because the gem's views sit outside the host application's Tailwind content path. The previous commit worked around that with inline styles; reuse Active Admin's own class strings instead, so the login screen matches the rest of the admin and follows its theme. * The stub login banner now uses the exact classes and icon of Active Admin's error flash (_flash_messages.html.erb). * The dark mode toggle uses ms-auto, which _main_navigation.html.erb emits, in place of an inline margin-inline-start. The Active Admin 3 branch keeps its plain inline style: there is no Tailwind and no dark mode there. Co-Authored-By: Clanker
* Drop focus:outline-hidden from the dark mode toggle. It was copied from Active Admin's own site header, but nothing replaces the outline it removes, so a keyboard user could not see the button focused. The login card has no other control to borrow a focus style from. * Cover the toggle in login_page_spec: present on Active Admin 4, absent on Active Admin 3. The markup is the whole feature -- the JS that drives it already ships with Active Admin -- so dropping it fails silently. * Cover the banner styling in stub_login_spec. The old test asserted only the warning text, so it passed while the banner rendered white on white. Each matrix run now asserts the styling that run actually compiles: the dark variant on Active Admin 4, the inline style on Active Admin 3. Both specs were checked against a deliberately broken view to confirm they fail when the markup goes away. Co-Authored-By: Clanker
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new toggle icon has insufficient light-mode contrast in both AA4 view variants.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
text-gray-400 on the white login card is about 2.5:1, under the 3:1 that a graphical control needs. Move the light default to gray-500 and the hover to gray-700. Do the same on the dark side, where gray-500 on the dark card was equally thin: gray-400, hovering to gray-200. All four utilities are ones Active Admin already emits, so the host's Tailwind build has them. hover:text-gray-600, which the review suggested, is not emitted anywhere in Active Admin and would not have compiled. Co-Authored-By: Clanker
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.
Fix — stub login banner unreadable in dark mode
The banner added in #16 styled itself with Tailwind utilities (
bg-red-50,text-red-900,dark:bg-red-900/30, …). Those classes live in a view shipped inside the gem, which the host application's Tailwind build never scans, so the CSS for them was never generated. The banner rendered with no background and no text color — effectively white on white.Replaced with inline styles that need no build step: a translucent red tint and border, and no explicit text color, so the text inherits the page's and reads on both a light and a dark card.
Applied to the gem view (both the AA4 and AA3 branches) and to both generator templates.
Improvement — dark mode toggle on the login screen
Active Admin only renders its dark mode toggle in the signed-in top bar, so a signed-out admin had no way to switch themes.
The logged-out layout already renders
active_admin/html_head, which loads the JS that delegates clicks on.dark-mode-toggle— so the button needs no script of its own, only the markup, copied from Active Admin's own_site_headerpartial.Added to the AA4 markup only; AA3 has no dark mode. The auto margin is an inline style rather than
ms-autofor the same reason as above: the gem's views are outside the host's Tailwind content path, so only utilities Active Admin itself already emits are safe to use here.Notes
Testing
bundle exec rspec— 146 examples, 0 failures.🤖 Generated with Claude Code
https://claude.ai/code/session_0167PK3Dbjyym6mCRrydTFCb