gavin/maybe
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

Improve dark mode styles across multiple pages #2125

Merged
menaguilherme merged 9 commits from fix/dark-mode-contrast into main 2025-04-23 21:42:30 +08:00
Conversation 13 Commits 9 Files Changed 63 +209 -154
menaguilherme commented 2025-04-19 02:27:19 +08:00 (Migrated from github.com)
Copy Link

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!

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!
❤️ 2
neo773 commented 2025-04-19 03:06:40 +08:00 (Migrated from github.com)
Copy Link

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
Patchset for more fixes ```diff 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 <img width="600" alt="image" src="https://github.com/user-attachments/assets/a2b2db6d-f822-4f6a-ba9e-8cb9c94c8666" /> After <img width="600" alt="image" src="https://github.com/user-attachments/assets/7af9ef1f-2e72-4eb0-ac61-5e5945badda1" /> <img width="1496" alt="image" src="https://github.com/user-attachments/assets/04da63c6-8139-44ff-b434-4238accf66da" />
neo773 commented 2025-04-19 03:25:25 +08:00 (Migrated from github.com)
Copy Link

@menaguilherme
Updated patch for more fixes

@menaguilherme Updated patch for more fixes
menaguilherme commented 2025-04-19 03:29:56 +08:00 (Migrated from github.com)
Copy Link

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

@neo773 Thank you for taking the time to update the patch — I’ve applied the changes and pushed them!
zachgoll (Migrated from github.com) reviewed 2025-04-19 04:24:12 +08:00
zachgoll (Migrated from github.com) left a comment
Copy Link

Thanks for these fixes, appreciate it!!

We're very much a WIP when it comes to design system, but I've published a little reference here that might help with this PR:

https://github.com/maybe-finance/maybe/wiki/Design-System

There are 2 main areas that I think we need to address (have left some contextual comments throughout):

  • We should try to consolidate all form related styles into our form builder (see doc above)
  • Remove all usage of dark:some-style (see doc above)

Other than those two points this looks good!

Thanks for these fixes, appreciate it!! We're very much a WIP when it comes to design system, but I've published a little reference here that might help with this PR: https://github.com/maybe-finance/maybe/wiki/Design-System There are 2 main areas that I think we need to address (have left some contextual comments throughout): - We should try to consolidate all form related styles into our form builder (see doc above) - Remove all usage of `dark:some-style` (see doc above) Other than those two points this looks good!
app/assets/tailwind/maybe-design-system.css
zachgoll (Migrated from github.com) commented 2025-04-19 04:22:18 +08:00
Copy Link

We'll want to use our Figma tokens (rather than bg-divider-adaptive) here (see doc I posted)

We'll want to use our Figma tokens (rather than `bg-divider-adaptive`) here (see doc I posted)
app/views/accounts/_account_type.html.erb
@@ -1,7 +1,7 @@
<%# locals: (accountable:) %>
zachgoll (Migrated from github.com) commented 2025-04-19 04:23:16 +08:00
Copy Link

We'll want to rely on our centralized maybe-design-system.css to define these hover/focus states in each "mode" to avoid the pseudo selectors dark:* in the views.

We'll want to rely on our centralized `maybe-design-system.css` to define these hover/focus states in each "mode" to avoid the pseudo selectors `dark:*` in the views.
app/views/categories/_form.html.erb
zachgoll (Migrated from github.com) commented 2025-04-19 04:24:09 +08:00
Copy Link

These selects/inputs should already be receiving styles from our centralized form builder, so we can update things there to centralize everything.

These selects/inputs should already be receiving styles from our centralized form builder, so we can update things there to centralize everything.
neo773 (Migrated from github.com) reviewed 2025-04-20 05:14:00 +08:00
app/views/layouts/wizard.html.erb
@@ -1,5 +1,5 @@
<%= render "layouts/shared/htmldoc" do %>
neo773 (Migrated from github.com) commented 2025-04-20 05:14:00 +08:00
Copy Link

this is not the right background color it conflicts with primary button, needs to be pure black

this is not the right background color it conflicts with primary button, needs to be pure black
menaguilherme (Migrated from github.com) reviewed 2025-04-20 05:26:11 +08:00
app/views/layouts/wizard.html.erb
@@ -1,5 +1,5 @@
<%= render "layouts/shared/htmldoc" do %>
menaguilherme (Migrated from github.com) commented 2025-04-20 05:26:11 +08:00
Copy Link

Thanks for pointing that out! I’ll revert that part and bring back the original background to avoid any conflicts. Appreciate the heads up!

Thanks for pointing that out! I’ll revert that part and bring back the original background to avoid any conflicts. Appreciate the heads up!
menaguilherme commented 2025-04-20 05:49:19 +08:00 (Migrated from github.com)
Copy Link

@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!

@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!
zachgoll (Migrated from github.com) reviewed 2025-04-21 22:31:33 +08:00
zachgoll (Migrated from github.com) left a comment
Copy Link

Looking good!

Just a few details to finish things up:

  • I'm still seeing a few class: "..." overrides in a few forms that I think we can remove and just leverage our form builder styles for
  • Address comment within form builder itself

In regards to bg-divider-adaptive, just spoke with @justinfar and he said for "dividers", we should be using those "border" utilities.

In this case, since we have div elements representing dividers, I think the most natural way to implement this is to switch those over to <hr> elements with that border. Does that work?

<hr class="border border-secondary" />
Looking good! Just a few details to finish things up: - I'm still seeing a few `class: "..."` overrides in a few forms that I _think_ we can remove and just leverage our form builder styles for - Address comment within form builder itself In regards to `bg-divider-adaptive`, just spoke with @justinfar and he said for "dividers", we should be using those "border" utilities. In this case, since we have `div` elements representing dividers, I think the most natural way to implement this is to switch those over to `<hr>` elements with that border. Does that work? ```erb <hr class="border border-secondary" /> ```
app/helpers/styled_form_builder.rb
zachgoll (Migrated from github.com) commented 2025-04-21 22:19:54 +08:00
Copy Link

I think these can move up to the metaprogramming block we have here, as both number/date fields are captured there:

a7dfafc907/app/helpers/styled_form_builder.rb (L6-L16)

I think these can move up to the metaprogramming block we have here, as both number/date fields are captured there: https://github.com/maybe-finance/maybe/blob/a7dfafc9072b2986834a0b9fff60e8241b1d82f1/app/helpers/styled_form_builder.rb#L6-L16
app/views/rule/actions/_action.html.erb
zachgoll (Migrated from github.com) commented 2025-04-21 22:22:40 +08:00
Copy Link

I think this needs to be removed? This form should be using styled_form_with. Unless there was a specific reason to override that I'm not seeing?

I _think_ this needs to be removed? This form should be using `styled_form_with`. Unless there was a specific reason to override that I'm not seeing?
app/views/rule/conditions/_condition.html.erb
zachgoll (Migrated from github.com) commented 2025-04-21 22:23:03 +08:00
Copy Link

Similar to comment above, think we can leverage the global form builder styles here?

Similar to comment above, think we can leverage the global form builder styles here?
menaguilherme commented 2025-04-22 00:24:56 +08:00 (Migrated from github.com)
Copy Link

@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!

@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!
zachgoll (Migrated from github.com) reviewed 2025-04-22 02:24:38 +08:00
zachgoll (Migrated from github.com) left a comment
Copy Link

No worries! Just a few more little edits and we should be good.

To get those checks passing locally, you can use the following commands:

bin/rubocop -f github -a
bundle exec erb_lint ./app/**/*.erb -a
bin/brakeman --no-pager
No worries! Just a few more little edits and we should be good. To get those checks passing locally, you can use the following commands: ``` bin/rubocop -f github -a bundle exec erb_lint ./app/**/*.erb -a bin/brakeman --no-pager ```
app/assets/tailwind/maybe-design-system.css
zachgoll (Migrated from github.com) commented 2025-04-22 02:22:25 +08:00
Copy Link

We can delete this and replace all usages with bg-container

We can delete this and replace all usages with `bg-container`
app/helpers/styled_form_builder.rb
zachgoll (Migrated from github.com) commented 2025-04-22 02:14:53 +08:00
Copy Link

Sorry to backtrack on this, but after seeing the final changes I don't think we need to make any changes to this file at all. Let's just make sure form-field__input has all the classes needed in the main design system CSS file and we should be good to go.

That way, whether we're using a form builder or manually passing in form-field__input, we'll get the same styling result.

So in summary:

  • Revert all changes to this file
  • Update main CSS file to ensure it has the text-ellipsis placeholder:text-subdued
Sorry to backtrack on this, but after seeing the final changes I don't think we need to make any changes to this file at all. Let's just make sure `form-field__input` has all the classes needed in the main design system CSS file and we should be good to go. That way, whether we're using a form builder or manually passing in `form-field__input`, we'll get the same styling result. So in summary: - Revert all changes to this file - Update main CSS file to ensure it has the `text-ellipsis placeholder:text-subdued`
app/views/layouts/wizard.html.erb
@@ -1,5 +1,5 @@
<%= render "layouts/shared/htmldoc" do %>
zachgoll (Migrated from github.com) commented 2025-04-22 02:18:57 +08:00
Copy Link
  <div class="bg-container flex flex-col h-dvh pt-safe">
```suggestion <div class="bg-container flex flex-col h-dvh pt-safe"> ```
menaguilherme (Migrated from github.com) reviewed 2025-04-22 05:36:08 +08:00
app/views/layouts/wizard.html.erb
@@ -1,5 +1,5 @@
<%= render "layouts/shared/htmldoc" do %>
menaguilherme (Migrated from github.com) commented 2025-04-22 05:36:07 +08:00
Copy Link

Hey @zachgoll , I applied the bg-container change and it works well overall, but I noticed some contrast issues in dark mode, especially for layered components like this budget setup step.

Here’s a comparison between light and dark mode with the current tokens applied:

📸 Light mode:
Screenshot 2025-04-21 at 18 14 04

🌒 Dark mode:
Screenshot 2025-04-21 at 18 13 58

In dark mode, the progress bar and surrounding sections feel a bit flattened — it’s hard to visually distinguish the layers (e.g., background vs. container vs. progress track), which impacts readability and hierarchy.

I reviewed the tokens in maybe-design-system.css and tried options like bg-surface, bg-overlay, bg-container-inset, etc., but couldn’t find a combination that resolved the contrast issue effectively.

That said, it’s possible I missed a better combination or intended use of those tokens.
Is there a visual reference or Figma spec I could check to understand how these background layers should be applied in cases like this?

Just want to be sure I’m staying within the system and not introducing anything unintended. Appreciate any guidance!

Hey @zachgoll , I applied the bg-container change and it works well overall, but I noticed some contrast issues in dark mode, especially for layered components like this budget setup step. Here’s a comparison between light and dark mode with the current tokens applied: 📸 Light mode: ![Screenshot 2025-04-21 at 18 14 04](https://github.com/user-attachments/assets/8d8f4542-a99d-48d0-b15d-25a1ba0e4f19) 🌒 Dark mode: ![Screenshot 2025-04-21 at 18 13 58](https://github.com/user-attachments/assets/fe7fb694-0703-4443-bb4c-398635c11e9d) In dark mode, the progress bar and surrounding sections feel a bit flattened — it’s hard to visually distinguish the layers (e.g., background vs. container vs. progress track), which impacts readability and hierarchy. I reviewed the tokens in [maybe-design-system.css](https://github.com/maybe-finance/maybe/blob/main/app/assets/tailwind/maybe-design-system.css) and tried options like bg-surface, bg-overlay, bg-container-inset, etc., but couldn’t find a combination that resolved the contrast issue effectively. That said, it’s possible I missed a better combination or intended use of those tokens. Is there a visual reference or Figma spec I could check to understand how these background layers should be applied in cases like this? Just want to be sure I’m staying within the system and not introducing anything unintended. Appreciate any guidance!
menaguilherme commented 2025-04-23 07:45:01 +08:00 (Migrated from github.com)
Copy Link

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 🙏

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 🙏
zachgoll (Migrated from github.com) approved these changes 2025-04-23 21:42:20 +08:00
zachgoll (Migrated from github.com) left a comment
Copy Link

Looks great. Thanks for your work on this!

Looks great. Thanks for your work on this!
Sign in to join this conversation.
Reviewers
No Reviewers
neo773
menaguilherme
zachgoll
Labels
Clear labels
📆 Planned
Not ready to be worked on yet, but planned for future
 💻 Self Hosted only
Issues pertaining to self-hosted versions of Maybe
 ⚙️ Config
ℹ️ Needs info
Further information is requested
 📱 Mobile
Bugs exclusively happening on mobile devices
 1️⃣ High Priority
High priority issues will be worked on next
 🚨 Urgent
Issues that are urgent for app functionality
 3️⃣ Low priority
Contributions accepted, but Maybe team will not be working on this in the near term
 2️⃣ Medium Priority
Community contributions accepted, Maybe team only works on if there are no high priority items open
 ⚡ Performance
Performance issues to address
 🎨 UI / UX
Issues related to UI / UX interactions and design.
 💎 Bounty
💰 Rewarded
📊 Data issue
Issues where the app functionally works, but the data is missing or inaccurate.
 🙋 Bounty claim
$1.5K
Community
This is a great issue for community members to work on
 dependencies
Pull requests that update a dependency file
 Design
Hosted only
Hosted app only
 Required self hoster data migration
Self hosters must run data migrations to resolve this issue
 ruby
Pull requests that update Ruby code
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
gavin
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gavin/maybe#2125
Delete Branch "fix/dark-mode-contrast"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?