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: Implement dark theme #95

Open
chrisbobbe opened this issue May 18, 2023 · 8 comments
Open

ui: Implement dark theme #95

chrisbobbe opened this issue May 18, 2023 · 8 comments
Assignees
Labels
a-design Visual and UX design

Comments

@chrisbobbe
Copy link
Collaborator

The user should be able to choose from these three options for the app's appearance:

  • Light
  • Dark
  • Follow OS

Flutter's Material Design implementation paves the way to a pretty good 1.0 version of a dark theme, for the boring, simple bits of UI we paint with Theme.of(context).colorScheme. For example, here's the result of just this simple change:

diff --git lib/widgets/app.dart lib/widgets/app.dart
index 533d3df8d..2eec13d40 100644
--- lib/widgets/app.dart
+++ lib/widgets/app.dart
@@ -20,7 +20,7 @@ class ZulipApp extends StatelessWidget {
       //   https://api.flutter.dev/flutter/material/ColorScheme/ColorScheme.fromSeed.html
       // Or try this tool to see the whole palette:
       //   https://m3.material.io/theme-builder#/custom
-      colorScheme: ColorScheme.fromSeed(seedColor: kZulipBrandColor));
+      colorScheme: ColorScheme.fromSeed(seedColor: kZulipBrandColor, brightness: Brightness.dark));
     return GlobalStoreWidget(
       child: MaterialApp(
         title: 'Zulip',
Before After
image image

For the message list, though, we don't draw from Theme.of(context).colorScheme, so it doesn't respond to setting Brightness.dark there:

Before After
image image

Probably we should bundle all the message list's light-theme colors into a palette called messageListColors or something, then create a version of that palette for dark theme.

@gnprice
Copy link
Member

gnprice commented May 19, 2023

If we do this before we have local settings #97, we can have it just follow the system theme — that's what should be the default anyway, so it may as well become the only setting in the alpha. If we already have #97 when we implement this, though, might as well offer the setting from the beginning.

@gnprice gnprice added the m-beta label May 26, 2023
@gnprice gnprice added this to the Beta milestone May 27, 2023
@gnprice gnprice removed the m-beta label May 27, 2023
@gnprice gnprice added the a-design Visual and UX design label Jun 14, 2023
gnprice added a commit that referenced this issue Jun 14, 2023
Filed #187, #188, and #190.

Other items already existed as #80, #82, #95, #122, and #123,
or were already complete (a compose box, and three ways of
attaching something to a message.)
@gnprice gnprice mentioned this issue Oct 30, 2023
@gnprice gnprice modified the milestones: Beta 1, Beta 2 Nov 8, 2023
@gnprice gnprice modified the milestones: Beta 2, Beta 3 Nov 22, 2023
@neeldoshii

This comment was marked as off-topic.

@gnprice

This comment was marked as off-topic.

@gnprice
Copy link
Member

gnprice commented Mar 22, 2024

An implementation note:

For all the colors that we have that don't correspond to anything in particular in the Material Design ColorScheme, we can put them on a ThemeExtension.

That will let us neatly bundle together a number of colors for a light theme, and corresponding colors for a dark theme, and switch between themes centrally — and even transition smoothly between them when the theme changes, which e.g. the user might have chosen in system settings to have happen on a daily cycle. In short it means we'll use the same nice mechanics that Flutter's Material implementation uses, but for areas that needn't have any design elements in common with Material.

@SharmaDhruv2511

This comment was marked as off-topic.

@chrisbobbe

This comment was marked as off-topic.

@Syrineladeb02

This comment was marked as off-topic.

@gnprice
Copy link
Member

gnprice commented Apr 1, 2024

@Syrineladeb02 Please see our guide to picking an issue to work on:
https://github.com/zulip/zulip-flutter#picking-an-issue-to-work-on

chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 4, 2024
We added the light-mode color computations in zulip#381.

We're not ready to use this yet, but the color computations are
unlikely to change before that time comes, and finding the right
ones is a chore that's good to get done.

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 4, 2024
We added the light-mode color computations in zulip#381.

We're not ready to use this yet, but the color computations are
unlikely to change before that time comes, and finding the right
ones is a chore that's good to get done.

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 9, 2024
We added the light-mode color computations in zulip#381.

We're not ready to use this yet, but the color computations are
unlikely to change before that time comes, and finding the right
ones is a chore that's good to get done.

Related: zulip#95
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue May 10, 2024
We added the light-mode color computations in zulip#381.

We're not ready to use this yet, but the color computations are
unlikely to change before that time comes, and finding the right
ones is a chore that's good to get done.

Related: zulip#95
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue May 16, 2024
It should now be easy to drop in natural dependencies of our content
widgets, such as a ThemeExtension for custom light- and dark-theme
colors, for all the tests of our Zulip content widgets. To do that,
we can just add unconditional code to this function.

After this change, some ad-hoc `tester.pumpWidget` calls do remain
in this file; see tests of RealmContentNetworkImage and AvatarImage.
But those aren't "Zulip content widgets" in the same sense as most
of the widgets in lib/widgets/content.dart, which are used
exclusively to render parsed Zulip Markdown. (There isn't even any
Zulip Markdown that would produce an AvatarImage.) So, leave them
be. Perhaps these widgets belong in some other file.

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 18, 2024
This gives a path forward for dark theme (zulip#95), even with the open
question we commented about in the previous commit.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 18, 2024
This gives a path forward for dark theme (zulip#95), even with the open
question we commented about in the previous commit.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 18, 2024
…sign

From discussion:
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20colors/near/1801938

and the Figma linked there:
  https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2074%3A26325&t=rjuqcQaHMiGBUReF-1

Now, the app-bar bottom border is colored by the new Figma's
`border-bar`, and the main background color is `bg-main`. The app
bar background is unchanged because it already matches `bg-top-bar`.

And all these color variables have dark-theme variants! (See
discussion for what those are.) That'll help when we implement dark
theme, zulip#95.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
Related: zulip#95
In these places, I was unable to find a specific dark-theme color by
consulting web or the Figma (as it is now).
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
In these places, I was unable to find a specific dark-theme color by
consulting web or the Figma (as it is now).

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
In these places, I was unable to find a specific dark-theme color by
consulting web or the Figma (as it is now).

Related: zulip#95
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
…sign

From discussion:
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20colors/near/1801938

and the Figma linked there:
  https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2074%3A26325&t=rjuqcQaHMiGBUReF-1

Now, the app-bar bottom border is colored by the new Figma's
`border-bar`, and the main background color is `bg-main`. The app
bar background is unchanged because it already matches `bg-top-bar`.
I've labeled these values with comments in the code.

And all these color variables have dark-theme variants! (See
discussion for what those are.) That'll help when we implement dark
theme, zulip#95.
gnprice pushed a commit to chrisbobbe/zulip-flutter that referenced this issue May 20, 2024
…sign

From discussion:
  https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/design.3A.20colors/near/1801938

and the Figma linked there:
  https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2074%3A26325&t=rjuqcQaHMiGBUReF-1

Now, the app-bar bottom border is colored by the new Figma's
`border-bar`, and the main background color is `bg-main`. The app
bar background is unchanged because it already matches `bg-top-bar`.
I've labeled these values with comments in the code.

And all these color variables have dark-theme variants! (See
discussion for what those are.) That'll help when we implement dark
theme, zulip#95.
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 6, 2024
With the code-block span styles all bundled together in a new
CodeBlockTextStyles class, it'll be convenient to make separate
`light` and `dark` constructors toward zulip#95 dark theme.

Next, we'll pass a BuildContext to the CodeBlockTextStyles
constructor, in order to use weightVariableTextStyle for the spans
that are supposed to be bold. (The code-block font, Source Code Pro,
is variable-weight.) This would have been possible before the
current refactor, of course, but this way the spans don't have to
repeat the weightVariableTextStyle computations on-demand. They can
instead just look up the result from this central source.

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 12, 2024
With the code-block span styles all bundled together in a new
CodeBlockTextStyles class, it'll be convenient to make separate
`light` and `dark` constructors toward zulip#95 dark theme.

Next, we'll pass a BuildContext to the CodeBlockTextStyles
constructor, in order to use weightVariableTextStyle for the spans
that are supposed to be bold. (The code-block font, Source Code Pro,
is variable-weight.) This would have been possible before the
current refactor, of course, but this way the spans don't have to
repeat the weightVariableTextStyle computations on-demand. They can
instead just look up the result from this central source.

Related: zulip#95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 17, 2024
With the code-block span styles all bundled together in a new
CodeBlockTextStyles class, it'll be convenient to make separate
`light` and `dark` constructors toward zulip#95 dark theme.

Next, we'll pass a BuildContext to the CodeBlockTextStyles
constructor, in order to use weightVariableTextStyle for the spans
that are supposed to be bold. (The code-block font, Source Code Pro,
is variable-weight.) This would have been possible before the
current refactor, of course, but this way the spans don't have to
repeat the weightVariableTextStyle computations on-demand. They can
instead just look up the result from this central source.

Related: zulip#95
chrisbobbe added a commit that referenced this issue Jun 17, 2024
With the code-block span styles all bundled together in a new
CodeBlockTextStyles class, it'll be convenient to make separate
`light` and `dark` constructors toward #95 dark theme.

Next, we'll pass a BuildContext to the CodeBlockTextStyles
constructor, in order to use weightVariableTextStyle for the spans
that are supposed to be bold. (The code-block font, Source Code Pro,
is variable-weight.) This would have been possible before the
current refactor, of course, but this way the spans don't have to
repeat the weightVariableTextStyle computations on-demand. They can
instead just look up the result from this central source.

Related: #95
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 18, 2024
We'll want to get these colors from theme data, soon, for zulip#95
(supporting dark theme).
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 18, 2024
Instead of putting them in Subscription model objects.

This helps toward zulip#95 by bundling this with our other
ThemeExtensions that will switch together between light and dark
variants when the theme changes. It's also just nicer to separate
this very UI-focused code out of api/model/ (zulip#393).

Not marking [nfc] just because of differences in caching logic,
although I don't expect a user-visible perf effect.

Related: zulip#95
Fixes: zulip#393
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Jun 18, 2024
Instead of putting them in Subscription model objects.

This helps toward zulip#95 by bundling this with our other
ThemeExtensions that will switch together between light and dark
variants when the theme changes. It's also just nicer to separate
this very UI-focused code out of api/model/ (zulip#393).

Not marking [nfc] just because of differences in caching logic,
although I don't expect a user-visible perf effect.

Related: zulip#95
Fixes: zulip#393
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-design Visual and UX design
Projects
Status: No status
Development

No branches or pull requests

5 participants