fixed Darkmode-Lightmode switching issue #6525
Conversation
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
|
🚀 Preview for commit 9892cea at: https://683fac9decdd6d7e89e1a9ba--layer5.netlify.app |
|
some screen shots or recording will be helpful to review it @ashok-2003 |
|
sure @vr-varad , Screen.Recording.2025-06-04.131541.mp4 |
| @@ -215,7 +199,8 @@ const ViewsSectionWrapper = styled.div` | |||
| height:150px; | |||
| padding: 2rem; | |||
| box-sizing: border-box; | |||
| background-color: ${(props) => props.theme.backgroundColor}; | |||
| background-color: ${(props) => props.theme.grey191919ToGreyF2F5F7}; | |||
There was a problem hiding this comment.
@ashok-2003 we want to refrain from using props or tokens other than the ones mentioned in this palette
There was a problem hiding this comment.
Apologies, I just realised that these components are not using Sistent.
Can you use the isDark function? and toggle the colors accordingly? The backgroundColor token works fine, I believe. It's the text which we have to keep track of.
There was a problem hiding this comment.
Ah, sorry — I think the SVG was supposed to merge with the background to make it look seamless. Thanks for pointing that out. And yes, the main issue was with the text, which I’ve already addressed. I’ll go ahead and revert the background color changes.
Screen.Recording.2025-06-04.215805.mp4
| overflow: hidden; | ||
| height: 100%; | ||
| .hero-text { | ||
| display: flex; |
There was a problem hiding this comment.
@ashok-2003 it would be nice if you keep the indentation fixes in a commit towards the end, that way it's easier to track changes .
There was a problem hiding this comment.
Got it, thanks for the suggestion! I’ll make sure to keep indentation or formatting fixes in a separate commit towards the end — makes total sense for cleaner reviews. 🙌
|
If there are formatting issues, It would be nice to address them, as for the backgrounds |
@LibenHailu yes, I don't think that |
In the deployed version check it out it looks close. |
|
@LibenHailu Yeah, I later realized that a contrasting background might actually work better. Initially, I was aiming for a more blended look, but I’ll revert it back to the original for better clarity. @M-DEV-1 Just to clarify, |
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
|
🚀 Preview for commit bd385c8 at: https://68407bd8d2d571565f5b4084--layer5.netlify.app |
|
@ashok-2003 will you please be reverting any formatting, white spaces as other reviews have noted as well? |
|
@ashok-2003 Add it as an agenda item to the meeting minutes, if you would :) |
|
🚀 Preview for commit ec02150 at: https://684662cf9928c8757983814e--layer5.netlify.app |
|
@vishalvivekm, thank you for your review. Reverting those formatting changes may likely cause issues, as discussed in the meetings. Are these the required changes, or is there anything more I’m expected to do? |
|
Hi @ashok-2003, as discussed on the sites meeting let's fix format issues if there are any, you can use |
Signed-off-by: Ashok Kumar <ashokgupta2678@gmail.com>
…layer5 into fix-dark-light-issue
| @@ -106,7 +106,7 @@ const ViewsSectionWrapper = styled.div` | |||
| /* max-width: 90%; */ | |||
| padding-bottom: 2%; | |||
| } | |||
| } | |||
There was a problem hiding this comment.
This was causing the issue where the Card component wasn't rendering the color according to the background.
|
@LibenHailu, thank you for the suggestion. Unfortunately, npm run lint doesn’t work for me. After a lot of head-banging, I’ve found the main and minimal changes that are required. Are we good to go? @M-DEV-1, @vishalvivekm |
@ashok-2003 Please feel free to share the error logs on slack with me if the issue persists. I am not seeing the deployment link tho. |
Description
This PR fixes #6484 (Card Text Labels Become Invisible When Switching Between Dark and Light Themes)
Notes for Reviewers
Signed commits