From 5da93b2a22f81b5da31bc4cadab05caf1b302cdd Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 17:22:39 -0700 Subject: [PATCH 1/9] action_sheet [nfc]: Move an 8px top padding into BottomSheetHeader --- lib/widgets/action_sheet.dart | 4 +--- lib/widgets/emoji_reaction.dart | 6 ++---- lib/widgets/read_receipts.dart | 8 +++----- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 59be92482f..88576f7691 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -114,8 +114,6 @@ typedef WidgetBuilderFromTextStyle = Widget Function(TextStyle); /// The "build" params support richer content, such as [TextWithLink], /// and the callback is passed a [TextStyle] which is the base style. /// -/// Assumes 8px padding below the top of the bottom sheet. -/// /// Figma; just message no title: /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3481-26993&m=dev /// @@ -171,7 +169,7 @@ class BottomSheetHeader extends StatelessWidget { }; return Padding( - padding: EdgeInsets.fromLTRB(16, 8, 16, 4), + padding: EdgeInsets.fromLTRB(16, 16, 16, 4), child: Column( crossAxisAlignment: CrossAxisAlignment.stretch, spacing: 8, diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index 2c7c5d61ab..a26f7866e4 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -804,10 +804,8 @@ class ViewReactionsHeader extends StatelessWidget { final reactions = message?.reactions; if (reactions == null || reactions.aggregated.isEmpty) { - return Padding( - padding: const EdgeInsets.only(top: 8), - child: BottomSheetHeader(message: zulipLocalizations.seeWhoReactedSheetNoReactions), - ); + return BottomSheetHeader( + message: zulipLocalizations.seeWhoReactedSheetNoReactions); } return Padding( diff --git a/lib/widgets/read_receipts.dart b/lib/widgets/read_receipts.dart index 46c920372a..f00bca43f7 100644 --- a/lib/widgets/read_receipts.dart +++ b/lib/widgets/read_receipts.dart @@ -128,11 +128,9 @@ class _ReadReceiptsHeader extends StatelessWidget { markup: zulipLocalizations.actionSheetReadReceiptsReadCount(receiptCount)); } - return Padding( - padding: const EdgeInsets.only(top: 8), - child: BottomSheetHeader( - title: zulipLocalizations.actionSheetReadReceipts, - buildMessage: headerMessageBuilder)); + return BottomSheetHeader( + title: zulipLocalizations.actionSheetReadReceipts, + buildMessage: headerMessageBuilder); } } From 3f8f832a95891ccc050f42d7b9bbface6c489c77 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 16:23:00 -0700 Subject: [PATCH 2/9] action_sheet [nfc]: Dartdoc _showActionSheet --- lib/widgets/action_sheet.dart | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 88576f7691..479bf92fb5 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -35,11 +35,25 @@ import 'text.dart'; import 'theme.dart'; import 'topic_list.dart'; +/// Show an action sheet with scrollable menu buttons +/// and an optional scrollable header. +/// +/// [header] should not use vertical padding to position itself on the sheet. +/// It will be wrapped in vertical padding, +/// a scroll view, and an [InsetShadowBox] to support scrolling. void _showActionSheet( BuildContext pageContext, { Widget? header, required List> buttonSections, }) { + // This assert does look absurd, but see dartdoc -- [BottomSheetHeader] adds + // vertical padding to position itself on the sheet, so isn't suitable here. + // When it grows a param to omit that padding, soon, it'll be usable here. + // (Currently the only caller that passes `header` is the message action sheet, + // and that header widget only adds internal padding, on a distinct-colored + // surface, not padding for positioning.) + assert(header is! BottomSheetHeader); + // Could omit this if we need _showActionSheet outside a per-account context. final accountId = PerAccountStoreWidget.accountIdOf(pageContext); From 7d985eaf74b8965d1354af2491e83543a0b119ab Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 17:37:54 -0700 Subject: [PATCH 3/9] action_sheet: Add ActionSheetHeader.outerVerticalPadding --- lib/widgets/action_sheet.dart | 25 ++++++++++++++++--------- lib/widgets/emoji_reaction.dart | 1 + lib/widgets/read_receipts.dart | 1 + 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 479bf92fb5..345ef3e080 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -46,13 +46,7 @@ void _showActionSheet( Widget? header, required List> buttonSections, }) { - // This assert does look absurd, but see dartdoc -- [BottomSheetHeader] adds - // vertical padding to position itself on the sheet, so isn't suitable here. - // When it grows a param to omit that padding, soon, it'll be usable here. - // (Currently the only caller that passes `header` is the message action sheet, - // and that header widget only adds internal padding, on a distinct-colored - // surface, not padding for positioning.) - assert(header is! BottomSheetHeader); + assert(header is! BottomSheetHeader || !header.outerVerticalPadding); // Could omit this if we need _showActionSheet outside a per-account context. final accountId = PerAccountStoreWidget.accountIdOf(pageContext); @@ -128,6 +122,9 @@ typedef WidgetBuilderFromTextStyle = Widget Function(TextStyle); /// The "build" params support richer content, such as [TextWithLink], /// and the callback is passed a [TextStyle] which is the base style. /// +/// To add outer vertical padding to position the header on the sheet, +/// pass true for [outerVerticalPadding]. +/// /// Figma; just message no title: /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3481-26993&m=dev /// @@ -145,6 +142,7 @@ class BottomSheetHeader extends StatelessWidget { this.buildTitle, this.message, this.buildMessage, + this.outerVerticalPadding = false, }) : assert(message == null || buildMessage == null), assert(title == null || buildTitle == null), assert((message != null || buildMessage != null) @@ -154,6 +152,7 @@ class BottomSheetHeader extends StatelessWidget { final Widget Function(TextStyle)? buildTitle; final String? message; final Widget Function(TextStyle)? buildMessage; + final bool outerVerticalPadding; @override Widget build(BuildContext context) { @@ -182,12 +181,20 @@ class BottomSheetHeader extends StatelessWidget { _ => null, }; - return Padding( - padding: EdgeInsets.fromLTRB(16, 16, 16, 4), + Widget result = Padding( + padding: EdgeInsets.symmetric(horizontal: 16), child: Column( crossAxisAlignment: CrossAxisAlignment.stretch, spacing: 8, children: [?effectiveTitle, ?effectiveMessage])); + + if (outerVerticalPadding) { + result = Padding( + padding: EdgeInsets.only(top: 16, bottom: 4), + child: result); + } + + return result; } } diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index a26f7866e4..45131d7ad0 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -805,6 +805,7 @@ class ViewReactionsHeader extends StatelessWidget { if (reactions == null || reactions.aggregated.isEmpty) { return BottomSheetHeader( + outerVerticalPadding: true, message: zulipLocalizations.seeWhoReactedSheetNoReactions); } diff --git a/lib/widgets/read_receipts.dart b/lib/widgets/read_receipts.dart index f00bca43f7..8416d78c85 100644 --- a/lib/widgets/read_receipts.dart +++ b/lib/widgets/read_receipts.dart @@ -129,6 +129,7 @@ class _ReadReceiptsHeader extends StatelessWidget { } return BottomSheetHeader( + outerVerticalPadding: true, title: zulipLocalizations.actionSheetReadReceipts, buildMessage: headerMessageBuilder); } From 9651e4186e2beacdc4ae0a49acb4ba25ab9a3d01 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 16:34:20 -0700 Subject: [PATCH 4/9] action_sheet [nfc]: Give action-sheet scrolling code more breathing room --- lib/widgets/action_sheet.dart | 78 +++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 345ef3e080..f1e5fb553a 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -61,6 +61,46 @@ void _showActionSheet( isScrollControlled: true, builder: (BuildContext _) { final designVariables = DesignVariables.of(pageContext); + + Widget? effectiveHeader; + if (header != null) { + effectiveHeader = Flexible( + // TODO(upstream) Enforce a flex ratio (e.g. 1:3) + // only when the header height plus the buttons' height + // exceeds available space. Otherwise let one or the other + // grow to fill available space even if it breaks the ratio. + // Needs support for separate properties like `flex-grow` + // and `flex-shrink`. + flex: 1, + child: InsetShadowBox( + top: 8, bottom: 8, + color: designVariables.bgContextMenu, + child: SingleChildScrollView( + padding: EdgeInsets.symmetric(vertical: 8), + child: header))); + } + + final body = Flexible( + flex: 3, + child: Padding( + padding: const EdgeInsets.fromLTRB(16, 0, 16, 0), + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + mainAxisSize: MainAxisSize.min, + children: [ + Flexible(child: InsetShadowBox( + top: 8, bottom: 8, + color: designVariables.bgContextMenu, + child: SingleChildScrollView( + padding: const EdgeInsets.symmetric(vertical: 8), + child: Column( + mainAxisSize: MainAxisSize.min, + spacing: 8, + children: buttonSections.map((buttons) => + MenuButtonsShape(buttons: buttons)).toList())))), + const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.cancel), + ]))); + return PerAccountStoreWidget( accountId: accountId, child: Semantics( @@ -70,43 +110,11 @@ void _showActionSheet( child: Column( mainAxisSize: MainAxisSize.min, children: [ - if (header != null) - Flexible( - // TODO(upstream) Enforce a flex ratio (e.g. 1:3) - // only when the header height plus the buttons' height - // exceeds available space. Otherwise let one or the other - // grow to fill available space even if it breaks the ratio. - // Needs support for separate properties like `flex-grow` - // and `flex-shrink`. - flex: 1, - child: InsetShadowBox( - top: 8, bottom: 8, - color: designVariables.bgContextMenu, - child: SingleChildScrollView( - padding: EdgeInsets.symmetric(vertical: 8), - child: header))) + if (effectiveHeader != null) + effectiveHeader else SizedBox(height: 8), - Flexible( - flex: 3, - child: Padding( - padding: const EdgeInsets.fromLTRB(16, 0, 16, 0), - child: Column( - crossAxisAlignment: CrossAxisAlignment.stretch, - mainAxisSize: MainAxisSize.min, - children: [ - Flexible(child: InsetShadowBox( - top: 8, bottom: 8, - color: designVariables.bgContextMenu, - child: SingleChildScrollView( - padding: const EdgeInsets.symmetric(vertical: 8), - child: Column( - mainAxisSize: MainAxisSize.min, - spacing: 8, - children: buttonSections.map((buttons) => - MenuButtonsShape(buttons: buttons)).toList())))), - const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.cancel), - ]))), + body, ])))); }); } From 8d43acff219d8737ef5c49a96ca1527e37d2b05e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 16:51:17 -0700 Subject: [PATCH 5/9] action_sheet: Add `headerScrollable` param to _showActionSheet --- lib/widgets/action_sheet.dart | 41 ++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index f1e5fb553a..0491eeeee8 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -39,11 +39,12 @@ import 'topic_list.dart'; /// and an optional scrollable header. /// /// [header] should not use vertical padding to position itself on the sheet. -/// It will be wrapped in vertical padding, -/// a scroll view, and an [InsetShadowBox] to support scrolling. +/// It will be wrapped in vertical padding +/// and, if [headerScrollable], a scroll view and an [InsetShadowBox]. void _showActionSheet( BuildContext pageContext, { Widget? header, + bool headerScrollable = true, required List> buttonSections, }) { assert(header is! BottomSheetHeader || !header.outerVerticalPadding); @@ -64,24 +65,30 @@ void _showActionSheet( Widget? effectiveHeader; if (header != null) { - effectiveHeader = Flexible( - // TODO(upstream) Enforce a flex ratio (e.g. 1:3) - // only when the header height plus the buttons' height - // exceeds available space. Otherwise let one or the other - // grow to fill available space even if it breaks the ratio. - // Needs support for separate properties like `flex-grow` - // and `flex-shrink`. - flex: 1, - child: InsetShadowBox( - top: 8, bottom: 8, - color: designVariables.bgContextMenu, - child: SingleChildScrollView( - padding: EdgeInsets.symmetric(vertical: 8), - child: header))); + effectiveHeader = headerScrollable + ? Flexible( + // TODO(upstream) Enforce a flex ratio (e.g. 1:3) + // only when the header height plus the buttons' height + // exceeds available space. Otherwise let one or the other + // grow to fill available space even if it breaks the ratio. + // Needs support for separate properties like `flex-grow` + // and `flex-shrink`. + flex: 1, + child: InsetShadowBox( + top: 8, bottom: 8, + color: designVariables.bgContextMenu, + child: SingleChildScrollView( + padding: EdgeInsets.symmetric(vertical: 8), + child: header))) + : Padding( + padding: EdgeInsets.only(top: 16, bottom: 4), + child: header); } final body = Flexible( - flex: 3, + flex: (effectiveHeader != null && headerScrollable) + ? 3 + : 1, child: Padding( padding: const EdgeInsets.fromLTRB(16, 0, 16, 0), child: Column( From 6795e9649a74cb007e0e62ba9c5392fe8958eebb Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 6 Oct 2025 17:59:11 -0700 Subject: [PATCH 6/9] action_sheet test: Support channel action sheet tests with unknown channel --- test/widgets/action_sheet_test.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 06e0eab5ca..2c055f7596 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -255,19 +255,21 @@ void main() { await tester.pump(const Duration(milliseconds: 250)); } - Future showFromTopicListAppBar(WidgetTester tester) async { + Future showFromTopicListAppBar(WidgetTester tester, {int? streamId}) async { + streamId ??= someChannel.streamId; final transitionDurationObserver = TransitionDurationObserver(); connection.prepare(json: GetStreamTopicsResult(topics: []).toJson()); await tester.pumpWidget(TestZulipApp( navigatorObservers: [transitionDurationObserver], accountId: eg.selfAccount.id, - child: TopicListPage(streamId: someChannel.streamId))); + child: TopicListPage(streamId: streamId))); await tester.pump(); + final titleText = store.streams[streamId]?.name ?? '(unknown channel)'; await tester.longPress(find.descendant( of: find.byType(ZulipAppBar), - matching: find.text(someChannel.name))); + matching: find.text(titleText))); await transitionDurationObserver.pumpPastTransition(tester); } From 1a43fe245702593592b17bd647d1c477092bc7d9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 17:53:14 -0700 Subject: [PATCH 7/9] action_sheet: Show channel name in channel action sheet This excludes the channel description for now; that's #1896. Fixes-partly: #1533 --- lib/widgets/action_sheet.dart | 16 +++- lib/widgets/text.dart | 139 ++++++++++++++++++++++++++++ test/api/model/model_checks.dart | 3 + test/widgets/action_sheet_test.dart | 56 +++++++++++ 4 files changed, 213 insertions(+), 1 deletion(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 0491eeeee8..7b3f192776 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -502,7 +502,21 @@ void showChannelActionSheet(BuildContext context, { [UnsubscribeButton(pageContext: pageContext, channelId: channelId)], ]; - _showActionSheet(pageContext, buttonSections: buttonSections); + final header = BottomSheetHeader( + buildTitle: (baseStyle) => Text.rich( + style: baseStyle, + channelTopicLabelSpan( + context: context, + channelId: channelId, + fontSize: baseStyle.fontSize!, + color: baseStyle.color!)), + // TODO(#1896) show channel description + ); + + _showActionSheet(pageContext, + header: header, + headerScrollable: false, + buttonSections: buttonSections); } class SubscribeButton extends ActionSheetMenuItemButton { diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 3fe48484e3..577b6cdbb8 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -4,6 +4,9 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; +import '../generated/l10n/zulip_localizations.dart'; +import 'icons.dart'; +import 'store.dart'; import 'theme.dart'; /// An app-wide [Typography] for Zulip, customized from the Material default. @@ -533,3 +536,139 @@ class _TextWithLinkState extends State { span); } } + +/// Data to size and position a square icon in a span of text. +class InlineIconGeometryData { + /// What size the icon should be, + /// as a fraction of the surrounding text's font size. + final double sizeFactor; + + /// Where to assign the icon's baseline, as a fraction of the icon's size, + /// when the span is rendered with [TextBaseline.alphabetic]. + /// + /// This is ignored when the span is rendered with [TextBaseline.ideographic]; + /// zero is used instead. + final double alphabeticBaselineFactor; + + /// How much horizontal padding should separate the icon from surrounding text, + /// as a fraction of the icon's size. + final double paddingFactor; + + const InlineIconGeometryData._({ + required this.sizeFactor, + required this.alphabeticBaselineFactor, + required this.paddingFactor, + }); + + factory InlineIconGeometryData.forIcon(IconData icon) { + final result = _inlineIconGeometries[icon]; + assert(result != null); + return result ?? _defaultGeometry; + } + + // Values are ad hoc unless otherwise specified. + static final Map _inlineIconGeometries = { + ZulipIcons.globe: InlineIconGeometryData._( + sizeFactor: 0.8, + alphabeticBaselineFactor: 1 / 8, + paddingFactor: 1 / 4), + + ZulipIcons.hash_sign: InlineIconGeometryData._( + sizeFactor: 0.8, + alphabeticBaselineFactor: 1 / 16, + paddingFactor: 1 / 4), + + ZulipIcons.lock: InlineIconGeometryData._( + sizeFactor: 0.8, + alphabeticBaselineFactor: 1 / 16, + paddingFactor: 1 / 4), + }; + + static final _defaultGeometry = InlineIconGeometryData._( + sizeFactor: 0.8, + alphabeticBaselineFactor: 1 / 16, + paddingFactor: 1 / 4, + ); +} + +/// An icon, sized and aligned for use in a span of text. +WidgetSpan iconWidgetSpan({ + required IconData icon, + required double fontSize, + required TextBaseline baselineType, + required Color? color, + bool padBefore = false, + bool padAfter = false, +}) { + final InlineIconGeometryData( + :sizeFactor, + :alphabeticBaselineFactor, + :paddingFactor, + ) = InlineIconGeometryData.forIcon(icon); + + final size = sizeFactor * fontSize; + + final effectiveBaselineOffset = switch (baselineType) { + TextBaseline.alphabetic => alphabeticBaselineFactor * size, + TextBaseline.ideographic => 0.0, + }; + + Widget child = Icon(size: size, color: color, icon); + + if (effectiveBaselineOffset != 0) { + child = Transform.translate( + offset: Offset(0, effectiveBaselineOffset), + child: child); + } + + if (padBefore || padAfter) { + final padding = paddingFactor * size; + child = Padding( + padding: EdgeInsetsDirectional.only( + start: padBefore ? padding : 0, + end: padAfter ? padding : 0, + ), + child: child); + } + + return WidgetSpan( + alignment: PlaceholderAlignment.baseline, + baseline: baselineType, + child: child); +} + +/// An [InlineSpan] with a channel privacy icon and channel name. +/// +/// Pass this to [Text.rich], which can be styled arbitrarily. +/// Pass the [fontSize] of surrounding text +/// so that the icon is sized appropriately. +InlineSpan channelTopicLabelSpan({ + required BuildContext context, + required int channelId, + required double fontSize, + required Color color, +}) { + final zulipLocalizations = ZulipLocalizations.of(context); + final store = PerAccountStoreWidget.of(context); + final channel = store.streams[channelId]; + final subscription = store.subscriptions[channelId]; + final swatch = colorSwatchFor(context, subscription); + final channelIcon = channel != null ? iconDataForStream(channel) : null; + final baselineType = localizedTextBaseline(context); + + return TextSpan(children: [ + if (channelIcon != null) + iconWidgetSpan( + icon: channelIcon, + fontSize: fontSize, + baselineType: baselineType, + color: swatch.iconOnPlainBackground, + padAfter: true), + if (channel != null) + TextSpan(text: channel.name) + else + TextSpan( + style: TextStyle(fontStyle: FontStyle.italic), + text: zulipLocalizations.unknownChannelName), + ]); +} diff --git a/test/api/model/model_checks.dart b/test/api/model/model_checks.dart index a7db054b55..5cb6df7c24 100644 --- a/test/api/model/model_checks.dart +++ b/test/api/model/model_checks.dart @@ -54,6 +54,9 @@ extension SavedSnippetChecks on Subject { extension ZulipStreamChecks on Subject { Subject get streamId => has((x) => x.streamId, 'streamId'); + + Subject get inviteOnly => has((x) => x.inviteOnly, 'inviteOnly'); + Subject get isWebPublic => has((x) => x.isWebPublic, 'isWebPublic'); } extension ChannelFolderChecks on Subject { diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 2c055f7596..7a65008974 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -38,6 +38,7 @@ import 'package:zulip/widgets/topic_list.dart'; import 'package:zulip/widgets/user.dart'; import '../api/fake_api.dart'; +import '../api/model/model_checks.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; @@ -292,6 +293,61 @@ void main() { checkButton('Copy link to channel'); } + group('header', () { + final findHeader = find.descendant( + of: actionSheetFinder, + matching: find.byType(BottomSheetHeader)); + + Finder findInHeader(Finder finder) => + find.descendant(of: findHeader, matching: finder); + + testWidgets('public channel', (tester) async { + await prepare(); + check(store.streams[someChannel.streamId]).isNotNull() + ..inviteOnly.isFalse()..isWebPublic.isFalse(); + await showFromInbox(tester); + check(findInHeader(find.byIcon(ZulipIcons.hash_sign))).findsOne(); + check(findInHeader(find.textContaining(someChannel.name))).findsOne(); + }); + + testWidgets('web-public channel', (tester) async { + await prepare(); + await store.handleEvent(ChannelUpdateEvent(id: 1, + streamId: someChannel.streamId, + name: someChannel.name, + property: null, value: null, + // (Ideally we'd use `property` and `value` but I'm not sure if + // modern servers actually do that or if they still use this + // separate field.) + isWebPublic: true)); + check(store.streams[someChannel.streamId]).isNotNull() + ..inviteOnly.isFalse()..isWebPublic.isTrue(); + await showFromInbox(tester); + check(findInHeader(find.byIcon(ZulipIcons.globe))).findsOne(); + check(findInHeader(find.textContaining(someChannel.name))).findsOne(); + }); + + testWidgets('private channel', (tester) async { + await prepare(); + await store.handleEvent(eg.channelUpdateEvent(someChannel, + property: ChannelPropertyName.inviteOnly, value: true)); + check(store.streams[someChannel.streamId]).isNotNull() + ..inviteOnly.isTrue()..isWebPublic.isFalse(); + await showFromInbox(tester); + check(findInHeader(find.byIcon(ZulipIcons.lock))).findsOne(); + check(findInHeader(find.textContaining(someChannel.name))).findsOne(); + }); + + testWidgets('unknown channel', (tester) async { + await prepare(); + await store.handleEvent(ChannelDeleteEvent(id: 1, streams: [someChannel])); + check(store.streams[someChannel.streamId]).isNull(); + await showFromTopicListAppBar(tester); + check(findInHeader(find.byType(Icon))).findsNothing(); + check(findInHeader(find.textContaining('(unknown channel)'))).findsOne(); + }); + }); + testWidgets('show from inbox', (tester) async { await prepare(); await showFromInbox(tester); From 536aa9950607486a1f0afd498eb4fac67aa4cd24 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 8 Oct 2025 16:38:14 -0700 Subject: [PATCH 8/9] action_sheet: Increase line spacing in bottom-sheet title It turns out we can do this without wasting space with extra margin above or below the title, using TextHeightBehavior. --- lib/widgets/action_sheet.dart | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 7b3f192776..04f4fdfda8 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -175,16 +175,30 @@ class BottomSheetHeader extends StatelessWidget { final baseTitleStyle = TextStyle( fontSize: 20, - height: 20 / 20, + // More height than in Figma, but it was looking too tight: + // https://github.com/zulip/zulip-flutter/pull/1877#issuecomment-3379664807 + // (See use of TextHeightBehavior below.) + height: 24 / 20, color: designVariables.title, ).merge(weightVariableTextStyle(context, wght: 600)); - final effectiveTitle = switch ((buildTitle, title)) { + Widget? effectiveTitle = switch ((buildTitle, title)) { (final build?, null) => build(baseTitleStyle), (null, final data?) => Text(style: baseTitleStyle, data), _ => null, }; + if (effectiveTitle != null) { + effectiveTitle = DefaultTextHeightBehavior( + textHeightBehavior: TextHeightBehavior( + // We want some breathing room between lines, + // without adding margin above or below the title. + applyHeightToFirstAscent: false, + applyHeightToLastDescent: false, + ), + child: effectiveTitle); + } + final baseMessageStyle = TextStyle( color: designVariables.labelTime, fontSize: 17, From 3d3b1c1bb4340ab3b7b8429742b8b6cc023c54cb Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Sep 2025 18:22:59 -0700 Subject: [PATCH 9/9] action_sheet: Show topic in topic action sheet Fixes #1533. --- lib/widgets/action_sheet.dart | 15 +++++++++++++- lib/widgets/text.dart | 29 +++++++++++++++++++++++--- test/widgets/action_sheet_test.dart | 32 +++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 04f4fdfda8..30bbf433f5 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -785,7 +785,20 @@ void showTopicActionSheet(BuildContext context, { narrow: TopicNarrow(channelId, topic, with_: someMessageIdInTopic), pageContext: context)); - _showActionSheet(pageContext, buttonSections: [optionButtons]); + final header = BottomSheetHeader( + buildTitle: (baseStyle) => Text.rich( + style: baseStyle, + channelTopicLabelSpan( + context: context, + channelId: channelId, + topic: topic, + fontSize: baseStyle.fontSize!, + color: baseStyle.color!))); + + _showActionSheet(pageContext, + header: header, + headerScrollable: false, + buttonSections: [optionButtons]); } class UserTopicUpdateButton extends ActionSheetMenuItemButton { diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 577b6cdbb8..10a6485023 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -4,6 +4,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; +import '../api/model/model.dart'; import '../generated/l10n/zulip_localizations.dart'; import 'icons.dart'; import 'store.dart'; @@ -582,6 +583,11 @@ class InlineIconGeometryData { sizeFactor: 0.8, alphabeticBaselineFactor: 1 / 16, paddingFactor: 1 / 4), + + ZulipIcons.chevron_right: InlineIconGeometryData._( + sizeFactor: 1, + alphabeticBaselineFactor: 5 / 24, + paddingFactor: 0), }; static final _defaultGeometry = InlineIconGeometryData._( @@ -637,14 +643,16 @@ WidgetSpan iconWidgetSpan({ child: child); } -/// An [InlineSpan] with a channel privacy icon and channel name. +/// An [InlineSpan] with a channel privacy icon, channel name, +/// and optionally a chevron-right icon plus topic. /// /// Pass this to [Text.rich], which can be styled arbitrarily. -/// Pass the [fontSize] of surrounding text -/// so that the icon is sized appropriately. +/// Pass the [fontSize] and [color] of surrounding text +/// so that the icons are sized and colored appropriately. InlineSpan channelTopicLabelSpan({ required BuildContext context, required int channelId, + TopicName? topic, required double fontSize, required Color color, }) { @@ -670,5 +678,20 @@ InlineSpan channelTopicLabelSpan({ TextSpan( style: TextStyle(fontStyle: FontStyle.italic), text: zulipLocalizations.unknownChannelName), + if (topic != null) ...[ + iconWidgetSpan( + icon: ZulipIcons.chevron_right, + fontSize: fontSize, + baselineType: baselineType, + color: color, + padBefore: true, + padAfter: true), + if (topic.displayName != null) + TextSpan(text: topic.displayName) + else + TextSpan( + style: TextStyle(fontStyle: FontStyle.italic), + text: store.realmEmptyTopicDisplayName), + ], ]); } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 7a65008974..621f759f28 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -805,6 +805,38 @@ void main() { checkButton('Copy link to topic'); } + group('header', () { + final findHeader = find.descendant( + of: actionSheetFinder, + matching: find.byType(BottomSheetHeader)); + + Finder findInHeader(Finder finder) => + find.descendant(of: findHeader, matching: finder); + + testWidgets('with topic', (tester) async { + await prepare(); + check(store.streams[someChannel.streamId]).isNotNull() + ..inviteOnly.isFalse()..isWebPublic.isFalse(); + await showFromAppBar(tester); + check(findInHeader(find.byIcon(ZulipIcons.hash_sign))).findsOne(); + check(findInHeader(find.textContaining(someChannel.name))).findsOne(); + check(findInHeader(find.textContaining(someTopic))).findsOne(); + }); + + testWidgets('without topic (general chat)', (tester) async { + await prepare(topic: ''); + check(store.streams[someChannel.streamId]).isNotNull() + ..inviteOnly.isFalse()..isWebPublic.isFalse(); + final message = eg.streamMessage( + stream: someChannel, topic: '', sender: eg.otherUser); + await showFromAppBar(tester, messages: [message], topic: eg.t('')); + check(findInHeader(find.byIcon(ZulipIcons.hash_sign))).findsOne(); + check(findInHeader(find.textContaining(someChannel.name))).findsOne(); + check(findInHeader(find.textContaining(store.realmEmptyTopicDisplayName))) + .findsOne(); + }); + }); + testWidgets('show from inbox; message in Unreads but not in MessageStore', (tester) async { await prepare(unreadMsgs: eg.unreadMsgs(count: 1, channels: [eg.unreadChannelMsgs(