Skip to content
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

Convert message list to new, 2023+ design #157

Closed
gnprice opened this issue Jun 7, 2023 · 6 comments
Closed

Convert message list to new, 2023+ design #157

gnprice opened this issue Jun 7, 2023 · 6 comments
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-design Visual and UX design a-msglist The message-list screen, except what's label:a-content

Comments

@gnprice
Copy link
Member

gnprice commented Jun 7, 2023

In the current prototype, the visual design of the message list is based closely on Zulip web's design as it was last December, when I started on the prototype and wrote most of the code we have for that so far. This also makes it broadly similar to the zulip-mobile RN app, which uses a design based on the same generation of Zulip web's design.

Since then Zulip web has undergone a substantial redesign, to designs by @terpimost:
https://blog.zulip.com/2023/05/31/zulip-7-0-released/#redesign
and some of the major changes there are in the message list. Among them:

We should target the new design, instead of the old.

Probably the way to do this is to basically go from scratch, rather than incrementally modify the existing design. (This is one advantage we have as a prototype compared to making the same change in web.) That means:

  • In all the places in widgets/message_list.dart and widgets/content.dart where we're controlling styling and layout, expect to rip that code out and replace it with newly-written code. (On the other hand, the code that's about finding appropriate data to show, or walking a ContentNode tree, should mostly not need to change for this.)
  • To write the new versions of that code, repeat the reverse-engineering work I did back in 2022-12 on web's design, but this time on the new design. Look at the corresponding rendering in web using browser devtools; refer to the CSS styles for exact values for spacing, a rounded corner's radius, and so on; and use Flutter desktop to put the prototype side by side with web on screen, and Flutter's fast hot reload to dial details in exactly.

We won't want to match web's design exactly:

  • Notably there's that wide gutter at the right of the message content, which is costly on a mobile-sized screen and in any case serves no purpose on a touchscreen because it's just reserving space for UI to appear on hover. So we'll cut that gutter.
  • The "blue box" cursor is for the sake of keyboard shortcuts, and we'll leave it out.
  • There's probably some other aspect of the message list where there'll be a good reason for mobile to differ from web, but I'm not thinking of one right now.
@gnprice gnprice added the a-msglist The message-list screen, except what's label:a-content label Jun 7, 2023
@gnprice gnprice added this to the Beta milestone Jun 7, 2023
@terpimost
Copy link

If confused about anything or willing to simplify design because of any reason please tell me and I try my best to help

@gnprice
Copy link
Member Author

gnprice commented Jun 14, 2023

A note for when we take this on: I think it will make sense to pursue both the light theme and dark theme at the same time.

That's because I think that when we're in the midst of doing that reverse-engineering work described above, it will be relatively little work to include the dark-theme version of a given area of the design in addition to the light-theme version, compared to the work that would be involved to complete the reverse-engineering for one theme and then at some later point come back and get back into the reverse-engineering to take care of the other theme.

