Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Ensure profile panel text and button visibility in dark mode #1662

Merged
merged 5 commits into from
Feb 9, 2025
Merged

Fix: Ensure profile panel text and button visibility in dark mode #1662

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7835b85
Fix: Ensure profile panel text and button visibility in dark mode
Br0wnHammer Jan 29, 2025
aac1d17
Merge remote-tracking branch 'checkmate/develop' into fix/fe/profilep…
Br0wnHammer Jan 30, 2025
959e555
Fix: Enhance MuiButton disabled state styles in theme
Br0wnHammer Jan 30, 2025
1d292b9
Merge remote-tracking branch 'checkmate/develop' into fix/fe/profilep…
Br0wnHammer Feb 6, 2025
263427d
Fix: Disabled button color theme
Br0wnHammer Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Client/src/Components/Inputs/Image/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const ImageField = ({ id, src, loading, onChange }) => {
<Typography
component="span"
fontSize="inherit"
color={theme.palette.primary.main}
color="info"
fontWeight={500}
>
Click to upload
Expand Down
4 changes: 4 additions & 0 deletions Client/src/Utils/Theme/globalTheme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ const baseTheme = (palette) => ({
"&:hover": {
boxShadow: "none",
},
"&.Mui-disabled": {
backgroundColor: theme.palette.secondary.main,
color: theme.palette.primary.main,
},
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Yo dawg, we need to handle this style conflict!

There's a potential style collision between the new disabled state and the existing loading button disabled state. The loading button uses different colour tokens:

  • New disabled state: primary.main for text
  • Loading button disabled: primary.contrastText for text

This might cause inconsistent appearances when buttons are both disabled and loading.

Let's fix this spaghetti with this diff:

     "&.Mui-disabled": {
       backgroundColor: theme.palette.secondary.main,
-      color: theme.palette.primary.main,
+      color: theme.palette.primary.contrastText,
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"&.Mui-disabled": {
backgroundColor: theme.palette.secondary.main,
color: theme.palette.primary.main,
},
"&.Mui-disabled": {
backgroundColor: theme.palette.secondary.main,
color: theme.palette.primary.contrastText,
},

💡 Codebase verification

Yo dawg, we got a contrast catastrophe! 🚨

The disabled button's dark mode colors are straight-up invisible, fam! We're talking:

  • Background: #313131 (secondary.main)
  • Text: #131315 (primary.main)

These colors are so close they're practically twins - users won't be able to read that text! We need to bump up that contrast ratio for accessibility. Consider using a lighter shade for either the text or background.

🔗 Analysis chain

Verify these colours pop in dark mode, fam!

Since this change is specifically for dark mode visibility, we need to ensure these theme tokens provide sufficient contrast.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for theme token definitions to verify contrast ratios
rg -A 5 'palette.*main|palette.*contrastText' --type js

Length of output: 67132

Script:

#!/bin/bash
# Search for theme/palette color definitions
rg -A 10 'createTheme|palette.*dark|darkTheme' --type js

Length of output: 3806

Script:

#!/bin/bash
# Search for semantic colors and extraction logic
rg -A 15 'newSemanticColors.*=|extractThemeColors.*=|const.*colors.*=' --type js Client/src/Utils/Theme/

Length of output: 1101

Script:

#!/bin/bash
# Search for complete color definitions with more context
rg -A 50 'const newColors|const newSemanticColors' --type js Client/src/Utils/Theme/

# Also search for color utility functions
rg -A 10 'function (lighten|darken)' --type js Client/src/Utils/Theme/

Length of output: 5318

"&.MuiLoadingButton-root": {
"&:disabled": {
backgroundColor: theme.palette.secondary.main,
Expand Down