-
Notifications
You must be signed in to change notification settings - Fork 984
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
Update navigation bar to support dark mode #16762
Conversation
Jenkins BuildsClick to see older builds (12)
|
{:position :absolute | ||
:top 0 | ||
:left 0 | ||
:right 0 | ||
:height navigation-bar-height | ||
:background-color (colors/theme-colors colors/white-opa-70 :transparent) | ||
:background-color (colors/theme-colors colors/white-opa-70 colors/neutral-100-opa-70) |
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.
while you're making changes here can you update this component to use quo2.theme
with-theme HOC and then pass in theme
as a 3rd param. e,g
(defn background-view [theme]
{:position :absolute
:top 0
:left 0
:right 0
:height navigation-bar-height
:background-color (colors/theme-colors colors/white-opa-70 colors/neutral-100-opa-70 theme)
44% of end-end tests have passed
Not executed tests (3)Failed tests (20)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Passed tests (16)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
@J-Son89 done! |
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.
Nice @alwx! 😀
78% of end-end tests have passed
Not executed tests (3)Failed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Passed tests (28)Click to expandClass TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
|
hi, @Francesca-G could you check if the top bar is suitable on the dark mode? No issues from my side |
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.
@VolodLytvynenko the blur doesn't look right (see screenshots).
I'm aware that the blur on Android is a known issue but I tested it too.
Hope this helps!
@Francesca-G. That's interesting. The bar looks different for you. This is how it looks for me: It seems that our builds are different. Could you please check if the previous PR build was removed to avoid the current last PR build installation over the previous one? If it does not help, could you provide the model of devices you use? |
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.
@VolodLytvynenko thanks for helping me fix the builds, it was a problem on my side!
Here's the Figma frame with the design review
@Francesca-G thank you for the feedback! Hi @alwx |
* Dark mode top bar * Fixes * Style fix
Makes nav bar look properly with dark mode.
Platforms
Areas that maybe impacted
Functional
status: ready