Compose box redesign
Fixes: #915
(getting the screenshots ready)
The design specifies a 8px high padding before the message content:
The content can be scrolled past the padding, then clipped at the top border of the compose box:
This turned out to be tricky to implement, because InputDecorator's contentPadding also pads the scrollable view, and the content cannot be scrolled past the padding, leaving a gap:
| with padding | without padding |
|---|---|
A previous workaround was to fully vertically expand TextField by wrapping it within a SingleChildScrollView (implemented in e16251febac9b000c50969845025bc4a1b54e67a), which does not handle the case that the cursor gets scrolled out of view when editing:
Example
The latest workaround disables clipping on the TextField and wraps it with a ClipRect, whose size is determined by a ConstrainedBox under our control.
I think I have found a true simple solution to this: set TextField.clipBehavior to Clip.none, set InputDecoration.contentPadding and wrap the TextField with a ClipRect under our control. This seems to work perfectly without the cursor issue.
Preview
| light | dark |
|---|---|
Updated the PR with the screenshots. Ready for @chrisbobbe and @alya to review!
I haven't compared carefully vs. the designs in Figma, but the screenshots look nice!
Thanks for the previous comments @chrisbobbe! This is ready for review now.
This is ready for review now (forgot to comment on that).
Extracted InsetShadowBox so that #853 can reuse this.
~~Will push an update here soon.~~
Ready for review!
Cool, reviewing now.
Thanks. The PR is ready for review again!
Thanks for the review! I have updated the PR.
The last three commits are for the proof-of-concept tests for the content input scaling feature; they are optional:
- 3e282e4b wip; compose_box content input text scaling test
- 81b27f55 compose_box test: Set topic and content through a helper (cherry-picked from #1033)
- 93d5f999 compose_box test: Use a layout similar to the message list page (#1034)
Not sure if this aspect of the UI requires its own set of tests verifying the maximum number of lines to display. It basically verifies that the max height sticks to a certain number, and that the maximum number of visible lines matches where we expected from the figma design.
Thanks for the review! I have updated the PR.
There is another revision on top of this that I didn't push, because it reorganizes the commits (even though the diff with all commits combined is identical to this one), which might make verifying the update a bit more difficult.
tools/check is failing at this commit:
compose_box: Use new icons from the redesign
00:19 +1525 ~166: /Users/chrisbobbe/dev/zulip-flutter/test/widgets/compose_box_test.dart: ComposeBox typing notices hitting send button sends a "typing stopped" notice
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
The finder "Found 0 widgets with icon "IconData(U+0E571)": []" (used in a call to "tap()") could not
find any matching widgets.
When the exception was thrown, this was the stack:
#0 WidgetController._getElementPoint (package:flutter_test/src/controller.dart:1895:7)
#1 WidgetController.getCenter (package:flutter_test/src/controller.dart:1795:12)
#2 WidgetController.tap (package:flutter_test/src/controller.dart:1043:18)
#3 main.<anonymous closure>.<anonymous closure> (file:///Users/chrisbobbe/dev/zulip-flutter/test/widgets/compose_box_test.dart:281:20)
<asynchronous suspension>
#4 testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:189:15)
<asynchronous suspension>
#5 TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1027:5)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)
The test description was:
hitting send button sends a "typing stopped" notice
════════════════════════════════════════════════════════════════════════════════════════════════════
00:19 +1525 ~166 -1: /Users/chrisbobbe/dev/zulip-flutter/test/widgets/compose_box_test.dart: ComposeBox typing notices hitting send button sends a "typing stopped" notice [E]
Test failed. See exception logs above.
The test description was: hitting send button sends a "typing stopped" notice
Pushed to fix this. Thanks!
Added:
- eaed8fba emoji: Use withFadedAlpha for highlightColor
Screenshots (highlighted reactions)
| before | after |
|---|---|
- b60f13c6 color: Add ColorExtension.withFadedAlpha
There are some remaining callers of withValues:
Details
$ git grep withValues
lib/widgets/channel_colors.dart: .withValues(alpha: 0.3),
lib/widgets/channel_colors.dart: .withValues(alpha: 0.3),
lib/widgets/color.dart: return withValues(alpha: a * factor);
lib/widgets/content.dart: colorThematicBreak: const HSLColor.fromAHSL(1, 0, 0, .87).toColor().withValues(alpha: 0.2),
lib/widgets/emoji_reaction.dart: borderSelected: Colors.black.withValues(alpha: 0.45),
lib/widgets/emoji_reaction.dart: borderUnselected: Colors.black.withValues(alpha: 0.05),
lib/widgets/emoji_reaction.dart: bgSelected: Colors.black.withValues(alpha: 0.8),
lib/widgets/emoji_reaction.dart: bgUnselected: Colors.black.withValues(alpha: 0.3),
lib/widgets/emoji_reaction.dart: borderSelected: Colors.white.withValues(alpha: 0.75),
lib/widgets/emoji_reaction.dart: borderUnselected: Colors.white.withValues(alpha: 0.15),
lib/widgets/emoji_reaction.dart: textSelected: Colors.white.withValues(alpha: 0.85),
lib/widgets/emoji_reaction.dart: textUnselected: Colors.white.withValues(alpha: 0.75),
lib/widgets/inset_shadow.dart: colors: [color, color.withValues(alpha: 0)]));
lib/widgets/lightbox.dart: final appBarBackgroundColor = Colors.grey.shade900.withValues(alpha: 0.87);
lib/widgets/message_list.dart: streamRecipientHeaderChevronRight: Colors.black.withValues(alpha: 0.3),
lib/widgets/message_list.dart: unreadMarkerGap: Colors.white.withValues(alpha: 0.6),
lib/widgets/message_list.dart: streamRecipientHeaderChevronRight: Colors.white.withValues(alpha: 0.3),
lib/widgets/theme.dart: bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15),
lib/widgets/theme.dart: borderBar: Colors.black.withValues(alpha: 0.2),
lib/widgets/theme.dart: editorButtonPressedBg: Colors.black.withValues(alpha: 0.06),
lib/widgets/theme.dart: groupDmConversationIcon: Colors.black.withValues(alpha: 0.5),
lib/widgets/theme.dart: modalBarrierColor: const Color(0xff000000).withValues(alpha: 0.3),
lib/widgets/theme.dart: unreadCountBadgeTextForChannel: Colors.black.withValues(alpha: 0.9),
lib/widgets/theme.dart: bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37),
lib/widgets/theme.dart: borderBar: Colors.black.withValues(alpha: 0.5),
lib/widgets/theme.dart: contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75),
lib/widgets/theme.dart: editorButtonPressedBg: Colors.white.withValues(alpha: 0.06),
lib/widgets/theme.dart: labelCounterUnread: const Color(0xffffffff).withValues(alpha: 0.7),
lib/widgets/theme.dart: labelMenuButton: const Color(0xffffffff).withValues(alpha: 0.85),
lib/widgets/theme.dart: textInput: const Color(0xffffffff).withValues(alpha: 0.9),
lib/widgets/theme.dart: groupDmConversationIcon: Colors.white.withValues(alpha: 0.5),
lib/widgets/theme.dart: modalBarrierColor: const Color(0xff000000).withValues(alpha: 0.5),
lib/widgets/theme.dart: unreadCountBadgeTextForChannel: Colors.white.withValues(alpha: 0.9),
test/widgets/color_test.dart: .isSameColorAs(color.withValues(alpha: 0.25));
test/widgets/color_test.dart: .isSameColorAs(color.withValues(alpha: 0.5));
Most of them, other than the ones in lib/widgets/channel_colors.dart are from themes defining the colors. The obvious ones are handled in the new commits, and we can do a full sweep in a follow-up PR.
I also noticed that the text scale tests stop being reliable when the text scale factor is lower than or equal to 0.5, reporting that we get less than 8 fully or partially visible lines. However, that doesn't happen when I run flutter run test/widgets/compose_box_test.dart, which is puzzling.
I also noticed that the text scale tests stop being reliable when the text scale factor is lower than or equal to 0.5
Odd. But iOS at least doesn't let you choose a factor that small. 🤷♂️ We've written down some of iOS's options in test/widgets/text_test.dart:
// From trying the options on an iPhone 13 Pro running iOS 16.6.1:
const kTextScaleFactors = <double>[
0.8235, // smallest
1,
1.3529, // largest without using the "Larger Accessibility Sizes" setting
3.1176, // largest
];
Thanks! Updated the PR.
I have pushed a change reorganizing the commits for better readability. This is effectively an NFC change overall — no functionality change after reordering.
Before 66d55db6 theme: Update borderBar to match Figma design 6dd42643 compose_box: Use new icons from the redesign f52104a3 compose_box: Use new send button color, shape and icon b13840c3 compose_box [nfc]: Refactor away shared iconTheme override d18ec182 fix link; compose_box: Support new compose box layout d5fa7f93 compose_box: Support redesigned button feedback 23ef514f color [nfc]: Add ColorExtension.withFadedAlpha a29e69b5 emoji: Use withFadedAlpha for highlightColor 00386935 compose_box: Implement topic input redesign 6df7931e compose_box: Implement content input redesign f8f97dd7 optional; compose_box content input text scaling test
After prep changes 13ab190c color [nfc]: Add ColorExtension.withFadedAlpha ef6cf91b emoji: Use withFadedAlpha for highlightColor ce761fdf theme: Update borderBar to match Figma design a3da9918 compose_box [nfc]: Refactor away shared iconTheme override 2780d25f compose_box: Use new icons from the redesign main redesign changes 17372299 fix link; compose_box: Support new compose box layout 455a28ec compose_box: Use new send button color, shape and icon 70376681 compose_box: Support redesigned button feedback 61904987 compose_box: Implement input redesign cbac604e optional; compose_box content input text scaling test
Changes
-
Moved 23ef514f color [nfc]: Add ColorExtension.withFadedAlpha and a29e69b5 emoji: Use withFadedAlpha for highlightColor to the very front of the sequence. They no longer touch the colors that use
withValuesinlib/widgets/compose_box.dart, leaving them to the later commits that implement the UI changes. -
Grouped the lighter commits together as prep changes: 17372299 fix link; compose_box: Support new compose box layout 455a28ec compose_box: Use new send button color, shape and icon 70376681 compose_box: Support redesigned button feedback 61904987 compose_box: Implement input redesign cbac604e (HEAD -> main, me/dev-compose) optional; compose_box content input text scaling test These commits barely requires checking the Figma redesign, other than confirming the icons
-
Moved f52104a3 compose_box: Use new send button color, shape and icon after d18ec182 fix link; compose_box: Support new compose box layout so that the order is more natural, simplifying the diffs for changes relevant to the send button.
-
Combined 00386935 compose_box: Implement topic input redesign and 6df7931e compose_box: Implement content input redesign into 61904987 compose_box: Implement input redesign
Cool, thanks for the update! Reviewing again now.
Moving this to integration review since I'm about to be on vacation.
Thanks for the review! I have updated the PR.
One commit-message nit: use "compose" for the prefix rather than "compose_box"
Ah, I see we raced and you've already made that change :slightly_smiling_face:
Thanks! Updated the PR.
Moved the links to the paragraph they each correspond to, and added the comment on ClipRect (the related paragraph is still kept in the commit, in case the reader is looking back at this for a more verbose explanation on this design's origin).
Looks good — merging. Thanks again for all your work on this!