-
Notifications
You must be signed in to change notification settings - Fork 109
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.
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
ui: Adjust app bar bg and scaffold bg colors toward new Figma design #687
ui: Adjust app bar bg and scaffold bg colors toward new Figma design #687
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Small comment below.
lib/widgets/theme.dart
Outdated
@@ -14,7 +14,10 @@ ThemeData zulipThemeData(BuildContext context) { | |||
scrolledUnderElevation: 0, | |||
backgroundColor: Color(0xfff5f5f5), | |||
|
|||
shape: Border(bottom: BorderSide(color: Color(0xffcccccc))), | |||
shape: Border(bottom: BorderSide( | |||
color: Color(0x33000000), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, the app-bar bottom border is colored by the new Figma's
`border-bar`, and the main background color is `bg-main`. The app
bar background is unchanged because it already matches `bg-top-bar`.
This feels like the sort of information that's helpful to have in the code itself, so as a comment something like:
color: Color(0x33000000), | |
color: Color(0x33000000), // `border-bar` in Figma |
As part of #95 I think you'll be turning these into some sort of named reference, so that this would be foo.borderBarColor
for some foo
. That'll be another way of putting this information into the code, which is better than a comment. But until then a comment seems helpful.
Thanks! Revision pushed. |
eb29d0b
to
00c031a
Compare
…sign From discussion: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20colors/near/1801938 and the Figma linked there: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2074%3A26325&t=rjuqcQaHMiGBUReF-1 Now, the app-bar bottom border is colored by the new Figma's `border-bar`, and the main background color is `bg-main`. The app bar background is unchanged because it already matches `bg-top-bar`. I've labeled these values with comments in the code. And all these color variables have dark-theme variants! (See discussion for what those are.) That'll help when we implement dark theme, zulip#95.
Thanks! Looks good; merging. |
00c031a
to
10d0fe8
Compare
(This replaces #682.)
From discussion:
https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20colors/near/1801938
and the Figma linked there:
https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2074%3A26325&t=rjuqcQaHMiGBUReF-1
Now, the app-bar bottom border is colored by the new Figma's
border-bar
, and the main background color isbg-main
. The app bar background is unchanged because it already matchesbg-top-bar
.And all these color variables have dark-theme variants! (See discussion for what those are.) That'll help when we implement dark theme, #95.