-
Notifications
You must be signed in to change notification settings - Fork 441
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(App): switch toggle button to dark theme in call #11040
Conversation
/backport to stable28 |
src/App.vue
Outdated
@@ -750,6 +750,7 @@ export default { | |||
:deep(.app-navigation-toggle) { | |||
/* Force white handle when inside a call */ | |||
color: #D8D8D8; | |||
background-color: transparent !important; |
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.
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.
After clearing overwritten styles:
Screencast.from.30.11.2023.12.56.38.webm
Dark theme:
Screencast.from.30.11.2023.12.58.00.webm
Not sure if we need to apply dark-theme here either
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.
Not sure if we need to apply dark-theme here either
We manually change all buttons in the call view to dark theme because the call view is always dark. Having it white seems very bright and eye-catching. Why not always use a dark theme here?
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.
cc @marcoambrosini for a design change review
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.
Also agree that we should keep the buttons dark themed in the call view. It should look no different from other buttons in terms of styling.
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.
But other buttons are not dark-themed... they are transparent with semi-transparent hover effect, which won't fit for the toggle overlapping them 🤕
I could apply a dark-theme colors here, but it still stands out for me
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.
I think Maksim is right, using dark theme is not the common style in call view so it looks peculiar from other buttons.
Or is there a reason to make it stand out intentionally ?
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.
But the light theme looks even more eye-catching on a dark background around other dark buttons.
I'm fine with some other colors, but the transparent button overlaps on the mobile view.
b90455a
to
cbab802
Compare
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
cbab802
to
da6202d
Compare
As background in call could be changed to a solid color (ref #11005), I'd go with basic dark-theme styles for now and merge Then we could adjust the button color to mach the theme color (something similar to |
☑️ Resolves
🖌️ UI Checklist
🖼️ Screenshots / Screencasts
Screencast.from.04.12.2023.11.18.58.webm
🏁 Checklist