From 8972754db6a25532718be009c0331d8d3c685e59 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 12 Nov 2024 10:59:01 -0500 Subject: [PATCH 1/8] color [nfc]: Add ColorExtension.withFadedAlpha 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. Signed-off-by: Zixuan James Li --- lib/widgets/action_sheet.dart | 9 +++++---- lib/widgets/color.dart | 17 +++++++++++++++++ test/widgets/color_test.dart | 26 ++++++++++++++++++++++++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 844f820c13..d90f5f0c5f 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -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'; @@ -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) @@ -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); }, diff --git a/lib/widgets/color.dart b/lib/widgets/color.dart index 59c75826ba..4aff79ee16 100644 --- a/lib/widgets/color.dart +++ b/lib/widgets/color.dart @@ -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) { + assert(factor >= 0); + assert(factor <= 1); + return withValues(alpha: a * factor); + } } diff --git a/test/widgets/color_test.dart b/test/widgets/color_test.dart index 22f24a78b5..f88e9d3645 100644 --- a/test/widgets/color_test.dart +++ b/test/widgets/color_test.dart @@ -1,6 +1,6 @@ -import 'dart:ui'; - import 'package:checks/checks.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_checks/flutter_checks.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/widgets/color.dart'; @@ -14,5 +14,27 @@ void main() { check(Color(testCase).argbInt).equals(testCase); } }); + + const color = Color.fromRGBO(100, 200, 100, 0.5); + + test('withFadedAlpha smoke', () { + check(color.withFadedAlpha(0.5)) + .isSameColorAs(color.withValues(alpha: 0.25)); + }); + + test('withFadedAlpha opaque color', () { + const color = Colors.black; + + check(color.withFadedAlpha(0.5)) + .isSameColorAs(color.withValues(alpha: 0.5)); + }); + + test('withFadedAlpha factor > 1 fails', () { + check(() => color.withFadedAlpha(1.1)).throws(); + }); + + test('withFadedAlpha factor < 0 fails', () { + check(() => color.withFadedAlpha(-0.1)).throws(); + }); }); } From 026c0483202cecff6be77956fea8d6be67e18b64 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Tue, 12 Nov 2024 11:25:19 -0500 Subject: [PATCH 2/8] emoji: Use withFadedAlpha for highlightColor Because splashColor in dark mode is transparent, this changes the appearance. Signed-off-by: Zixuan James Li --- lib/widgets/emoji_reaction.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 3d87b7a8a0..d92a050b05 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -4,6 +4,7 @@ import 'package:flutter/material.dart'; import '../api/model/model.dart'; import '../api/route/messages.dart'; import '../model/emoji.dart'; +import 'color.dart'; import 'content.dart'; import 'store.dart'; import 'text.dart'; @@ -166,7 +167,7 @@ class ReactionChip extends StatelessWidget { final labelColor = selfVoted ? reactionTheme.textSelected : reactionTheme.textUnselected; final backgroundColor = selfVoted ? reactionTheme.bgSelected : reactionTheme.bgUnselected; final splashColor = selfVoted ? reactionTheme.bgUnselected : reactionTheme.bgSelected; - final highlightColor = splashColor.withValues(alpha: 0.5); + final highlightColor = splashColor.withFadedAlpha(0.5); final borderSide = BorderSide( color: borderColor, From 56b7a1959824d02362c102b598b454377c67e1e9 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 10 Oct 2024 17:58:39 -0700 Subject: [PATCH 3/8] theme: Update borderBar to follow change in Figma design See also: https://github.com/zulip/zulip-flutter/pull/928#discussion_r1796191390 Signed-off-by: Zixuan James Li --- lib/widgets/theme.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 0fe1ac6cfd..123a8507d8 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -117,7 +117,7 @@ class DesignVariables extends ThemeExtension { bgContextMenu: const Color(0xfff2f2f2), bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15), bgTopBar: const Color(0xfff5f5f5), - borderBar: const Color(0x33000000), + borderBar: Colors.black.withValues(alpha: 0.2), contextMenuCancelText: const Color(0xff222222), contextMenuItemBg: const Color(0xff6159e1), contextMenuItemText: const Color(0xff381da7), @@ -153,7 +153,7 @@ class DesignVariables extends ThemeExtension { bgContextMenu: const Color(0xff262626), bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37), bgTopBar: const Color(0xff242424), - borderBar: Colors.black.withValues(alpha: 0.41), + borderBar: Colors.black.withValues(alpha: 0.5), contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75), contextMenuItemBg: const Color(0xff7977fe), contextMenuItemText: const Color(0xff9398fd), From 58debf538e2fc0e1d4b2e89c76db1cfd35b365c0 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 23 Oct 2024 15:48:28 -0400 Subject: [PATCH 4/8] compose [nfc]: Refactor away shared iconTheme override Because these buttons share the same superclass, we can de-duplicate styling code without the theme override. This helps de-nest the layout code. While more types of buttons that are not subclasses of `_AttachUploadsButton` might be added, a theme is probably still unnecessary given that we can lift a common button class that handles the styling. Signed-off-by: Zixuan James Li --- lib/widgets/compose_box.dart | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 2060f420da..0ae69a1441 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -659,9 +659,10 @@ abstract class _AttachUploadsButton extends StatelessWidget { @override Widget build(BuildContext context) { + ColorScheme colorScheme = Theme.of(context).colorScheme; final zulipLocalizations = ZulipLocalizations.of(context); return IconButton( - icon: Icon(icon), + icon: Icon(icon, color: colorScheme.onSurfaceVariant), tooltip: tooltip(zulipLocalizations), onPressed: () => _handlePress(context)); } @@ -1033,14 +1034,11 @@ class _ComposeBoxLayout extends StatelessWidget { const SizedBox(width: 8), sendButton, ]), - Theme( - data: themeData.copyWith( - iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), - child: Row(children: [ - _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), - ])), + Row(children: [ + _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), + ]), ])); } } From 88b2ff93d7eddc32b8c29f11f212e399f3e89010 Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Thu, 14 Nov 2024 21:09:14 -0500 Subject: [PATCH 5/8] compose [nfc]: Extract compose buttons as a list Signed-off-by: Zixuan James Li --- lib/widgets/compose_box.dart | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 0ae69a1441..eee9787a9d 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1020,6 +1020,12 @@ class _ComposeBoxLayout extends StatelessWidget { ), ); + final composeButtons = [ + _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), + _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), + ]; + return _ComposeBoxContainer( child: Column(children: [ Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ @@ -1034,11 +1040,7 @@ class _ComposeBoxLayout extends StatelessWidget { const SizedBox(width: 8), sendButton, ]), - Row(children: [ - _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), - _AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode), - ]), + Row(children: composeButtons), ])); } } From 59565288fb55871e421be01d017b8f58f23fe72b Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 4 Sep 2024 18:40:20 -0400 Subject: [PATCH 6/8] compose: Use new icons from the redesign The icons are exported from the Figma design: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=544-22131&node-type=CANVAS&t=hNeqhPGojoFpTJnp-0 Due to the limitation of the icon font, image.svg and send.svg have been adjusted with a tool that converts SVG strokes to fills: https://www.npmjs.com/package/oslllo-svg-fixer See also: - https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Icon.20not.20rendering.20correctly/near/1936793 Signed-off-by: Zixuan James Li --- assets/icons/ZulipIcons.ttf | Bin 8016 -> 9220 bytes assets/icons/attach_file.svg | 3 ++ assets/icons/camera.svg | 3 ++ assets/icons/image.svg | 1 + assets/icons/send.svg | 1 + lib/widgets/compose_box.dart | 9 ++--- lib/widgets/icons.dart | 52 +++++++++++++++++----------- test/widgets/compose_box_test.dart | 15 ++++---- test/widgets/message_list_test.dart | 2 +- 9 files changed, 54 insertions(+), 32 deletions(-) create mode 100644 assets/icons/attach_file.svg create mode 100644 assets/icons/camera.svg create mode 100644 assets/icons/image.svg create mode 100644 assets/icons/send.svg diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 0f902b96fcc70d61669e566977920ad9a1245d7a..be5225e5a1efddad5a89338b4d32fb8f94c4c369 100644 GIT binary patch delta 2596 zcmZ`*U2I!t8GhgM>wm}gu}>T)C;p4w#Ic@2FSevS1u2cXenP*b_vs0J z!gG9wf6Gt!Uy?=&`6x^K=qO33egGef24O!yIm*)z6=31VSfGB|gL#^Uu`)s>MDu;d zKI8ah(LEiuX(~GYS!yu|qDY|xf{a640#OOeV=yd&MxhX?RK|K9dWkgZr2u)HRBEtF z!>^1aRD`u?)CsMC(F>2FlhST5!Igxa1+N^UXcTH8$UZtu&(Uenic0!Hyjz8+BSUde z4LSM}R&sDJk^!1sUBXGPTXhT}O6XXIOx$(ZL=Ov8r-P_H2(J=VoaD1$YyGd08uIB! zB_%BOqrPrEd#H-=ij(uW(+wZ0w_qbWB3fxv4f6v0y1fa*w;w6wq3`yr$MF|Tq5~>6 zLD-A?$g>z~|8+K~4@qP}-PNVAT67X(WG*_hyX!@?Zx{_9b!-*MXm82WoNC{bztaAj z>}mUyKwDFK+JBX;_LOpY`~!p&*Lw&q?nWVfcr-sigr8$BHdMm>Sz4y6bZsZq{!kgN zrV(=#kuu0(*m>S^C}IRLvra>7WK=}Aa@HEN2a1JSMcD7C*V{)Ge=8tTlQzVNs|Mg~J4iz|UEsLUoQqM&c^7*i&%4M# zF1RQ`F1i?oyx>Bd=_MCKkjpJG06E}A7gb1{T?ZqOt1ilrYc8^o7hPD8mt2fNuDcK~ zm6u!$KwfrHgcJn}j2AGx>_Sw3#YF}3s*6F$Yc9sOf21e4wS$+B08S3=B)dTOX{WCX z>;vs!Cj`Wf;0j?!f5;kN;y+4~G$%ch{;j;KysKulgmy*you1WyqkrVN?s?=r=Y8Ol zd^5f~{zIShNq*kKq3x)*#(1^hlHZp9PKWR`WHp^bY*>=rnB<0?FvK)lk?NQpVB3-m z$H3Ke4KyQRi`9BXl5Quf(axro$xO|hn6da~AeRhuHb>U3t{pFzNBD*{dtzoPld(FR z(Q2)lH;m__{6Zj~40e7HOy&c8Ga8HKtMcnHundAi*z%1)G8Y6(xqKW%f+d}vnms-% zScG}4DoB#K07wvzZ$!ty9i#S_e2-g-Q{Q9x+j#aT@pkzds%-3Mdys1=Cmu0EQZ9pf zCL;9%(te)QwGz4jeWI4J8_*Uoudz|VSjE(Qf>P)3F^DQxK_xf*YE!C;;P5Dh~;Do(GH|aJ}EW}~fbWw zI2`dvFvi8Is;K(t#QX2fb@nEPL#99E9WwJrQoYeo!k>?LhWh=HzKEws3k19-e^oas z)2!&SqUgEd&h?+J+nU!WzoCsZH5L4Ea8jOyU1CjW1_?vWARK$-1QJzO;Gl z*O^q07M5e${d+37RlQv2HzvwDauaFtNyUt$4rk3FZ#dj0AdF4oPBk^bmGnKd_mKO; z+D7?#d;iR>eLz2;VXT|BjNg6$Z?m%7}^=iJn-{qfXQTv z63uZ6i|B_Bd0Mg*%OHcagsD&eC-@L`QFa3M*C+vexAzm za~S1BG`GZY)og(NVa~msV!vUI(_$bgkz$!hXCF}pEAmthbAmdVxHpL-RFvGEoLvaV zc{ix-*@k&)QW;^SfZJ#9?=o3(jwKabC7(?3lXKU5S`0h7_Of$93lK-0x3%}Dy7ZL4 zAK0i|703g8lDdri3HP#m57(ElipyyII_%8&MoV$#k{J@rU2q?fB)iEoo8dM;!HP^) zUE;W67?N(}FJ_B1<2-)t^n6;f&X@%WXWZP9J!*9AXgPE@{&S!Hx$Omn+o#X-%^N>w-rosQ(-E@#g2mPepg|Hd|4qzZYhj+p7tFV3)~&y z6LeE}geBe;zk3Ytf%iAYMXD@n7@b>#OwL)&2UMe$YSrf3MCL{U?QUd+=Uo zHTa&t^I)jr?R*q@qVo&$L1w}EBa_m+iyKX+Xr4~%HG6$|$6l^#{HU#I^_^xlxW&)m lrLAhYy4q-NJ2%a__@XF_s@N4ZvBqyvODu`H*c7sL;BUSb#fty{ diff --git a/assets/icons/attach_file.svg b/assets/icons/attach_file.svg new file mode 100644 index 0000000000..1976e8fb06 --- /dev/null +++ b/assets/icons/attach_file.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/camera.svg b/assets/icons/camera.svg new file mode 100644 index 0000000000..5e54c454de --- /dev/null +++ b/assets/icons/camera.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/image.svg b/assets/icons/image.svg new file mode 100644 index 0000000000..d63d12ac5a --- /dev/null +++ b/assets/icons/image.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/icons/send.svg b/assets/icons/send.svg new file mode 100644 index 0000000000..4d87a3e446 --- /dev/null +++ b/assets/icons/send.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index eee9787a9d..ae6c62e845 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -15,6 +15,7 @@ import '../model/narrow.dart'; import '../model/store.dart'; import 'autocomplete.dart'; import 'dialog.dart'; +import 'icons.dart'; import 'store.dart'; import 'theme.dart'; @@ -726,7 +727,7 @@ class _AttachFileButton extends _AttachUploadsButton { const _AttachFileButton({required super.contentController, required super.contentFocusNode}); @override - IconData get icon => Icons.attach_file; + IconData get icon => ZulipIcons.attach_file; @override String tooltip(ZulipLocalizations zulipLocalizations) => @@ -742,7 +743,7 @@ class _AttachMediaButton extends _AttachUploadsButton { const _AttachMediaButton({required super.contentController, required super.contentFocusNode}); @override - IconData get icon => Icons.image; + IconData get icon => ZulipIcons.image; @override String tooltip(ZulipLocalizations zulipLocalizations) => @@ -759,7 +760,7 @@ class _AttachFromCameraButton extends _AttachUploadsButton { const _AttachFromCameraButton({required super.contentController, required super.contentFocusNode}); @override - IconData get icon => Icons.camera_alt; + IconData get icon => ZulipIcons.camera; @override String tooltip(ZulipLocalizations zulipLocalizations) => @@ -959,7 +960,7 @@ class _SendButtonState extends State<_SendButton> { tapTargetSize: MaterialTapTargetSize.shrinkWrap, ), color: foregroundColor, - icon: const Icon(Icons.send), + icon: const Icon(ZulipIcons.send), onPressed: _send)); } } diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index ebdb6a362c..03356448ef 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -33,65 +33,77 @@ abstract final class ZulipIcons { /// The Zulip custom icon "at_sign". static const IconData at_sign = IconData(0xf103, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "attach_file". + static const IconData attach_file = IconData(0xf104, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "bot". - static const IconData bot = IconData(0xf104, fontFamily: "Zulip Icons"); + static const IconData bot = IconData(0xf105, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "camera". + static const IconData camera = IconData(0xf106, fontFamily: "Zulip Icons"); /// The Zulip custom icon "chevron_right". - static const IconData chevron_right = IconData(0xf105, fontFamily: "Zulip Icons"); + static const IconData chevron_right = IconData(0xf107, fontFamily: "Zulip Icons"); /// The Zulip custom icon "clock". - static const IconData clock = IconData(0xf106, fontFamily: "Zulip Icons"); + static const IconData clock = IconData(0xf108, fontFamily: "Zulip Icons"); /// The Zulip custom icon "copy". - static const IconData copy = IconData(0xf107, fontFamily: "Zulip Icons"); + static const IconData copy = IconData(0xf109, fontFamily: "Zulip Icons"); /// The Zulip custom icon "format_quote". - static const IconData format_quote = IconData(0xf108, fontFamily: "Zulip Icons"); + static const IconData format_quote = IconData(0xf10a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "globe". - static const IconData globe = IconData(0xf109, fontFamily: "Zulip Icons"); + static const IconData globe = IconData(0xf10b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "group_dm". - static const IconData group_dm = IconData(0xf10a, fontFamily: "Zulip Icons"); + static const IconData group_dm = IconData(0xf10c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_sign". - static const IconData hash_sign = IconData(0xf10b, fontFamily: "Zulip Icons"); + static const IconData hash_sign = IconData(0xf10d, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "image". + static const IconData image = IconData(0xf10e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "language". - static const IconData language = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData language = IconData(0xf10f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "lock". - static const IconData lock = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData lock = IconData(0xf110, fontFamily: "Zulip Icons"); /// The Zulip custom icon "mute". - static const IconData mute = IconData(0xf10e, fontFamily: "Zulip Icons"); + static const IconData mute = IconData(0xf111, fontFamily: "Zulip Icons"); /// The Zulip custom icon "read_receipts". - static const IconData read_receipts = IconData(0xf10f, fontFamily: "Zulip Icons"); + static const IconData read_receipts = IconData(0xf112, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "send". + static const IconData send = IconData(0xf113, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share". - static const IconData share = IconData(0xf110, fontFamily: "Zulip Icons"); + static const IconData share = IconData(0xf114, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share_ios". - static const IconData share_ios = IconData(0xf111, fontFamily: "Zulip Icons"); + static const IconData share_ios = IconData(0xf115, fontFamily: "Zulip Icons"); /// The Zulip custom icon "smile". - static const IconData smile = IconData(0xf112, fontFamily: "Zulip Icons"); + static const IconData smile = IconData(0xf116, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star". - static const IconData star = IconData(0xf113, fontFamily: "Zulip Icons"); + static const IconData star = IconData(0xf117, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star_filled". - static const IconData star_filled = IconData(0xf114, fontFamily: "Zulip Icons"); + static const IconData star_filled = IconData(0xf118, fontFamily: "Zulip Icons"); /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf115, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf119, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf116, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf11a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf117, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf11b, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 008b0f39c8..60e47b6e01 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -19,6 +19,7 @@ import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/page.dart'; +import 'package:zulip/widgets/icons.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -299,7 +300,7 @@ void main() { connection.prepare(json: {}); connection.prepare(json: SendMessageResult(id: 123).toJson()); - await tester.tap(find.byIcon(Icons.send)); + await tester.tap(find.byIcon(ZulipIcons.send)); await tester.pump(Duration.zero); final requests = connection.takeRequests(); checkSetTypingStatusRequests([requests.first], [(TypingOp.stop, narrow)]); @@ -453,7 +454,7 @@ void main() { group('uploads', () { void checkAppearsLoading(WidgetTester tester, bool expected) { final sendButtonElement = tester.element(find.ancestor( - of: find.byIcon(Icons.send), + of: find.byIcon(ZulipIcons.send), matching: find.byType(IconButton))); final sendButtonWidget = sendButtonElement.widget as IconButton; final colorScheme = Theme.of(sendButtonElement).colorScheme; @@ -492,7 +493,7 @@ void main() { connection.prepare(delay: const Duration(seconds: 1), json: UploadFileResult(uri: '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/image.jpg').toJson()); - await tester.tap(find.byIcon(Icons.image)); + await tester.tap(find.byIcon(ZulipIcons.image)); await tester.pump(); final call = testBinding.takePickFilesCalls().single; check(call.allowMultiple).equals(true); @@ -554,7 +555,7 @@ void main() { connection.prepare(delay: const Duration(seconds: 1), json: UploadFileResult(uri: '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/image.jpg').toJson()); - await tester.tap(find.byIcon(Icons.camera_alt)); + await tester.tap(find.byIcon(ZulipIcons.camera)); await tester.pump(); final call = testBinding.takePickImageCalls().single; check(call.source).equals(ImageSource.camera); @@ -602,9 +603,9 @@ void main() { void checkComposeBoxParts({required bool areShown}) { final inputFieldCount = inputFieldFinder().evaluate().length; areShown ? check(inputFieldCount).isGreaterThan(0) : check(inputFieldCount).equals(0); - check(attachButtonFinder(Icons.attach_file).evaluate().length).equals(areShown ? 1 : 0); - check(attachButtonFinder(Icons.image).evaluate().length).equals(areShown ? 1 : 0); - check(attachButtonFinder(Icons.camera_alt).evaluate().length).equals(areShown ? 1 : 0); + check(attachButtonFinder(ZulipIcons.attach_file).evaluate().length).equals(areShown ? 1 : 0); + check(attachButtonFinder(ZulipIcons.image).evaluate().length).equals(areShown ? 1 : 0); + check(attachButtonFinder(ZulipIcons.camera).evaluate().length).equals(areShown ? 1 : 0); } void checkBannerWithLabel(String label, {required bool isShown}) { diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index 444f78c4c9..37ce921f43 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -653,7 +653,7 @@ void main() { ..controller.isNotNull().text.equals('Some text'); connection.prepare(json: SendMessageResult(id: 1).toJson()); - await tester.tap(find.byIcon(Icons.send)); + await tester.tap(find.byIcon(ZulipIcons.send)); await tester.pump(); check(connection.lastRequest).isA() ..method.equals('POST') From 3adb39557c51604d85741664026c85225fba52fa Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Wed, 9 Oct 2024 17:30:50 -0700 Subject: [PATCH 7/8] compose: Implement most of compose box redesign - We drop `_sendButtonSize` and `_inputVerticalPadding` because we no longer need them for setting the button's minHeight, along with `ButtonStyle` for the send button that was added in #399, which is irrelevant to the new design. - `ClipRect`'s size is determined by the `ConstrainedBox`. This is mainly for showing the content through the `contentPadding` of the `TextField`, so that our `InsetShadowBox` can fade it smoothly there. The shadow is always there, but it is only visible when the `TextField` is long enough to be scrollable. Discussion here: https://github.com/zulip/zulip-flutter/pull/928#issuecomment-2335042432 - For `InputDecorationTheme` on `_ComposeBoxLayout`, we zero out `contentPadding` while keeping `isDense` as `true`, to explicitly remove paddings on the input widgets. - The height of the compose buttons is 42px in the Figma design, but 44px in the implementation. We change that to match the minimum button size per the accessibility recommendation from Apple. Discussion here: https://github.com/zulip/zulip-flutter/pull/928#discussion_r1842988359 - Note that we use `withFadedAlpha` on `designVariables.textInput` because the color is already transparent in dark mode, and the helper allows us to multiply, instead of to override, the alpha channel of the color with a factor. Discussion here: https://github.com/zulip/zulip-flutter/pull/928#discussion_r1815661095 - DesignVariables.icon's value has been updated to match the current design. This would affect the appearance of the ChooseAccountPageOverflowButton on the choose account page, which is intentional. This is "most of" the redesign because the new button feedback is supported later. Design spec here: - https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3954-13395 - https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3862-14350 Signed-off-by: Zixuan James Li --- lib/widgets/compose_box.dart | 231 +++++++++++++++++------------ lib/widgets/theme.dart | 25 +++- test/widgets/compose_box_test.dart | 76 +++++++++- 3 files changed, 228 insertions(+), 104 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index ae6c62e845..00788cc877 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -14,13 +14,15 @@ import '../model/compose.dart'; import '../model/narrow.dart'; import '../model/store.dart'; import 'autocomplete.dart'; +import 'color.dart'; import 'dialog.dart'; import 'icons.dart'; +import 'inset_shadow.dart'; import 'store.dart'; +import 'text.dart'; import 'theme.dart'; -const double _inputVerticalPadding = 8; -const double _sendButtonSize = 36; +const double _composeButtonSize = 44; /// A [TextEditingController] for use in the compose box. /// @@ -364,34 +366,77 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve } } + static double maxHeight(BuildContext context) { + final clampingTextScaler = MediaQuery.textScalerOf(context) + .clamp(maxScaleFactor: 1.5); + final scaledLineHeight = clampingTextScaler.scale(_fontSize) * _lineHeightRatio; + + // Reserve space to fully show the first 7th lines and just partially + // clip the 8th line, where the height matches the spec at + // https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3960-5147&node-type=text&m=dev + // > Maximum size of the compose box is suggested to be 178px. Which + // > has 7 fully visible lines of text + // + // The partial line hints that the content input is scrollable. + // + // Using the ambient TextScale means this works for different values of the + // system text-size setting. We clamp to a max scale factor to limit + // how tall the content input can get; that's to save room for the message + // list. The user can still scroll the input to see everything. + return _verticalPadding + 7.727 * scaledLineHeight; + } + + static const _verticalPadding = 8.0; + static const _fontSize = 17.0; + static const _lineHeight = 22.0; + static const _lineHeightRatio = _lineHeight / _fontSize; + @override Widget build(BuildContext context) { - ColorScheme colorScheme = Theme.of(context).colorScheme; - - return InputDecorator( - decoration: const InputDecoration(), - child: ConstrainedBox( - constraints: const BoxConstraints( - minHeight: _sendButtonSize - 2 * _inputVerticalPadding, - - // TODO constrain this adaptively (i.e. not hard-coded 200) - maxHeight: 200, - ), - child: ComposeAutocomplete( - narrow: widget.narrow, - controller: widget.controller, - focusNode: widget.focusNode, - fieldViewBuilder: (context) { - return TextField( + final designVariables = DesignVariables.of(context); + + return ComposeAutocomplete( + narrow: widget.narrow, + controller: widget.controller, + focusNode: widget.focusNode, + fieldViewBuilder: (context) => ConstrainedBox( + constraints: BoxConstraints(maxHeight: maxHeight(context)), + // This [ClipRect] replaces the [TextField] clipping we disable below. + child: ClipRect( + child: InsetShadowBox( + top: _verticalPadding, bottom: _verticalPadding, + color: designVariables.composeBoxBg, + child: TextField( controller: widget.controller, focusNode: widget.focusNode, - style: TextStyle(color: colorScheme.onSurface), - decoration: InputDecoration.collapsed(hintText: widget.hintText), + // Let the content show through the `contentPadding` so that + // our [InsetShadowBox] can fade it smoothly there. + clipBehavior: Clip.none, + style: TextStyle( + fontSize: _fontSize, + height: _lineHeightRatio, + color: designVariables.textInput), + // From the spec at + // https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3960-5147&node-type=text&m=dev + // > Compose box has the height to fit 2 lines. This is [done] to + // > have a bigger hit area for the user to start the input. […] + minLines: 2, maxLines: null, textCapitalization: TextCapitalization.sentences, - ); - }), - )); + decoration: InputDecoration( + // This padding ensures that the user can always scroll long + // content entirely out of the top or bottom shadow if desired. + // With this and the `minLines: 2` above, an empty content input + // gets 60px vertical distance (with no text-size scaling) + // between the top of the top shadow and the bottom of the + // bottom shadow. That's a bit more than the 54px given in the + // Figma, and we can revisit if needed, but it's tricky to get + // that 54px distance while also making the scrolling work like + // this and offering two lines of touchable area. + contentPadding: const EdgeInsets.symmetric(vertical: _verticalPadding), + hintText: widget.hintText, + hintStyle: TextStyle( + color: designVariables.textInput.withFadedAlpha(0.5)))))))); } } @@ -474,20 +519,32 @@ class _TopicInput extends StatelessWidget { @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); - ColorScheme colorScheme = Theme.of(context).colorScheme; + final designVariables = DesignVariables.of(context); + TextStyle topicTextStyle = TextStyle( + fontSize: 20, + height: 22 / 20, + color: designVariables.textInput.withFadedAlpha(0.9), + ).merge(weightVariableTextStyle(context, wght: 600)); return TopicAutocomplete( streamId: streamId, controller: controller, focusNode: focusNode, contentFocusNode: contentFocusNode, - fieldViewBuilder: (context) => TextField( - controller: controller, - focusNode: focusNode, - textInputAction: TextInputAction.next, - style: TextStyle(color: colorScheme.onSurface), - decoration: InputDecoration(hintText: zulipLocalizations.composeBoxTopicHintText), - )); + fieldViewBuilder: (context) => Container( + padding: const EdgeInsets.only(top: 10, bottom: 9), + decoration: BoxDecoration(border: Border(bottom: BorderSide( + width: 1, + color: designVariables.foreground.withFadedAlpha(0.2)))), + child: TextField( + controller: controller, + focusNode: focusNode, + textInputAction: TextInputAction.next, + style: topicTextStyle, + decoration: InputDecoration( + hintText: zulipLocalizations.composeBoxTopicHintText, + hintStyle: topicTextStyle.copyWith( + color: designVariables.textInput.withFadedAlpha(0.5)))))); } } @@ -660,12 +717,14 @@ abstract class _AttachUploadsButton extends StatelessWidget { @override Widget build(BuildContext context) { - ColorScheme colorScheme = Theme.of(context).colorScheme; + final designVariables = DesignVariables.of(context); final zulipLocalizations = ZulipLocalizations.of(context); - return IconButton( - icon: Icon(icon, color: colorScheme.onSurfaceVariant), - tooltip: tooltip(zulipLocalizations), - onPressed: () => _handlePress(context)); + return SizedBox( + width: _composeButtonSize, + child: IconButton( + icon: Icon(icon, color: designVariables.foreground.withFadedAlpha(0.5)), + tooltip: tooltip(zulipLocalizations), + onPressed: () => _handlePress(context))); } } @@ -929,38 +988,22 @@ class _SendButtonState extends State<_SendButton> { @override Widget build(BuildContext context) { - final disabled = _hasValidationErrors; - final colorScheme = Theme.of(context).colorScheme; + final designVariables = DesignVariables.of(context); final zulipLocalizations = ZulipLocalizations.of(context); - // Copy FilledButton defaults (_FilledButtonDefaultsM3.backgroundColor) - final backgroundColor = disabled - ? colorScheme.onSurface.withValues(alpha: 0.12) - : colorScheme.primary; + final iconColor = _hasValidationErrors + ? designVariables.icon.withFadedAlpha(0.5) + : designVariables.icon; - // Copy FilledButton defaults (_FilledButtonDefaultsM3.foregroundColor) - final foregroundColor = disabled - ? colorScheme.onSurface.withValues(alpha: 0.38) - : colorScheme.onPrimary; - - return Ink( - decoration: BoxDecoration( - borderRadius: const BorderRadius.all(Radius.circular(8.0)), - color: backgroundColor, - ), + return SizedBox( + width: _composeButtonSize, child: IconButton( tooltip: zulipLocalizations.composeBoxSendTooltip, - style: const ButtonStyle( - // Match the height of the content input. - minimumSize: WidgetStatePropertyAll(Size.square(_sendButtonSize)), - // With the default of [MaterialTapTargetSize.padded], not just the - // tap target but the visual button would get padded to 48px square. - // It would be nice if the tap target extended invisibly out from the - // button, to make a 48px square, but that's not the behavior we get. - tapTargetSize: MaterialTapTargetSize.shrinkWrap, - ), - color: foregroundColor, - icon: const Icon(ZulipIcons.send), + icon: Icon(ZulipIcons.send, + // We set [Icon.color] instead of [IconButton.color] because the + // latter implicitly uses colors derived from it to override the + // ambient [ButtonStyle.overlayColor]. + color: iconColor), onPressed: _send)); } } @@ -972,18 +1015,17 @@ class _ComposeBoxContainer extends StatelessWidget { @override Widget build(BuildContext context) { - ColorScheme colorScheme = Theme.of(context).colorScheme; + final designVariables = DesignVariables.of(context); // TODO(design): Maybe put a max width on the compose box, like we do on // the message list itself - return SizedBox(width: double.infinity, + return Container(width: double.infinity, + decoration: BoxDecoration( + border: Border(top: BorderSide(color: designVariables.borderBar))), child: Material( - color: colorScheme.surfaceContainerHighest, - child: SafeArea( - minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), - child: Padding( - padding: const EdgeInsets.only(top: 8.0), - child: child)))); + color: designVariables.composeBoxBg, + child: SafeArea(minimum: const EdgeInsets.symmetric(horizontal: 8), + child: child))); } } @@ -1004,22 +1046,14 @@ class _ComposeBoxLayout extends StatelessWidget { @override Widget build(BuildContext context) { - ThemeData themeData = Theme.of(context); - ColorScheme colorScheme = themeData.colorScheme; + final themeData = Theme.of(context); final inputThemeData = themeData.copyWith( - inputDecorationTheme: InputDecorationTheme( + inputDecorationTheme: const InputDecorationTheme( // Both [contentPadding] and [isDense] combine to make the layout compact. isDense: true, - contentPadding: const EdgeInsets.symmetric( - horizontal: 12.0, vertical: _inputVerticalPadding), - border: const OutlineInputBorder( - borderRadius: BorderRadius.all(Radius.circular(4.0)), - borderSide: BorderSide.none), - filled: true, - fillColor: colorScheme.surface, - ), - ); + contentPadding: EdgeInsets.zero, + border: InputBorder.none)); final composeButtons = [ _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), @@ -1029,19 +1063,22 @@ class _ComposeBoxLayout extends StatelessWidget { return _ComposeBoxContainer( child: Column(children: [ - Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ - Expanded( - child: Theme( - data: inputThemeData, - child: Column(children: [ - if (topicInput != null) topicInput!, - if (topicInput != null) const SizedBox(height: 8), - contentInput, - ]))), - const SizedBox(width: 8), - sendButton, - ]), - Row(children: composeButtons), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 8), + child: Theme( + data: inputThemeData, + child: Column(children: [ + if (topicInput != null) topicInput!, + contentInput, + ]))), + SizedBox( + height: _composeButtonSize, + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Row(children: composeButtons), + sendButton, + ])), ])); } } diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 123a8507d8..2e3348a403 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -118,14 +118,17 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15), bgTopBar: const Color(0xfff5f5f5), borderBar: Colors.black.withValues(alpha: 0.2), + composeBoxBg: const Color(0xffffffff), contextMenuCancelText: const Color(0xff222222), contextMenuItemBg: const Color(0xff6159e1), contextMenuItemText: const Color(0xff381da7), - icon: const Color(0xff666699), + foreground: const Color(0xff000000), + icon: const Color(0xff6159e1), labelCounterUnread: const Color(0xff222222), labelEdited: const HSLColor.fromAHSL(0.35, 0, 0, 0).toColor(), labelMenuButton: const Color(0xff222222), mainBackground: const Color(0xfff0f0f0), + textInput: const Color(0xff000000), title: const Color(0xff1a1a1a), channelColorSwatches: ChannelColorSwatches.light, atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(), @@ -154,14 +157,17 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37), bgTopBar: const Color(0xff242424), borderBar: Colors.black.withValues(alpha: 0.5), + composeBoxBg: const Color(0xff0f0f0f), contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75), contextMenuItemBg: const Color(0xff7977fe), contextMenuItemText: const Color(0xff9398fd), - icon: const Color(0xff7070c2), + foreground: const Color(0xffffffff), + icon: const Color(0xff7977fe), labelCounterUnread: const Color(0xffffffff).withValues(alpha: 0.7), labelEdited: const HSLColor.fromAHSL(0.35, 0, 0, 1).toColor(), labelMenuButton: const Color(0xffffffff).withValues(alpha: 0.85), mainBackground: const Color(0xff1d1d1d), + textInput: const Color(0xffffffff).withValues(alpha: 0.9), title: const Color(0xffffffff), channelColorSwatches: ChannelColorSwatches.dark, contextMenuCancelBg: const Color(0xff797986), // the same as the light mode in Figma @@ -197,14 +203,17 @@ class DesignVariables extends ThemeExtension { required this.bgCounterUnread, required this.bgTopBar, required this.borderBar, + required this.composeBoxBg, required this.contextMenuCancelText, required this.contextMenuItemBg, required this.contextMenuItemText, + required this.foreground, required this.icon, required this.labelCounterUnread, required this.labelEdited, required this.labelMenuButton, required this.mainBackground, + required this.textInput, required this.title, required this.channelColorSwatches, required this.atMentionMarker, @@ -241,14 +250,17 @@ class DesignVariables extends ThemeExtension { final Color bgCounterUnread; final Color bgTopBar; final Color borderBar; + final Color composeBoxBg; final Color contextMenuCancelText; final Color contextMenuItemBg; final Color contextMenuItemText; + final Color foreground; final Color icon; final Color labelCounterUnread; final Color labelEdited; final Color labelMenuButton; final Color mainBackground; + final Color textInput; final Color title; // Not exactly from the Figma design, but from Vlad anyway. @@ -280,14 +292,17 @@ class DesignVariables extends ThemeExtension { Color? bgCounterUnread, Color? bgTopBar, Color? borderBar, + Color? composeBoxBg, Color? contextMenuCancelText, Color? contextMenuItemBg, Color? contextMenuItemText, + Color? foreground, Color? icon, Color? labelCounterUnread, Color? labelEdited, Color? labelMenuButton, Color? mainBackground, + Color? textInput, Color? title, ChannelColorSwatches? channelColorSwatches, Color? atMentionMarker, @@ -314,14 +329,17 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: bgCounterUnread ?? this.bgCounterUnread, bgTopBar: bgTopBar ?? this.bgTopBar, borderBar: borderBar ?? this.borderBar, + composeBoxBg: composeBoxBg ?? this.composeBoxBg, contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText, contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg, contextMenuItemText: contextMenuItemText ?? this.contextMenuItemBg, + foreground: foreground ?? this.foreground, icon: icon ?? this.icon, labelCounterUnread: labelCounterUnread ?? this.labelCounterUnread, labelEdited: labelEdited ?? this.labelEdited, labelMenuButton: labelMenuButton ?? this.labelMenuButton, mainBackground: mainBackground ?? this.mainBackground, + textInput: textInput ?? this.textInput, title: title ?? this.title, channelColorSwatches: channelColorSwatches ?? this.channelColorSwatches, atMentionMarker: atMentionMarker ?? this.atMentionMarker, @@ -355,14 +373,17 @@ class DesignVariables extends ThemeExtension { bgCounterUnread: Color.lerp(bgCounterUnread, other.bgCounterUnread, t)!, bgTopBar: Color.lerp(bgTopBar, other.bgTopBar, t)!, borderBar: Color.lerp(borderBar, other.borderBar, t)!, + composeBoxBg: Color.lerp(composeBoxBg, other.composeBoxBg, t)!, contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!, contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!, contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemBg, t)!, + foreground: Color.lerp(foreground, other.foreground, t)!, icon: Color.lerp(icon, other.icon, t)!, labelCounterUnread: Color.lerp(labelCounterUnread, other.labelCounterUnread, t)!, labelEdited: Color.lerp(labelEdited, other.labelEdited, t)!, labelMenuButton: Color.lerp(labelMenuButton, other.labelMenuButton, t)!, mainBackground: Color.lerp(mainBackground, other.mainBackground, t)!, + textInput: Color.lerp(textInput, other.textInput, t)!, title: Color.lerp(title, other.title, t)!, channelColorSwatches: ChannelColorSwatches.lerp(channelColorSwatches, other.channelColorSwatches, t), atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!, diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 60e47b6e01..a19356ef40 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -17,9 +17,11 @@ import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/icons.dart'; +import 'package:zulip/widgets/theme.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -457,11 +459,12 @@ void main() { of: find.byIcon(ZulipIcons.send), matching: find.byType(IconButton))); final sendButtonWidget = sendButtonElement.widget as IconButton; - final colorScheme = Theme.of(sendButtonElement).colorScheme; - final expectedForegroundColor = expected - ? colorScheme.onSurface.withValues(alpha: 0.38) - : colorScheme.onPrimary; - check(sendButtonWidget.color).isNotNull().isSameColorAs(expectedForegroundColor); + final designVariables = DesignVariables.of(sendButtonElement); + final expectedIconColor = expected + ? designVariables.icon.withFadedAlpha(0.5) + : designVariables.icon; + check(sendButtonWidget.icon) + .isA().color.isNotNull().isSameColorAs(expectedIconColor); } group('attach from media library', () { @@ -796,4 +799,67 @@ void main() { }); }); }); + + group('ComposeBox content input scaling', () { + const verticalPadding = 8; + final stream = eg.stream(); + final narrow = TopicNarrow(stream.streamId, 'foo'); + + Future checkContentInputMaxHeight(WidgetTester tester, { + required double maxHeight, + required int maxVisibleLines, + }) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + // Add one line at a time, until the content input reaches its max height. + int numLines; + double? height; + for (numLines = 2; numLines <= 1000; numLines++) { + final content = List.generate(numLines, (_) => 'foo').join('\n'); + await tester.enterText(contentInputFinder, content); + await tester.pump(); + final newHeight = tester.getRect(contentInputFinder).height; + if (newHeight == height) { + break; + } + height = newHeight; + } + check(height).isNotNull().isCloseTo(maxHeight, 0.5); + // The last line added did not stretch the content input, + // so only the lines before it are at least partially visible. + check(numLines - 1).equals(maxVisibleLines); + } + + testWidgets('normal text scale factor', (tester) async { + await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + + await checkContentInputMaxHeight(tester, + maxHeight: verticalPadding + 170, maxVisibleLines: 8); + }); + + testWidgets('lower text scale factor', (tester) async { + tester.platformDispatcher.textScaleFactorTestValue = 0.8; + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await checkContentInputMaxHeight(tester, + maxHeight: verticalPadding + 170 * 0.8, maxVisibleLines: 8); + }); + + testWidgets('higher text scale factor', (tester) async { + tester.platformDispatcher.textScaleFactorTestValue = 1.5; + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await checkContentInputMaxHeight(tester, + maxHeight: verticalPadding + 170 * 1.5, maxVisibleLines: 8); + }); + + testWidgets('higher text scale factor exceeding threshold', (tester) async { + tester.platformDispatcher.textScaleFactorTestValue = 2; + addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); + await prepareComposeBox(tester, narrow: narrow, streams: [stream]); + await checkContentInputMaxHeight(tester, + maxHeight: verticalPadding + 170 * 1.5, maxVisibleLines: 6); + }); + }); } From 94cf40ebc61a1365702a1c18d76a797b5805b75c Mon Sep 17 00:00:00 2001 From: Zixuan James Li Date: Fri, 11 Oct 2024 17:08:43 -0700 Subject: [PATCH 8/8] compose: Support redesigned button feedback This disables the splash effect for all the compose buttons and the send button, and implements a rounded rectangular background that appears on hover/pressed. Different from the style that all the compose buttons share, this feedback also applies to the send button, and will apply to more buttons in the app. In the future we should migrate more buttons to use this style, so it doesn't belong to a shared base class. See also: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3707-41711&node-type=frame&t=sSYomsJzGCt34D8N-0 Fixes: #915 Signed-off-by: Zixuan James Li --- lib/widgets/compose_box.dart | 30 +++++++++++++++++++++++------- lib/widgets/theme.dart | 7 +++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 00788cc877..b6f187921b 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1002,7 +1002,8 @@ class _SendButtonState extends State<_SendButton> { icon: Icon(ZulipIcons.send, // We set [Icon.color] instead of [IconButton.color] because the // latter implicitly uses colors derived from it to override the - // ambient [ButtonStyle.overlayColor]. + // ambient [ButtonStyle.overlayColor], where we set the color for + // the highlight state to match the Figma design. color: iconColor), onPressed: _send)); } @@ -1047,6 +1048,7 @@ class _ComposeBoxLayout extends StatelessWidget { @override Widget build(BuildContext context) { final themeData = Theme.of(context); + final designVariables = DesignVariables.of(context); final inputThemeData = themeData.copyWith( inputDecorationTheme: const InputDecorationTheme( @@ -1055,6 +1057,18 @@ class _ComposeBoxLayout extends StatelessWidget { contentPadding: EdgeInsets.zero, border: InputBorder.none)); + // TODO(#417): Disable splash effects for all buttons globally. + final iconButtonThemeData = IconButtonThemeData( + style: IconButton.styleFrom( + splashFactory: NoSplash.splashFactory, + // TODO(#417): The Figma design specifies a different icon color on + // pressed, but `IconButton` currently does not have support for + // that. See also: + // https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3707-41711&node-type=frame&t=sSYomsJzGCt34D8N-0 + highlightColor: designVariables.editorButtonPressedBg, + shape: const RoundedRectangleBorder( + borderRadius: BorderRadius.all(Radius.circular(4))))); + final composeButtons = [ _AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode), _AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode), @@ -1073,12 +1087,14 @@ class _ComposeBoxLayout extends StatelessWidget { ]))), SizedBox( height: _composeButtonSize, - child: Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, - children: [ - Row(children: composeButtons), - sendButton, - ])), + child: IconButtonTheme( + data: iconButtonThemeData, + child: Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Row(children: composeButtons), + sendButton, + ]))), ])); } } diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index 2e3348a403..f8335745d2 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -122,6 +122,7 @@ class DesignVariables extends ThemeExtension { contextMenuCancelText: const Color(0xff222222), contextMenuItemBg: const Color(0xff6159e1), contextMenuItemText: const Color(0xff381da7), + editorButtonPressedBg: Colors.black.withValues(alpha: 0.06), foreground: const Color(0xff000000), icon: const Color(0xff6159e1), labelCounterUnread: const Color(0xff222222), @@ -161,6 +162,7 @@ class DesignVariables extends ThemeExtension { contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75), contextMenuItemBg: const Color(0xff7977fe), contextMenuItemText: const Color(0xff9398fd), + editorButtonPressedBg: Colors.white.withValues(alpha: 0.06), foreground: const Color(0xffffffff), icon: const Color(0xff7977fe), labelCounterUnread: const Color(0xffffffff).withValues(alpha: 0.7), @@ -207,6 +209,7 @@ class DesignVariables extends ThemeExtension { required this.contextMenuCancelText, required this.contextMenuItemBg, required this.contextMenuItemText, + required this.editorButtonPressedBg, required this.foreground, required this.icon, required this.labelCounterUnread, @@ -254,6 +257,7 @@ class DesignVariables extends ThemeExtension { final Color contextMenuCancelText; final Color contextMenuItemBg; final Color contextMenuItemText; + final Color editorButtonPressedBg; final Color foreground; final Color icon; final Color labelCounterUnread; @@ -296,6 +300,7 @@ class DesignVariables extends ThemeExtension { Color? contextMenuCancelText, Color? contextMenuItemBg, Color? contextMenuItemText, + Color? editorButtonPressedBg, Color? foreground, Color? icon, Color? labelCounterUnread, @@ -333,6 +338,7 @@ class DesignVariables extends ThemeExtension { contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText, contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg, contextMenuItemText: contextMenuItemText ?? this.contextMenuItemBg, + editorButtonPressedBg: editorButtonPressedBg ?? this.editorButtonPressedBg, foreground: foreground ?? this.foreground, icon: icon ?? this.icon, labelCounterUnread: labelCounterUnread ?? this.labelCounterUnread, @@ -377,6 +383,7 @@ class DesignVariables extends ThemeExtension { contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!, contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!, contextMenuItemText: Color.lerp(contextMenuItemText, other.contextMenuItemBg, t)!, + editorButtonPressedBg: Color.lerp(editorButtonPressedBg, other.editorButtonPressedBg, t)!, foreground: Color.lerp(foreground, other.foreground, t)!, icon: Color.lerp(icon, other.icon, t)!, labelCounterUnread: Color.lerp(labelCounterUnread, other.labelCounterUnread, t)!,