-
Notifications
You must be signed in to change notification settings - Fork 357
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
Add color tokens and use some of them in our theme #5211
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.
Reviewed 26 of 26 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
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.
Reviewable status: complete! all files reviewed, all discussions resolved
b20f96c
to
88ccd3d
Compare
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.
Reviewed 7 of 26 files at r1, 11 of 20 files at r2, 9 of 9 files at r3, all commit messages.
Reviewable status: 28 of 29 files reviewed, all discussions resolved (waiting on @Rawa)
e18740b
to
78e9c42
Compare
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.
Reviewed 1 of 20 files at r2, 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
a3369dc
to
9044a3e
Compare
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.
Reviewed 2 of 2 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/Scaffolding.kt
line 88 at r6 (raw file):
val canScroll = lazyListState.canScrollForward || lazyListState.canScrollBackward val scrollBehavior = TopAppBarDefaults.exitUntilCollapsedScrollBehavior(appBarState, canScroll = { canScroll })
Weird that this got added to this commit? I guess because rebase is needed? 😕
android/lib/theme/src/main/kotlin/net/mullvad/mullvadvpn/lib/theme/color/ColorTokens.kt
line 56 at r6 (raw file):
internal val md_theme_dark_onSurface = Color(0xFFFFFFFF) // MullvadWhite internal val md_theme_dark_surfaceVariant = Color(0xFFFFFFFF) // MullvadWhite internal val md_theme_dark_onSurfaceVariant = Color(0xFFA3ACB5) // Subtext
If SurfaceVariant is #FFFFFF then onSurfaceVariant #A3ACB5 seems to be too bright? Seems like we according to Zeplin use the PrimaryBlue or something like that?
https://app.zeplin.io/project/5f928a32fdc9962af9018d70/screen/6037df6562fa815ad838d9f7
6581917
to
77db52a
Compare
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.
Reviewed 7 of 7 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
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.
Reviewable status: complete! all files reviewed, all discussions resolved
8ebbe1d
to
3e5de1e
Compare
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.
Reviewed 5 of 5 files at r8, 4 of 7 files at r9, 5 of 5 files at r10, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
3e5de1e
to
c03802a
Compare
Also switch to dark theme as base theme
c03802a
to
88ad2c9
Compare
Also switch to dark theme as base theme
This is part of the design system overhaul.
This change is