-
Notifications
You must be signed in to change notification settings - Fork 985
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
Fix incorrect background color in Community Overview screen #16348
Fix incorrect background color in Community Overview screen #16348
Conversation
:before (when label icon) | ||
:size 32 | ||
:override-theme icon-override-theme | ||
:override-background-color icon-background-color} |
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.
FYI: I used the exact same name icon-background-color
that's used for the left-section-view
component in this file. The prop was essentially missing.
@@ -234,7 +235,9 @@ | |||
:ellipsize-mode :tail | |||
:weight :semi-bold | |||
:size :heading-1 | |||
:style {:margin-top 56}} | |||
:style {:margin-top (+ scroll-page.style/picture-radius |
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.
FYI: I parameterized certain style values. I think this practice of magical values is one of the sources of constant bugs in the UI. The question I ask myself to decide if we need vars is: if somebody changes the style value, will they remember to change this magical number? If the answer is no, then ideally, vars/constants should be used.
Jenkins Builds
|
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 @ilmotta!
18% of end-end tests have passed
Failed tests (27)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (6)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
|
0ac069a
to
db3aaf0
Compare
Fixes the main issue #16313, but also other correlated bugs. - Fix: quo2 component navigation.page-nav does not stretch out to the maximum available height when there are only 1 to ~6 community channels. - Fix: options menu button didn't use the correct background color (it should have 40% transparency). - Fix: Remove bottom-left & bottom-right rounded borders from the bottom of the page-nav container. - Fix: Remove top-left & top-right rounded borders from the screen's header. - Fix: Use correct background color in the page-nav, now neutral-95 instead of neutral-90.
Fixes #16313
Summary
In this PR I'm fixing the main issue #16313, but also other correlated bugs (except for the top right button background which I couldn't resist fixing right here). This screen has even more UI bugs, but we'll fix them in other PRs.
navigation.page-nav
does not stretch out to the maximum available height when there are only 1 to ~6 community channels.page-nav
container.page-nav
, nowneutral-95
instead ofneutral-90
.Steps to test
Suggestions:
status: ready