Skip to content

Commit

Permalink
Fixing the issue zulip#930 by passing the narrow and message.topic to…
Browse files Browse the repository at this point in the history
… the LightBoxHeroTag

and adding narrow on lightbox_test and content_test
  • Loading branch information
sher999 committed Dec 13, 2024
1 parent 28b3536 commit 0c637e1
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 27 deletions.
50 changes: 31 additions & 19 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import '../model/avatar_url.dart';
import '../model/binding.dart';
import '../model/content.dart';
import '../model/internal_link.dart';
import '../model/narrow.dart';
import 'code_block.dart';
import 'dialog.dart';
import 'icons.dart';
Expand Down Expand Up @@ -278,10 +279,11 @@ const double kBaseFontSize = 17;
/// This does not include metadata like the sender's name and avatar, the time,
/// or the message's status as starred or edited.
class MessageContent extends StatelessWidget {
const MessageContent({super.key, required this.message, required this.content});
const MessageContent({super.key, required this.message, required this.content, required this.narrow});

final Message message;
final ZulipMessageContent content;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -290,7 +292,7 @@ class MessageContent extends StatelessWidget {
child: DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: switch (content) {
ZulipContent() => BlockContentList(nodes: content.nodes),
ZulipContent() => BlockContentList(nodes: content.nodes, narrow: narrow),
PollContent() => PollWidget(messageId: message.id, poll: content.poll),
}));
}
Expand Down Expand Up @@ -318,9 +320,10 @@ class InheritedMessage extends InheritedWidget {

/// A list of DOM nodes to display in block layout.
class BlockContentList extends StatelessWidget {
const BlockContentList({super.key, required this.nodes});
const BlockContentList({super.key, required this.nodes, required this.narrow});

final List<BlockContentNode> nodes;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -334,18 +337,18 @@ class BlockContentList extends StatelessWidget {
ThematicBreakNode() => const ThematicBreak(),
ParagraphNode() => Paragraph(node: node),
HeadingNode() => Heading(node: node),
QuotationNode() => Quotation(node: node),
ListNode() => ListNodeWidget(node: node),
SpoilerNode() => Spoiler(node: node),
QuotationNode() => Quotation(node: node, narrow: narrow,),
ListNode() => ListNodeWidget(node: node, narrow: narrow,),
SpoilerNode() => Spoiler(node: node, narrow: narrow,),
CodeBlockNode() => CodeBlock(node: node),
MathBlockNode() => MathBlock(node: node),
ImageNodeList() => MessageImageList(node: node),
ImageNodeList() => MessageImageList(node: node, narrow: narrow,),
ImageNode() => (){
assert(false,
"[ImageNode] not allowed in [BlockContentList]. "
"It should be wrapped in [ImageNodeList]."
);
return MessageImage(node: node);
return MessageImage(node: node, narrow: narrow,);
}(),
InlineVideoNode() => MessageInlineVideo(node: node),
EmbedVideoNode() => MessageEmbedVideo(node: node),
Expand Down Expand Up @@ -451,9 +454,10 @@ class Heading extends StatelessWidget {
}

class Quotation extends StatelessWidget {
const Quotation({super.key, required this.node});
const Quotation({super.key, required this.node, required this.narrow});

final QuotationNode node;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -467,14 +471,15 @@ class Quotation extends StatelessWidget {
width: 5,
// Web has the same color in light and dark mode.
color: const HSLColor.fromAHSL(1, 0, 0, 0.87).toColor()))),
child: BlockContentList(nodes: node.nodes)));
child: BlockContentList(nodes: node.nodes, narrow: narrow,)));
}
}

class ListNodeWidget extends StatelessWidget {
const ListNodeWidget({super.key, required this.node});
const ListNodeWidget({super.key, required this.node, required this.narrow});

final ListNode node;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -494,7 +499,7 @@ class ListNodeWidget extends StatelessWidget {
// TODO(#59) ordered lists starting not at 1
case ListStyle.ordered: marker = "${index+1}. "; break;
}
return ListItemWidget(marker: marker, nodes: item);
return ListItemWidget(marker: marker, nodes: item, narrow: narrow,);
});
return Padding(
padding: const EdgeInsets.only(top: 2, bottom: 5),
Expand All @@ -503,10 +508,11 @@ class ListNodeWidget extends StatelessWidget {
}

class ListItemWidget extends StatelessWidget {
const ListItemWidget({super.key, required this.marker, required this.nodes});
const ListItemWidget({super.key, required this.marker, required this.nodes, required this.narrow});

final String marker;
final List<BlockContentNode> nodes;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -519,15 +525,16 @@ class ListItemWidget extends StatelessWidget {
width: 20, // TODO handle long numbers in <ol>, like https://github.com/zulip/zulip/pull/25063
child: Align(
alignment: AlignmentDirectional.topEnd, child: Text(marker))),
Expanded(child: BlockContentList(nodes: nodes)),
Expanded(child: BlockContentList(nodes: nodes, narrow: narrow,)),
]);
}
}

