From bd37691813709f3750b73f81c754a20268141e9b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 7 May 2024 14:54:00 -0700 Subject: [PATCH 1/5] content [nfc]: Stop redundantly setting spans' background in code blocks The code block background is currently white anyway, so this has been redundant. But we're about to set it to something off-white, to align with web, and this white background color on the text spans would interfere with that. --- lib/widgets/content.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index be8cff89a8..41a5beeafc 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -773,9 +773,7 @@ final _kInlineCodeStyle = kMonospaceTextStyle .merge(const TextStyle(backgroundColor: Color(0xffeeeeee))); final _kCodeBlockStyle = kMonospaceTextStyle - .merge(const TextStyle( - backgroundColor: Color.fromRGBO(255, 255, 255, 1), - fontSize: 0.825 * kBaseFontSize)) + .merge(const TextStyle(fontSize: 0.825 * kBaseFontSize)) .merge( // TODO(a11y) pass a BuildContext, to handle platform request for bold text. // To get one, the result of this whole computation (to the TextStyle From 89d76b14213b9b03a07556ad32e254005398a0e4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 7 May 2024 14:37:22 -0700 Subject: [PATCH 2/5] content: Match code blocks' background and border colors to web (Specifically by eliminating the border and using a particular shade of gray for the background.) Also the background of inline code, which in web uses the same CSS variable as the background color for code blocks. --- lib/widgets/content.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 41a5beeafc..298dd45751 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -406,12 +406,10 @@ class CodeBlock extends StatelessWidget { final CodeBlockNode node; - static final _borderColor = const HSLColor.fromAHSL(0.15, 0, 0, 0).toColor(); - @override Widget build(BuildContext context) { return _CodeBlockContainer( - borderColor: _borderColor, + borderColor: Colors.transparent, child: Text.rich(_buildNodes(node.spans))); } @@ -436,7 +434,7 @@ class _CodeBlockContainer extends StatelessWidget { Widget build(BuildContext context) { return Container( decoration: BoxDecoration( - color: Colors.white, + color: const HSLColor.fromAHSL(0.04, 0, 0, 0).toColor(), border: Border.all( width: 1, color: borderColor), @@ -725,7 +723,6 @@ class _InlineContentBuilder { // TODO `code`: find equivalent of web's `unicode-bidi: embed; direction: ltr` return _buildNodes( - // Use a light gray background, instead of a border. style: widget.style .merge(_kInlineCodeStyle) .apply(fontSizeFactor: _kInlineCodeFontSizeFactor), @@ -770,7 +767,8 @@ const _kInlineCodeFontSizeFactor = 0.825; // assuming the text gets the effect of [weightVariableTextStyle] // through inheritance, e.g., from a [DefaultTextStyle]. final _kInlineCodeStyle = kMonospaceTextStyle - .merge(const TextStyle(backgroundColor: Color(0xffeeeeee))); + .merge(TextStyle( + backgroundColor: const HSLColor.fromAHSL(0.04, 0, 0, 0).toColor())); final _kCodeBlockStyle = kMonospaceTextStyle .merge(const TextStyle(fontSize: 0.825 * kBaseFontSize)) From 49895126e363c07257a7ac76e17b0e8da0b64b35 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 9 May 2024 18:25:54 -0600 Subject: [PATCH 3/5] content: Style @-mention pills with 2023+ direct-mention style background As the TODO comments say, there's more we have to do to bring this up-to-date with the new, 2023+ design. But now at least we've removed a style that doesn't appear at all in that new design, and we're left with something we can make a dark-theme variant for by straightforwardly checking what web does. --- lib/widgets/content.dart | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 298dd45751..345431edae 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -817,17 +817,18 @@ class UserMention extends StatelessWidget { // One hopes an @-mention can't contain an embedded link. // (The parser on creating a UserMentionNode has a TODO to check that.) linkRecognizers: null, + + // TODO(#647) when self-user is non-silently mentioned, make bold, and: + // TODO(#646) when self-user is non-silently mentioned, + // distinguish font color between direct and wildcard mentions style: surroundingTextStyle, + nodes: node.nodes)); } static get _kDecoration => BoxDecoration( - gradient: const LinearGradient( - colors: [Color.fromRGBO(0, 0, 0, 0.1), Color.fromRGBO(0, 0, 0, 0)], - begin: Alignment.topCenter, - end: Alignment.bottomCenter), - border: Border.all( - color: const Color.fromRGBO(0xcc, 0xcc, 0xcc, 1), width: 1), + // TODO(#646) different background between direct and wildcard mentions + color: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), borderRadius: const BorderRadius.all(Radius.circular(3))); // This is a more literal translation of Zulip web's CSS. From ab82af1817d27bd810ccb309a06fe3b0c7caf996 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 9 May 2024 18:40:18 -0600 Subject: [PATCH 4/5] content [nfc]: In UserMention, inline _kDecoration --- lib/widgets/content.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 345431edae..8dd6fc7d28 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -809,7 +809,10 @@ class UserMention extends StatelessWidget { @override Widget build(BuildContext context) { return Container( - decoration: _kDecoration, + decoration: BoxDecoration( + // TODO(#646) different background between direct and wildcard mentions + color: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), + borderRadius: const BorderRadius.all(Radius.circular(3))), padding: const EdgeInsets.symmetric(horizontal: 0.2 * kBaseFontSize), child: InlineContent( // If an @-mention is inside a link, let the @-mention override it. @@ -826,11 +829,6 @@ class UserMention extends StatelessWidget { nodes: node.nodes)); } - static get _kDecoration => BoxDecoration( - // TODO(#646) different background between direct and wildcard mentions - color: const HSLColor.fromAHSL(0.2, 240, 0.7, 0.7).toColor(), - borderRadius: const BorderRadius.all(Radius.circular(3))); - // This is a more literal translation of Zulip web's CSS. // But it turns out CSS `box-shadow` has a quirk we rely on there: // it doesn't apply under the element itself, even if the element's From cedbb081961ebc52614863ddde3043c7666991f9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 9 May 2024 17:11:26 -0600 Subject: [PATCH 5/5] msglist: Set content font color to match web With Chrome's web inspector and Mac's "Digital Color Meter", I verified that our color now matches web's color. Since GlobalTime's clock icon is a text-like element, change it to the new color too, and add a content test that makes sure the colors match each other, and match the DefaultTextStyle that content elements expect to inherit from. I didn't find any other text-like elements that need extra treatment like this. --- lib/widgets/content.dart | 5 ++++- lib/widgets/message_list.dart | 4 +--- test/widgets/content_test.dart | 23 +++++++++++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index 8dd6fc7d28..77fcc962da 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -915,7 +915,10 @@ class GlobalTime extends StatelessWidget { child: Row( mainAxisSize: MainAxisSize.min, children: [ - Icon(ZulipIcons.clock, size: surroundingFontSize), + Icon( + size: surroundingFontSize, + color: DefaultTextStyle.of(context).style.color!, + ZulipIcons.clock), // Ad-hoc spacing adjustment per feedback: // https://chat.zulip.org/#narrow/stream/101-design/topic/clock.20icons/near/1729345 const SizedBox(width: 1), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index f3c8b63ba0..b29026e9ae 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -267,9 +267,7 @@ class _MessageListState extends State with PerAccountStoreAwareStat if (!model!.fetched) return const Center(child: CircularProgressIndicator()); return DefaultTextStyle.merge( - // TODO figure out text color -- web is supposedly hsl(0deg 0% 20%), - // but seems much darker than that - style: const TextStyle(color: Color.fromRGBO(0, 0, 0, 1)), + style: TextStyle(color: const HSLColor.fromAHSL(1, 0, 0, 0.15).toColor()), // Pad the left and right insets, for small devices in landscape. child: SafeArea( // Don't let this be the place we pad the bottom inset. When there's diff --git a/test/widgets/content_test.dart b/test/widgets/content_test.dart index 4277416d73..ee325db131 100644 --- a/test/widgets/content_test.dart +++ b/test/widgets/content_test.dart @@ -614,6 +614,29 @@ void main() { tester.widget(find.textContaining(renderedTextRegexp)); }); + testWidgets('clock icon and text are the same color', (tester) async { + await tester.pumpWidget(MaterialApp(home: DefaultTextStyle( + style: const TextStyle(color: Colors.green), + child: BlockContentList(nodes: + parseContent('

$timeSpanHtml

').nodes), + ))); + + final icon = tester.widget( + find.descendant(of: find.byType(GlobalTime), + matching: find.byIcon(ZulipIcons.clock))); + + final textSpan = tester.renderObject( + find.descendant(of: find.byType(GlobalTime), + matching: find.textContaining(renderedTextRegexp) + )).text; + final textColor = mergedStyleOfSubstring(textSpan, renderedTextRegexp)!.color; + check(textColor).isNotNull(); + + check(icon).color + ..equals(textColor!) + ..equals(Colors.green); + }); + testWidgets('maintains font-size ratio with surrounding text', (tester) async { Future doCheck(double Function(GlobalTime widget) sizeFromWidget) async { await checkFontSizeRatio(tester,