-
Notifications
You must be signed in to change notification settings - Fork 306
Show image previews side-by-side when consecutive #193
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
Labels
Milestone
Comments
gnprice
added a commit
that referenced
this issue
Jun 15, 2023
9 tasks
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Dec 20, 2023
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Jan 11, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Jan 11, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Jan 17, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Jan 18, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 5, 2024
Merged
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 9, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 9, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 9, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 12, 2024
sirpengi
added a commit
to sirpengi/zulip-flutter
that referenced
this issue
Feb 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Zulip web shows image previews that are fairly small. Then when a message has several images consecutively, it shows them next to each other in a row.
Currently we show image previews with a design that's modeled on Zulip web, except that we haven't implemented the logic to show several images next to each other on a row: we just show them one above the other, much like we would if there were text paragraphs in between.
(Specifically based on Zulip web with its old design — see #157 — but if this area has changed at all in the 2023 redesign, the changes were subtle. So I don't think they affect this issue, and we can handle this issue just as well before or after doing #157.)
As long as we're matching Zulip web's small previews, we should follow Zulip web in taking advantage of the small size to save vertical space, by showing multiple previews next to each other.
(Potentially we'll later decide to do something more different from Zulip web's approach instead. But for anything that's fancier than showing the previews one after the other vertically, we'll want the same code changes discussed below, so this work will be useful regardless.)
In the code, this may be a bit subtle because it will involve making some of our perhaps over-simple abstractions a bit less simple: currently we have a one-to-one mapping from the relevant HTML elements, via
parseBlockContentList
toImageNode
objects that go in a list, then viaBlockContentList
toMessageImage
widgets that go in aColumn
. To have several images in a row, we'll need their widgets to have a common parent that goes in theColumn
, instead of each going directly into theColumn
.On the principle of keeping complexity out of widget build time, let's let
BlockContentList
continue to have a simple one-to-one mapping from node inputs to widget outputs, and put the new complexity in the content nodes instead. We can add a new type ofBlockContentNode
that contains several images, and giveparseBlockContentList
the job of detecting runs of consecutive image-preview elements and putting them into a single such node.The text was updated successfully, but these errors were encountered: