maybe icon indicating copy to clipboard operation
maybe copied to clipboard

Improve dark mode styles across multiple pages

Open menaguilherme opened this issue 8 months ago • 5 comments

While using the app in dark mode, I noticed that in some places the text was hard to read or not visible at all due to low contrast.

This PR adjusts the styles on several pages to improve readability — mostly by tweaking background and text colors where the contrast was too low. I tried to stick to the existing theme and keep things consistent.

Let me know if you'd like any changes! Also, let me know if you’d like a short video demo — happy to record one if it helps!

menaguilherme avatar Apr 18 '25 18:04 menaguilherme

Patchset for more fixes

diff --git a/app/assets/tailwind/maybe-design-system.css b/app/assets/tailwind/maybe-design-system.css
index a0682155..71528c7d 100644
--- a/app/assets/tailwind/maybe-design-system.css
+++ b/app/assets/tailwind/maybe-design-system.css
@@ -848,4 +848,20 @@
   @variant theme-dark {
     @apply bg-white;
   }
+}
+
+@utility bg-divider-adaptive {
+  @apply bg-black;
+
+  @variant theme-dark {
+    @apply bg-white;
+  }
+}
+
+@utility bg-primary-adaptive {
+  @apply bg-white;
+
+  @variant theme-dark {
+    @apply bg-black ;
+  }
 }
\ No newline at end of file
diff --git a/app/views/budgets/_budget_nav.html.erb b/app/views/budgets/_budget_nav.html.erb
index 559d1a51..dd5743bb 100644
--- a/app/views/budgets/_budget_nav.html.erb
+++ b/app/views/budgets/_budget_nav.html.erb
@@ -31,7 +31,7 @@
         </div>
       <% end %>
 
-      <div class="h-px bg-alpha-black-200 w-12 group-last:hidden"></div>
+      <div class="h-px bg-divider-adaptive w-12 group-last:hidden"></div>
     </li>
   <% end %>
 </ul>
diff --git a/app/views/imports/_nav.html.erb b/app/views/imports/_nav.html.erb
index 6ee889d5..de8b8331 100644
--- a/app/views/imports/_nav.html.erb
+++ b/app/views/imports/_nav.html.erb
@@ -43,7 +43,7 @@
         </div>
       <% end %>
 
-      <div class="h-px bg-alpha-black-200 w-12 group-last:hidden"></div>
+      <div class="h-px bg-divider-adaptive w-12 group-last:hidden"></div>
     </li>
   <% end %>
 </ul>
diff --git a/app/views/layouts/wizard.html.erb b/app/views/layouts/wizard.html.erb
index d5a652f6..0e223ede 100644
--- a/app/views/layouts/wizard.html.erb
+++ b/app/views/layouts/wizard.html.erb
@@ -1,5 +1,5 @@
 <%= render "layouts/shared/htmldoc" do %>
-  <div class="flex flex-col h-dvh pt-safe">
+  <div class="bg-primary-adaptive flex flex-col h-dvh pt-safe">
     <header class="flex items-center justify-between p-8">
       <%= link_to content_for(:previous_path) || root_path do %>
         <%= lucide_icon "arrow-left", class: "w-5 h-5 text-secondary" %>
diff --git a/app/views/pages/dashboard/_balance_sheet.html.erb b/app/views/pages/dashboard/_balance_sheet.html.erb
index 7f8d7228..4bc652dc 100644
--- a/app/views/pages/dashboard/_balance_sheet.html.erb
+++ b/app/views/pages/dashboard/_balance_sheet.html.erb
@@ -17,7 +17,7 @@
               <div class="flex items-center gap-2 text-sm">
                 <div class="h-2.5 w-2.5 rounded-full" style="background-color: <%= account_group.color %>;"></div>
                 <p class="text-secondary"><%= account_group.name %></p>
-                <p class="text-black font-mono"><%= number_to_percentage(account_group.weight, precision: 0) %></p>
+                <p class="text-primary font-mono"><%= number_to_percentage(account_group.weight, precision: 0) %></p>
               </div>
             <% end %>
           </div>

Before

image

After

image image

neo773 avatar Apr 18 '25 19:04 neo773

@menaguilherme Updated patch for more fixes

neo773 avatar Apr 18 '25 19:04 neo773

@neo773 Thank you for taking the time to update the patch — I’ve applied the changes and pushed them!

menaguilherme avatar Apr 18 '25 19:04 menaguilherme

@zachgoll All feedback addressed — removed all dark: usages, replaced them with design tokens, and updated number_field and date_field to use the StyledFormBuilder.

Still a bit unsure how to best handle the bg-divider-adaptive cases — I reintroduced the utility in one spot where the token replacement didn’t quite work visually. Open to suggestions if there’s a better direction.

Appreciate the guidance!

menaguilherme avatar Apr 19 '25 21:04 menaguilherme

@zachgoll All feedback addressed — removed redundant class: usage from form fields, moved number_field and date_field into the builder’s metaprogramming block, and replaced the bg-divider-adaptive dividers with semantic <hr> elements using border-secondary.

Apologies for the extra back-and-forth — I missed a few things earlier but really appreciate the clear direction and your patience throughout. Let me know if there’s anything else I should adjust!

menaguilherme avatar Apr 21 '25 16:04 menaguilherme

Hey @zachgoll – I applied the requested changes and ran the linters you mentioned (rubocop, erb_lint, brakeman). Let me know if anything else is needed!

Thanks again for your time reviewing this 🙏

menaguilherme avatar Apr 22 '25 23:04 menaguilherme