In the code, this probably means:

  • Add some way, possibly crude, to switch between dark and light themes (ui: Implement dark theme #95). For work on this issue, that could be just that you edit the ZulipApp code to pass brightness as demoed in that issue's description, and use hot reload to switch back and forth. A perhaps nicer version might be a switch that appears in the app bar, perhaps only in the app bar of MessageListPage.
  • Then whenever specifying a color in the message-list code, use Theme.of(context).colorScheme.brightness to tell if the current theme is dark or light, and condition the color on that.

@gnprice
Copy link
Member Author

gnprice commented Oct 17, 2023

Ah, and thanks to @sirpengi for locating the zulip-mobile issue where @terpimost previously walked through a mobile version of the new design for the message list:

So I think that design is the place to start from. For all the details it doesn't cover, we'll fall back to reverse-engineering web as described above.

Here's also @terpimost's Figma design for the message list:
https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=538-20300

sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 24, 2023
This is a small change in line with zulip#157 and
moves the implementation closer to the new 2023+
design for mobile.
Messages now span from screen edge to screen edge
and borders around groups of messages (in particular
the left-side recipient border) was removed.
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 25, 2023
This is a small change in line with zulip#157 and
moves the implementation closer to the new 2023+
design for mobile.
Messages now span from screen edge to screen edge
and borders around groups of messages (in particular
the left-side recipient border) was removed.
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Oct 25, 2023
This is a small change in line with zulip#157 and
moves the implementation closer to the new 2023+
design for mobile.
Messages now span from screen edge to screen edge
and borders around groups of messages (in particular
the left-side recipient border) was removed.
@gnprice gnprice mentioned this issue Oct 25, 2023
@gnprice
Copy link
Member Author

gnprice commented Nov 8, 2023

I've just filed two sub-issues of this:

which we'll want for the upcoming Beta 1 milestone. We'll pursue the rest of this for Beta 2.

@gnprice gnprice modified the milestones: Beta 1, Beta 2 Nov 8, 2023
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Nov 15, 2023
Values obtained by looking at the "Computed" tab in the Chrome
DevTools element inspector, on CZO at 8.0-dev-2850-gef1c1ce05f.

Technically the web app has a computed `line-height` of 16.996px,
but that seems like a rounding error.

One reason for doing this now is to opt out of a default that would
take effect with the upcoming migration to Material 3. The text
would otherwise get a line height of 1.43 times the font size, and
we want it to be denser than that. We briefly explored how to
preserve the line height exactly, across the M3 migration, but the
solutions we found seemed more awkward than just taking care of
this now:
  zulip#380 (comment)

Fixes-partly: zulip#157
Fixes-partly: zulip#294
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue Nov 16, 2023
Values obtained by looking at the "Computed" tab in the Chrome
DevTools element inspector, on CZO at 8.0-dev-2850-gef1c1ce05f.

Technically the web app has a computed `line-height` of 16.996px,
but that seems like a rounding error.

One reason for doing this now is to opt out of a default that would
take effect with the upcoming migration to Material 3. The text
would otherwise get a line height of 1.43 times the font size, and
we want it to be denser than that. We briefly explored how to
preserve the line height exactly, across the M3 migration, but the
solutions we found seemed more awkward than just taking care of
this now:
  zulip#380 (comment)

Fixes-partly: zulip#157
Fixes-partly: zulip#294
@gnprice gnprice modified the milestones: Beta 2, Beta 3 Nov 22, 2023
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Jan 12, 2024
Moved widgets around such that the gutters to the
left and right of message contents are ready for
future decorative elements.

Fixes-partly: zulip#157
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Jan 12, 2024
Moved widgets around such that the gutters to the
left and right of message contents are ready for
future decorative elements.

Fixes-partly: zulip#157
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Jan 15, 2024
Moved widgets around such that the gutters to the
left and right of message contents are ready for
future decorative elements.

Fixes-partly: zulip#157
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Jan 23, 2024
Moved widgets around such that the gutters to the
left and right of message contents are ready for
future decorative elements.

Fixes-partly: zulip#157
sirpengi added a commit to sirpengi/zulip-flutter that referenced this issue Jan 24, 2024
Adjusted a test that was broken by the layout change.

Fixes-partly: zulip#157
@gnprice gnprice added the a-content Parsing and rendering Zulip HTML content, notably message contents label May 8, 2024
chrisbobbe added a commit to chrisbobbe/zulip-flutter that referenced this issue May 15, 2024
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 26, 2024
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#157.

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208 PIG208 linked a pull request Jun 26, 2024 that will close this issue
@gnprice
Copy link
Member Author

gnprice commented Jul 26, 2024

This umbrella issue has already been partly split up into individual issues. The next step is to finish doing that, and close the umbrella issue so we can track the individual pieces separately.

@gnprice
Copy link
Member Author

gnprice commented Nov 1, 2024

We've resolved quite a lot of the content of this umbrella issue already, as seen in the backlinks above from numerous issues and PRs.

I just made a pass through the code mentioned in the description:

In all the places in widgets/message_list.dart and widgets/content.dart where we're controlling styling and layout, […]

and also looked around the message list in the live app to look for anything that seemed like the old design.

I think at this point there's only a few pieces of this umbrella issue that remain open. Those are:

Also related are:

With that, I'm closing this umbrella issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-design Visual and UX design a-msglist The message-list screen, except what's label:a-content
Projects
Status: Done
Development

No branches or pull requests

2 participants