class Spoiler extends StatefulWidget {
const Spoiler({super.key, required this.node});
const Spoiler({super.key, required this.node, required this.narrow});

final SpoilerNode node;
final Narrow narrow;

@override
State<Spoiler> createState() => _SpoilerState();
Expand Down Expand Up @@ -588,6 +595,7 @@ class _SpoilerState extends State<Spoiler> with TickerProviderStateMixin {
child: DefaultTextStyle.merge(
style: weightVariableTextStyle(context, wght: 700),
child: BlockContentList(
narrow: widget.narrow,
nodes: effectiveHeader))),
RotationTransition(
turns: _animation.drive(Tween(begin: 0, end: 0.5)),
Expand All @@ -609,27 +617,29 @@ class _SpoilerState extends State<Spoiler> with TickerProviderStateMixin {
axisAlignment: -1,
child: Padding(
padding: const EdgeInsets.all(5),
child: BlockContentList(nodes: widget.node.content))),
child: BlockContentList(nodes: widget.node.content, narrow: widget.narrow,))),
]))));
}
}

class MessageImageList extends StatelessWidget {
const MessageImageList({super.key, required this.node});
const MessageImageList({super.key, required this.node, required this.narrow});

final ImageNodeList node;
final Narrow narrow;

@override
Widget build(BuildContext context) {
return Wrap(
children: node.images.map((imageNode) => MessageImage(node: imageNode)).toList());
children: node.images.map((imageNode) => MessageImage(node: imageNode, narrow: narrow)).toList());
}
}

