fix: Question panel dark mode color #1249
Conversation
|
Actually @GowthamGoush, what does it look like in darkmode |
| @@ -464,7 +464,7 @@ class _SummaryCardState extends State<SummaryCard> { | |||
| }, | |||
| child: SmoothCard( | |||
| margin: EdgeInsets.zero, | |||
| color: const Color(0xfff5f6fa), | |||
| color: Theme.of(context).primaryColor, | |||
There was a problem hiding this comment.
I think we rather use Theme.of(context).colorScheme.primary in the rest of the app, could you double-check that?
There was a problem hiding this comment.
Sure, let me fix that
There was a problem hiding this comment.
Using color scheme produces something like below:
If this color suits well, then definitely we can switch to Theme.of(context).colorScheme.primary
There was a problem hiding this comment.
@GowthamGoush I have no opinion regarding colors, only about code: I don't like the possible confusion. If I have a look at the current code I only see colorScheme.primary and never Theme.of(context).primaryColor. So let's definitely use colorScheme.primary (and use its value if necessary).
There was a problem hiding this comment.
Yes, I definitely agree with whatever you mentioned regarding the standards that we follow. I will fix that soon. Also, thanks for the kind suggestion and feedback.
There was a problem hiding this comment.
@M123-dev Then please decide a correct primary color and a correct secondary color. That's what they're here for.
There was a problem hiding this comment.
Currently, I have hardcoded these colors based on the brightness of the app but I would like to receive feedback from you regarding this and change it accordingly if needed.
I have attached the screenshots of the results for this:
Theme.of(context).colorScheme.brightness == Brightness.dark ? const Color(0xff212121) : const Color(0xfff5f6fa)
Light mode
Dark mode
There was a problem hiding this comment.
@GowthamGoush I think the color looks much better this way, we have several hardcoded values for colors in the app, but it would still probably be better to make them somehow centralized
There was a problem hiding this comment.
Looking at #1274, it looks like the color should be good by just removing the hardcoded value
Oops.. let me add a screenshot of that. |
|
Fixed in #1323, still thanks for your effort @GowthamGoush |
What
Impacted files
summary_card.dartScreenshot
Before
After
Fixes bug
Part of