Skip to content
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: Make some dark-theme adjustments from Greg's feedback #955

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chrisbobbe
Copy link
Collaborator

This addresses all the remaining items in the CZO thread at https://chat.zulip.org/#narrow/stream/48-mobile/topic/dark.20theme/near/1936853 .

Screenshots coming soon.

This seems like the preferred way to use the API.
An improvement that Greg pointed out:
  https://chat.zulip.org/#narrow/stream/48-mobile/topic/dark.20theme/near/1936853

This makes them the same color as the third-party login buttons, in
7c9c894.
Thanks to Greg for the reminder that this needed attention:
  https://chat.zulip.org/#narrow/stream/48-mobile/topic/dark.20theme/near/1936853

This is pretty low-contrast in dark mode, but as with many of our UI
elements, we'll adjust as necessary when we have a design from Vlad.
@chrisbobbe chrisbobbe added the integration review Added by maintainers when PR may be ready for integration label Sep 19, 2024
@chrisbobbe
Copy link
Collaborator Author

chrisbobbe commented Sep 19, 2024

Home page (light theme affected too because it was convenient to just plug in a light/dark pair of colors that comes from Material):

Before After
image image
image image

Error style (light theme unchanged):

Before After
image image

Dot for muted unreads (light theme unchanged):

The way this looks, I think it might not be an improvement, actually. The color matches what gets used on web, but it ends up with pretty low contrast because the background color differs between mobile and web. But also, like many UI elements that currently follow web, this is something we'll revisit once we have a design from Vlad.

Before After
image image

@chrisbobbe
Copy link
Collaborator Author

cc @alya in case you'd like to take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants