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.

Sign up for GitHub

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

Compose box redesign #928

Merged
merged 8 commits into from
Nov 16, 2024
Merged

Compose box redesign #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8972754
color [nfc]: Add ColorExtension.withFadedAlpha
PIG208 Nov 12, 2024
026c048
emoji: Use withFadedAlpha for highlightColor
PIG208 Nov 12, 2024
56b7a19
theme: Update borderBar to follow change in Figma design
PIG208 Oct 11, 2024
58debf5
compose [nfc]: Refactor away shared iconTheme override
PIG208 Oct 23, 2024
88b2ff9
compose [nfc]: Extract compose buttons as a list
PIG208 Nov 15, 2024
5956528
compose: Use new icons from the redesign
PIG208 Sep 4, 2024
3adb395
compose: Implement most of compose box redesign
PIG208 Oct 10, 2024
94cf40e
compose: Support redesigned button feedback
PIG208 Oct 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified assets/icons/ZulipIcons.ttf
View file
Open in desktop
Binary file not shown.
3 changes: 3 additions & 0 deletions assets/icons/attach_file.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/icons/camera.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions assets/icons/image.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions assets/icons/send.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 5 additions & 4 deletions lib/widgets/action_sheet.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import '../model/internal_link.dart';
import '../model/narrow.dart';
import 'actions.dart';
import 'clipboard.dart';
import 'color.dart';
import 'dialog.dart';
import 'icons.dart';
import 'inset_shadow.dart';
Expand Down Expand Up @@ -145,8 +146,8 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget {
foregroundColor: designVariables.contextMenuItemText,
splashFactory: NoSplash.splashFactory,
).copyWith(backgroundColor: WidgetStateColor.resolveWith((states) =>
designVariables.contextMenuItemBg.withValues(
alpha: states.contains(WidgetState.pressed) ? 0.20 : 0.12))),
designVariables.contextMenuItemBg.withFadedAlpha(
states.contains(WidgetState.pressed) ? 0.20 : 0.12))),
onPressed: () => _handlePressed(context),
child: Text(label(zulipLocalizations),
style: const TextStyle(fontSize: 20, height: 24 / 20)
Expand All @@ -168,8 +169,8 @@ class MessageActionSheetCancelButton extends StatelessWidget {
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
splashFactory: NoSplash.splashFactory,
).copyWith(backgroundColor: WidgetStateColor.resolveWith((states) =>
designVariables.contextMenuCancelBg.withValues(
alpha: states.contains(WidgetState.pressed) ? 0.20 : 0.15))),
designVariables.contextMenuCancelBg.withFadedAlpha(
states.contains(WidgetState.pressed) ? 0.20 : 0.15))),
onPressed: () {
Navigator.pop(context);
},
Expand Down
17 changes: 17 additions & 0 deletions lib/widgets/color.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,21 @@ extension ColorExtension on Color {
((g * 255.0).round() & 0xff) << 8 |
((b * 255.0).round() & 0xff) << 0;
}

/// Makes a copy of this color with [a] multiplied by `factor`.
///
/// `factor` must be between 0 and 1, inclusive.
///
/// To fade a color variable from [DesignVariables], [ContentTheme], etc.,
/// use this instead of calling [withValues] with `factor` passed as `alpha`,
/// which simply replaces the color's [a] instead of multiplying by it.
/// Using [withValues] gives the same result for an opaque color,
/// but a wrong result for a semi-transparent color,
/// and we want our color variables to be free to change
/// without breaking things.
Color withFadedAlpha(double factor) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

color [nfc]: Add ColorExtension.withFadedAlpha

We use this instead at most places where we preivously have
`.withValues(alpha:` on color variables, except the one in
`lib/widgets/emoji_reaction.dart` that is semi-transparent,
to keep this change NFC.

This also skips the ones in `lib/widgets/channel_colors.dart`,
and the one in `lib/widgets/lightbox.dart`
(`Colors.grey.shade900.withValues(alpha: 0.87)`), as determining if
those can be swapped out with `withFadedAlpha` seems out of scope as of
now.

Signed-off-by: Zixuan James Li <[email protected]>

Commit message nits:

In the first paragraph, I'm not sure what "instead" refers to. How about: "And use it everywhere we were using .withValues(alpha: to fade a color variable, except for one place in lib/widgets/emoji_reaction.dart where the variable is semitransparent to begin with. We'll do that as a non-NFC change in a later commit.

About the second paragraph:

  • I think the withValues calls in lib/widgets/channel_colors.dart should stay. The linked design for the computation (a replit by Vlad) doesn't specify multiplication; it says to set the alpha channel to 0.3, which we're doing with withValues.
  • I think the withValues in lightbox.dart can stay. It's setting an alpha channel on a constant from Flutter (Colors.grey.shade900) that realistically won't ever change to be semi-transparent. Even if it did, it's not clear that 0.87 would be the wrong value to end up with.
  • Like with the many withValues calls in lib/widgets/theme.dart, I think these don't need to be specifically mentioned in the commit message. They're using withValues(alpha: as intended, to create a color with its alpha channel set to a value. They might have done that same thing a different way, like in the Color constructor. That's different from "fad[ing] a color variable from [DesignVariables] […]", which we often do e.g. to translate from the Figma an opacity attribute on an element that's colored with a variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Changed the message to

And use it everywhere we were using `.withValues(alpha:`
to fade a color variable, except for one place in
`lib/widgets/emoji_reaction.dart` where the variable is
semi-transparent in dark mode to begin with.
We'll do that as a non-NFC change in a later commit.

with a minor tweak mentioning that, in dark mode, the color is semi-transparent.

assert(factor >= 0);
assert(factor <= 1);
return withValues(alpha: a * factor);
}
}
Loading