Skip to content

compose: Use "info" intent, not "danger", for cannot-post-here banners #1871

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

Merged
merged 4 commits into from
Oct 1, 2025
Merged

compose: Use "info" intent, not "danger", for cannot-post-here banners #1871

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f65e59
compose [nfc]: Factor banner background/label colors as `intent` enum
chrisbobbe Sep 24, 2025
f133805
compose [nfc]: Move PageRoot.contextOf() down into a _Banner.buildTra…
chrisbobbe Sep 29, 2025
0f40879
compose [nfc]: Factor _Banner as a concrete widget class with params
chrisbobbe Sep 24, 2025
38a2c63
compose: Use "info" intent, not "danger", for cannot-post-here banners
chrisbobbe Sep 24, 2025
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
138 changes: 64 additions & 74 deletions lib/widgets/compose_box.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,49 +1724,62 @@ class EditMessageComposeBoxController extends ComposeBoxController {
/// A banner to display over or instead of interactive compose-box content.
///
/// Must have a [PageRoot] ancestor.
abstract class _Banner extends StatelessWidget {
const _Banner();
class _Banner extends StatelessWidget {
const _Banner({
required this.intent,
required this.label,
this.trailing,
this.padEnd = true, // ignore: unused_element_parameter
});

String getLabel(ZulipLocalizations zulipLocalizations);
Color getLabelColor(DesignVariables designVariables);
Color getBackgroundColor(DesignVariables designVariables);
final _BannerIntent intent;
final String label;

/// A trailing element, with vertical but not horizontal outer padding
/// An optional trailing element.
///
/// It should include vertical but not horizontal outer padding
/// for spacing/positioning.
///
/// An interactive element's touchable area should have height at least 44px,
/// with some of that as "slop" vertical outer padding above and below
/// what gets painted:
/// https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023907300
///
/// To control the element's distance from the end edge, override [padEnd].
///
/// The passed [BuildContext] will be the result of [PageRoot.contextOf],
/// so it's expected to remain mounted until the whole page disappears,
/// which may be long after the banner disappears.
Widget? buildTrailing(BuildContext pageContext);
/// To control the element's distance from the end edge, use [padEnd].
// An "x" button could go here.
// 24px square with 8px touchable padding in all directions?
// and `padEnd: false`; see Figma:
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-17029&m=dev
final Widget? trailing;

/// Whether to apply `end: 8` in [SafeArea.minimum].
///
/// Subclasses can use `false` when the [buildTrailing] element
/// Pass `false` when the [trailing] element
/// is meant to abut the edge of the screen
/// in the common case that there are no horizontal device insets.
bool get padEnd => true;
///
/// Defaults to `true`.
final bool padEnd;

@override
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
final designVariables = DesignVariables.of(context);

final (labelColor, backgroundColor) = switch (intent) {
_BannerIntent.info =>
(designVariables.bannerTextIntInfo, designVariables.bannerBgIntInfo),
_BannerIntent.danger =>
(designVariables.btnLabelAttMediumIntDanger, designVariables.bannerBgIntDanger),
};

final labelTextStyle = TextStyle(
fontSize: 17,
height: 22 / 17,
color: getLabelColor(designVariables),
color: labelColor,
).merge(weightVariableTextStyle(context, wght: 600));

final trailing = buildTrailing(PageRoot.contextOf(context));
return DecoratedBox(
decoration: BoxDecoration(
color: getBackgroundColor(designVariables)),
decoration: BoxDecoration(color: backgroundColor),
child: SafeArea(
minimum: EdgeInsetsDirectional.only(start: 8, end: padEnd ? 8 : 0)
// (SafeArea.minimum doesn't take an EdgeInsetsDirectional)
Expand All @@ -1781,61 +1794,30 @@ abstract class _Banner extends StatelessWidget {
child: Text(
style: labelTextStyle,
textScaler: MediaQuery.textScalerOf(context).clamp(maxScaleFactor: 1.5),
getLabel(zulipLocalizations)))),
label))),
if (trailing != null) ...[
const SizedBox(width: 8),
trailing,
trailing!,
],
]))));
}
}