class MessageImage extends StatelessWidget {
const MessageImage({super.key, required this.node});
const MessageImage({super.key, required this.node, required this.narrow});

final ImageNode node;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -648,6 +658,7 @@ class MessageImage extends StatelessWidget {
return MessageMediaContainer(
onTap: resolvedSrcUrl == null ? null : () { // TODO(log)
Navigator.of(context).push(getImageLightboxRoute(
narrow: narrow,
context: context,
message: message,
src: resolvedSrcUrl,
Expand All @@ -658,6 +669,7 @@ class MessageImage extends StatelessWidget {
child: node.loading
? const CupertinoActivityIndicator()
: resolvedSrcUrl == null ? null : LightboxHero(
narrow: narrow,
message: message,
src: resolvedSrcUrl,
child: RealmContentNetworkImage(
Expand Down
16 changes: 14 additions & 2 deletions lib/widgets/lightbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import '../api/model/model.dart';
import '../generated/l10n/zulip_localizations.dart';
import '../log.dart';
import '../model/binding.dart';
import '../model/narrow.dart';
import 'content.dart';
import 'dialog.dart';
import 'page.dart';
Expand All @@ -21,15 +22,19 @@ import 'store.dart';
// fly to an image preview with a different URL, following a message edit
// while the lightbox was open.
class _LightboxHeroTag {
_LightboxHeroTag({required this.messageId, required this.src});
_LightboxHeroTag({required this.messageId, required this.src, required this.topic, required this.narrow});

final int messageId;
final Uri src;
final String topic;
final Narrow narrow;

@override
bool operator ==(Object other) {
return other is _LightboxHeroTag &&
other.messageId == messageId &&
other.topic == topic &&
other.narrow == narrow &&
other.src == src;
}

Expand All @@ -44,16 +49,18 @@ class LightboxHero extends StatelessWidget {
required this.message,
required this.src,
required this.child,
required this.narrow,
});

final Message message;
final Uri src;
final Widget child;
final Narrow narrow;

@override
Widget build(BuildContext context) {
return Hero(
tag: _LightboxHeroTag(messageId: message.id, src: src),
tag: _LightboxHeroTag(messageId: message.id, src: src, topic: message.topic, narrow: narrow),
flightShuttleBuilder: (
BuildContext flightContext,
Animation<double> animation,
Expand Down Expand Up @@ -227,6 +234,7 @@ class _ImageLightboxPage extends StatefulWidget {
required this.thumbnailUrl,
required this.originalWidth,
required this.originalHeight,
required this.narrow,
});

final Animation<double> routeEntranceAnimation;
Expand All @@ -235,6 +243,7 @@ class _ImageLightboxPage extends StatefulWidget {
final Uri? thumbnailUrl;
final double? originalWidth;
final double? originalHeight;
final Narrow narrow;

@override
State<_ImageLightboxPage> createState() => _ImageLightboxPageState();
Expand Down Expand Up @@ -314,6 +323,7 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> {
child: InteractiveViewer(
child: SafeArea(
child: LightboxHero(
narrow: widget.narrow,
message: widget.message,
src: widget.src,
child: RealmContentNetworkImage(widget.src,
Expand Down Expand Up @@ -599,12 +609,14 @@ Route<void> getImageLightboxRoute({
required Uri? thumbnailUrl,
required double? originalWidth,
required double? originalHeight,
required Narrow narrow
}) {
return _getLightboxRoute(
accountId: accountId,
context: context,
pageBuilder: (context, animation, secondaryAnimation) {
return _ImageLightboxPage(
narrow: narrow,
routeEntranceAnimation: animation,
message: message,
src: src,
Expand Down
10 changes: 7 additions & 3 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
case MessageListMessageItem():
final header = RecipientHeader(message: data.message, narrow: widget.narrow);
return MessageItem(
narrow: widget.narrow,
key: ValueKey(data.message.id),
header: header,
trailingWhitespace: i == 1 ? 8 : 11,
Expand Down Expand Up @@ -951,12 +952,14 @@ class MessageItem extends StatelessWidget {
super.key,
required this.item,
required this.header,
required this.narrow,
this.trailingWhitespace,
});

final MessageListMessageItem item;
final Widget header;
final double? trailingWhitespace;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand All @@ -970,7 +973,7 @@ class MessageItem extends StatelessWidget {
child: ColoredBox(
color: messageListTheme.streamMessageBgDefault,
child: Column(children: [
MessageWithPossibleSender(item: item),
MessageWithPossibleSender(item: item, narrow: narrow),
if (trailingWhitespace != null && item.isLastInBlock) SizedBox(height: trailingWhitespace!),
]))));
}
Expand Down Expand Up @@ -1275,9 +1278,10 @@ String formatHeaderDate(
// - https://github.com/zulip/zulip-mobile/issues/5511
// - https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538%3A20849&mode=dev
class MessageWithPossibleSender extends StatelessWidget {
const MessageWithPossibleSender({super.key, required this.item});
const MessageWithPossibleSender({super.key, required this.item, required this.narrow});

final MessageListMessageItem item;
final Narrow narrow;

@override
Widget build(BuildContext context) {
Expand Down Expand Up @@ -1362,7 +1366,7 @@ class MessageWithPossibleSender extends StatelessWidget {
Expanded(child: Column(
crossAxisAlignment: CrossAxisAlignment.stretch,
children: [
MessageContent(message: message, content: item.content),
MessageContent(message: message, content: item.content, narrow: narrow),
if ((message.reactions?.total ?? 0) > 0)
ReactionChipsList(messageId: message.id, reactions: message.reactions!),
if (editStateText != null)
Expand Down
6 changes: 6 additions & 0 deletions packages/zulip_plugin/pubspec.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Generated by pub
# See https://dart.dev/tools/pub/glossary#lockfile
packages: {}
sdks:
dart: ">=3.4.0-256.0.dev <4.0.0"
flutter: ">=3.3.0"
6 changes: 4 additions & 2 deletions test/widgets/content_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,17 @@ void main() {
TestZulipBinding.ensureInitialized();

Widget plainContent(String html) {
final narrow = TopicNarrow.ofMessage(eg.streamMessage());
return Builder(builder: (context) =>
DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: BlockContentList(nodes: parseContent(html).nodes)));
child: BlockContentList(nodes: parseContent(html).nodes, narrow: narrow,)));
}

Widget messageContent(String html) {
final narrow = TopicNarrow.ofMessage(eg.streamMessage());
return MessageContent(message: eg.streamMessage(content: html),
content: parseContent(html));
content: parseContent(html),narrow: narrow,);
}

// TODO(#488) For content that we need to show outside a per-message context
Expand Down
4 changes: 3 additions & 1 deletion test/widgets/lightbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:video_player_platform_interface/video_player_platform_interface.
import 'package:video_player/video_player.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/widgets/app.dart';
import 'package:zulip/widgets/content.dart';
import 'package:zulip/widgets/lightbox.dart';
Expand Down Expand Up @@ -199,6 +200,7 @@ void main() {

group('_ImageLightboxPage', () {
final src = Uri.parse('https://chat.example/lightbox-image.png');
final narrow = TopicNarrow.ofMessage(eg.streamMessage());

Future<void> setupPage(WidgetTester tester, {
Message? message,
Expand All @@ -219,7 +221,7 @@ void main() {
src: src,
thumbnailUrl: thumbnailUrl,
originalHeight: null,
originalWidth: null,
originalWidth: null, narrow: narrow,
)));
await tester.pump(); // per-account store
await tester.pump(const Duration(milliseconds: 301)); // nav transition
Expand Down

0 comments on commit 0c637e1

Please sign in to comment.