Skip to content

compose: Refactor some logic; implement redesigned error banner #1090

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

Merged
merged 24 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5e011a8
compose [nfc]: Move "there is none" logic out of ComposeBox widget
chrisbobbe Nov 25, 2024
39781c8
msglist [nfc]: Remove no-op SizedBox.shrink as a Column child
chrisbobbe Nov 25, 2024
ba6424f
action_sheet test [nfc]: Add groups for 'in {topic,DM} narrow' tests
chrisbobbe Nov 26, 2024
060208f
action_sheet: Fix rare null-check error on quote-and-reply
chrisbobbe Nov 26, 2024
35c966e
compose test [nfc]: Make controllerKey a shared `late` variable
chrisbobbe Dec 2, 2024
b5e37d6
compose: Dispose topic-input's focus node when stream compose box dis…
chrisbobbe Dec 3, 2024
1e45ca2
compose [nfc]: Make state "has-a" instead of "is-a" ComposeBoxController
chrisbobbe Nov 26, 2024
b233ee9
compose [nfc]: Shorten {topic,content}Controller to just {topic,content}
chrisbobbe Nov 26, 2024
f0b0b5b
compose: Store controller on ComposeBox widget, not descendant
chrisbobbe Nov 26, 2024
69d5c61
compose [nfc]: s/_ComposeBoxLayout/_ComposeBoxBody/
chrisbobbe Nov 26, 2024
009e415
compose [nfc]: Give _ComposeBoxBody `narrow` and `controller` params
chrisbobbe Nov 26, 2024
d124755
compose [nfc]: Refactor _{Stream,FixedDestination}ComposeBoxBody
chrisbobbe Nov 26, 2024
6169311
compose [nfc]: Put {_Stream,FixedDestination}ComposeBoxBody near base…
chrisbobbe Nov 26, 2024
6d1f026
compose [nfc]: Bring _ComposeBoxContainer out of _ComposeBoxBody
chrisbobbe Nov 26, 2024
5b3ca51
compose: Redesign error banner that replaces the compose box
chrisbobbe Nov 26, 2024
11072ed
compose [nfc]: Add TODO for overlaid linear progress indicator
chrisbobbe Nov 26, 2024
a883762
compose [nfc]: Simplify choose-body logic by switching on controller
chrisbobbe Nov 26, 2024
0c5af34
compose [nfc]: Pass whole `controller` down (1/6); _StreamContentInput
chrisbobbe Dec 2, 2024
6bc6169
compose [nfc]: Pass `controller` down (2/6); _FixedDestinationContent…
chrisbobbe Dec 2, 2024
f65dc80
compose [nfc]: Pass `controller` down (3/6); _ContentInput
chrisbobbe Dec 2, 2024
95b77f6
compose [nfc]: Pass `controller` down (4/6); _TopicInput
chrisbobbe Dec 2, 2024
29470d3
compose [nfc]: Pass `controller` down (5/6); _SendButton
chrisbobbe Dec 2, 2024
e1fc763
compose [nfc]: Pass `controller` down (6/6); _AttachUploadsButton
chrisbobbe Dec 2, 2024
f46187a
compose [nfc]: Simplify controller getters to final fields
gnprice Dec 3, 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
29 changes: 15 additions & 14 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import '../model/narrow.dart';
import 'actions.dart';
import 'clipboard.dart';
import 'color.dart';
import 'compose_box.dart';
import 'dialog.dart';
import 'icons.dart';
import 'inset_shadow.dart';
Expand Down Expand Up @@ -325,22 +326,22 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
@override void onPressed() async {
final zulipLocalizations = ZulipLocalizations.of(pageContext);

// This will be null only if the compose box disappeared after the
// message action sheet opened, and before "Quote and reply" was pressed.
// Currently a compose box can't ever disappear, so this is impossible.
var composeBoxController = findMessageListPage().composeBoxController!;
Comment on lines -328 to -331
Copy link
Member

Choose a reason for hiding this comment

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

Convenient that we had these comments explaining why the ! had been appropriate in the first place. That particularly helps with the commit
cfe6b3a action_sheet: Fix rare null-check error on quote-and-reply

that points out it's no longer appropriate.

final topicController = composeBoxController.topicController;
var composeBoxController = findMessageListPage().composeBoxController;
// The compose box doesn't null out its controller; it's either always null
// (e.g. in Combined Feed) or always non-null; it can't have been nulled out
// after the action sheet opened.
composeBoxController!;
if (
topicController != null
&& topicController.textNormalized == kNoTopicTopic
composeBoxController is StreamComposeBoxController
&& composeBoxController.topic.textNormalized == kNoTopicTopic
&& message is StreamMessage
) {
topicController.value = TextEditingValue(text: message.topic);
composeBoxController.topic.value = TextEditingValue(text: message.topic);
}

// This inserts a "[Quoting…]" placeholder into the content input,
// giving the user a form of progress feedback.
final tag = composeBoxController.contentController
final tag = composeBoxController.content
.registerQuoteAndReplyStart(PerAccountStoreWidget.of(pageContext),
message: message,
);
Expand All @@ -353,11 +354,11 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {

if (!pageContext.mounted) return;

// This will be null only if the compose box disappeared during the
// quotation request. Currently a compose box can't ever disappear,
// so this is impossible.
composeBoxController = findMessageListPage().composeBoxController!;
composeBoxController.contentController
composeBoxController = findMessageListPage().composeBoxController;
// The compose box doesn't null out its controller; it's either always null
// (e.g. in Combined Feed) or always non-null; it can't have been nulled out
// during the raw-content request.
composeBoxController!.content
.registerQuoteAndReplyEnd(PerAccountStoreWidget.of(pageContext), tag,
message: message,
rawContent: rawContent,
Expand Down
Loading