class _ErrorBanner extends _Banner {
const _ErrorBanner({
required String Function(ZulipLocalizations) getLabel,
}) : _getLabel = getLabel;

@override
String getLabel(ZulipLocalizations zulipLocalizations) =>
_getLabel(zulipLocalizations);
final String Function(ZulipLocalizations) _getLabel;

@override
Color getLabelColor(DesignVariables designVariables) =>
designVariables.btnLabelAttMediumIntDanger;

@override
Color getBackgroundColor(DesignVariables designVariables) =>
designVariables.bannerBgIntDanger;

@override
Widget? buildTrailing(pageContext) {
// An "x" button can go here.
// 24px square with 8px touchable padding in all directions?
// and `bool get padEnd => false`; see Figma:
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-17029&m=dev
return null;
}
enum _BannerIntent {
info,
danger,
}

class _EditMessageBanner extends _Banner {
const _EditMessageBanner({required this.composeBoxState});
class _EditMessageBannerTrailing extends StatelessWidget {
const _EditMessageBannerTrailing({required this.composeBoxState});

final ComposeBoxState composeBoxState;

@override
String getLabel(ZulipLocalizations zulipLocalizations) =>
zulipLocalizations.composeBoxBannerLabelEditMessage;

@override
Color getLabelColor(DesignVariables designVariables) =>
designVariables.bannerTextIntInfo;
void _handleTapSave (BuildContext context) async {
// (A BuildContext that's expected to remain mounted until the whole page
// disappears, which may be long after the banner disappears.)
final pageContext = PageRoot.contextOf(context);

@override
Color getBackgroundColor(DesignVariables designVariables) =>
designVariables.bannerBgIntInfo;

void _handleTapSave (BuildContext pageContext) async {
final store = PerAccountStoreWidget.of(pageContext);
final controller = composeBoxState.controller;
if (controller is! EditMessageComposeBoxController) return; // TODO(log)
Expand Down Expand Up @@ -1882,16 +1864,16 @@ class _EditMessageBanner extends _Banner {
}

@override
Widget buildTrailing(pageContext) {
final zulipLocalizations = ZulipLocalizations.of(pageContext);
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
return Row(mainAxisSize: MainAxisSize.min, spacing: 8, children: [
ZulipWebUiKitButton(label: zulipLocalizations.composeBoxBannerButtonCancel,
onPressed: composeBoxState.endEditInteraction),
// TODO(#1481) disabled appearance when there are validation errors
// or the original raw content hasn't loaded yet
ZulipWebUiKitButton(label: zulipLocalizations.composeBoxBannerButtonSave,
attention: ZulipWebUiKitButtonAttention.high,
onPressed: () => _handleTapSave(pageContext)),
onPressed: () => _handleTapSave(context)),
]);
}
}
Expand Down Expand Up @@ -2146,25 +2128,28 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
super.dispose();
}

/// An [_ErrorBanner] that replaces the compose box's text inputs.
Widget? _errorBannerComposingNotAllowed(BuildContext context) {
/// A [_Banner] that replaces the compose box's text inputs.
Widget? _bannerComposingNotAllowed(BuildContext context) {
final store = PerAccountStoreWidget.of(context);
final zulipLocalizations = ZulipLocalizations.of(context);
switch (widget.narrow) {
case ChannelNarrow(:final streamId):
case TopicNarrow(:final streamId):
final channel = store.streams[streamId];
if (channel == null || !store.selfCanSendMessage(inChannel: channel,
byDate: DateTime.now())) {
return _ErrorBanner(getLabel: (zulipLocalizations) =>
zulipLocalizations.errorBannerCannotPostInChannelLabel);
return _Banner(
intent: _BannerIntent.info,
label: zulipLocalizations.errorBannerCannotPostInChannelLabel);
Comment on lines -2158 to +2143
Copy link
Member

Choose a reason for hiding this comment

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

Neat, I like that the label is now found in a more straightforward way.

}

case DmNarrow(:final otherRecipientIds):
final hasDeactivatedUser = otherRecipientIds.any((id) =>
!(store.getUser(id)?.isActive ?? true));
if (hasDeactivatedUser) {
return _ErrorBanner(getLabel: (zulipLocalizations) =>
zulipLocalizations.errorBannerDeactivatedDmLabel);
return _Banner(
intent: _BannerIntent.info,
label: zulipLocalizations.errorBannerDeactivatedDmLabel);
}

case CombinedFeedNarrow():
Expand All @@ -2178,10 +2163,12 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM

@override
Widget build(BuildContext context) {
final errorBanner = _errorBannerComposingNotAllowed(context);
if (errorBanner != null) {
final zulipLocalizations = ZulipLocalizations.of(context);

final bannerComposingNotAllowed = _bannerComposingNotAllowed(context);
if (bannerComposingNotAllowed != null) {
return ComposeBoxInheritedWidget.fromComposeBoxState(this,
child: _ComposeBoxContainer(body: null, banner: errorBanner));
child: _ComposeBoxContainer(body: null, banner: bannerComposingNotAllowed));
}

final Widget? body;
Expand All @@ -2200,7 +2187,10 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
}
case EditMessageComposeBoxController(): {
body = _EditMessageComposeBoxBody(controller: controller, narrow: narrow);
banner = _EditMessageBanner(composeBoxState: this);
banner = _Banner(
intent: _BannerIntent.info,
label: zulipLocalizations.composeBoxBannerLabelEditMessage,
trailing: _EditMessageBannerTrailing(composeBoxState: this));
}
}

Expand Down