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

It's really hard to figure out the date of the messages you're looking at #1336

Open
timabbott opened this issue Oct 20, 2017 · 11 comments
Open

Comments

@timabbott
Copy link
Member

When you're looking at a set of messages, it can be really hard to figure out what date they were sent at. While I think this problem is much more annoying because of our scroll position issues which can often lead to looking at messages from a few days ago seemingly randomly, I think it's still relevant regardless.

I wonder if we should in the mobile app, start displaying timestamps in the sender rows that are not for today in the format of e.g. "Jun 5 15:50". It feels like we have the space, and I don't see a better way to surface this information. Since most of the time folks will be reading messages from today, there shouldn't be much added clutter from doing this.

(Though I suppose the problem here is actually that stick headers aren't working, since those are where we had the date before? So fixing that could be a better tactical solution)

@timabbott timabbott added the bug label Oct 20, 2017
@borisyankov
Copy link
Contributor

I'll think about this... but we never had the date in the sticky headers.

@timabbott
Copy link
Member Author

OK; I didn't remember, but that's the other obvious place to put it.

@pulkonet
Copy link
Collaborator

pulkonet commented Mar 9, 2018

Can I work on this? I am thinking to implement something like WhatsApp.

@borisyankov
Copy link
Contributor

How is it done in WhatsApp?
You can work on it, but keep in mind, when we do introduce a new feature, we do discuss it in detail (vs a fix, that you can just go ahead and implement)

@ishammahajan
Copy link
Collaborator

@borisyankov On WhatsApp there is always a sticky header displayed which displays only the date on which the message on top of the list currently visible was sent. Other headers (non-sticky) contain the date on which the first message below the header was sent.

This seems to be a very good option since we do have a lot of space in the header, as pointed out by @timabbott , which goes unused. Another option could be displaying the date below the message itself (where we currently show the time, but that would become cluttered very quickly). Please let me know what you think about this. I would like to claim this if it's not being worked on presently.

@borisyankov borisyankov removed their assignment Mar 17, 2019
@borisyankov
Copy link
Contributor

@ishammahajan it is not being worked on currently. I did unassign myself from the issue to make this clear.

I think the WhatsApp implementation is feasible but a little weird.
Not sure how the other suggestion you are making would look like :)

This is worth working on!

@ishammahajan
Copy link
Collaborator

I'll try and produce mockups of each implementation before implementing them in full.

@zulipbot claim

@ishammahajan
Copy link
Collaborator

@borisyankov made them, here they are

Mockup for implementation in header
https://imgur.com/LRB7dtZ

Mockup for implementation in message itself
https://imgur.com/DW11QQQ

I think the first implementation is better, no?

ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Mar 19, 2019
Many times when people are scrolling through messages they need to see what date it is. This modification is made for this use-case. Users can simply see the header to determine the date a particular message was sent.

This is implemented using some basic `css` and `js`. When writing the css selector for the header of the topic, there was an issue arising where the sticky header (because a small part of it gets cut out) was showing misalignment. This is the reason why `margin-top: 0.2em` was added to `.topic-header` selector.

Tests have been conducted, and behaviour is checked for both Android and iOS, light and dark mode.

Fixes zulip#1336
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Mar 19, 2019
Many times when people are scrolling through messages they need to see 
what date it is. This modification is made for this use-case. Users can 
simply see the header to determine the date a particular message was sent.

This is implemented using some basic `css` and `js`. When writing the css 
selector for the header of the topic, there was an issue arising where the 
sticky header (because a small part of it gets cut out) was showing 
misalignment. This is the reason why `margin-top: 0.2em` was added to 
`.topic-header` selector.

Tests have been conducted, and behaviour is checked for both Android and 
iOS, light and dark mode.

Fixes zulip#1336
@gnprice
Copy link
Member

gnprice commented Mar 20, 2019

Hi @ishammahajan -- thanks for your work on this and the PR #3418 !

That one implemented the "dates in topic bars" design. As I wrote there, though:

In particular, these won't show up at all when narrowed to a single topic, or to a PM (1:1 or group) conversation. So it doesn't really fix #1336 -- we need something that applies to those too.

I see a couple of possible directions that might work (and there may be others):

  • Put dates in topic bars... and then, like the webapp, have more general "recipient bars" appear even when you're in a narrow where the recipient will always be the same.
    image

  • As Tim suggested in the OP: put dates in message timestamps, except leave them out when the date is "today".

    • To fully solve the problem, this also requires addressing Show diverse post times for multiple posts from the same user #3375 -- show timestamps on individual messages when one user sends several in a row.
    • This is basically what Android Messages and FB Messenger both do, when they do show the timestamp. (Which they don't always, but if you touch a message, a small area expands to show it.)

@ishammahajan
Copy link
Collaborator

@gnprice Thank you, Zulip is the first open source project I'm working on, and I must say I'm loving open source! :)

The date should be anchored to one location, in my opinion, and the topic header is one such location. If the date was anchored to the message, as mentioned in #3375, the UI will become very convoluted.

I am in favour of including a header in the narrowed down topic screen as well. This will further increase readability on device screens and unify the experience of using the web application and the mobile application.

ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Mar 29, 2019
Many times when people are scrolling through messages they need to
see what date it is. This modification is made for this use-case.
Users can simply see the header to determine the date a particular
message was sent.

Header is converted into a flexbox and js for timestamp is added.
The date is human readable, and header is truncated if the topic
name is very large. This commit only adds the feature for existing
streams, subsequent commits add the rest of the functionality.

Tests have been conducted, and behaviour is checked for both Android
and iOS, light and dark mode.

Fixes zulip#1336
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Apr 6, 2019
Many times when people are scrolling through messages they need to
see what date it is. This modification is made for this use-case.
Users can simply see the header to determine the date a particular
message was sent.

Header is converted into a flexbox and js for timestamp is added.
The date is human readable, and header is truncated if the topic
name is very large. This commit only adds the feature for narrows
where headers already exist, subsequent commits add headers to the
rest of the files.

Tests have been conducted, and behaviour is checked for both Android
and iOS, light and dark mode.

Fixes zulip#1336
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Apr 22, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue Apr 28, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue May 2, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
gnprice pushed a commit to ishammahajan/zulip-mobile that referenced this issue May 3, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue May 4, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
ishammahajan added a commit to ishammahajan/zulip-mobile that referenced this issue May 4, 2019
Why this was required :-
The previous implementation of timestamp in header did not react to
changes in dates. The dates remained the same whether or not a
timerow passed by in the message list. This implementation fixes
that problem and makes the timestamp reactive to scrolling events.

Implementation :-
Every message in the webview now includes another attribute which
shows what date (human readable) it was sent.
In the webview js, after everytime the scroll event fires the first
visible message is calculated, and the closest header is walked
to. The human readable date is extracted from the attribute
introduced in message, and the content of the timestamp in the
header is changed to suit that.

Things to be considered :-
This might induce some performance issues. Optimisation seems to be
unnecessary for just one change in the DOM every scroll event,
however this is still something to watch for in case users report
slowdown in performance. This also has been tested for both devices
on simulators and there were no stutters on them.

Fixes zulip#1336.
@chrisbobbe
Copy link
Contributor

chrisbobbe commented Jan 22, 2020

Thanks to @ishammahajan for all your hard work on this!

It appears that this has gone through a few iterations, with #3491 having been merged, and #3435 becoming #3496, which is still open.

If I understand right, #3496, which would close this issue, is mostly done! 🙂It would make the date clear on private messages and topic narrows. It looks like some changes have been made in response to Greg's comments, and they're awaiting confirmation/feedback. I really wanted to write up a summary of the changes to help with the review, but there are several conflicted files because a lot has happened in the meantime, and I think it would take me much longer than it would take @gnprice or @ray-kraesig to resolve them, since I'm not as familiar with the code.

As an aside, I think I've found an orphaned, issue-less PR that may be a candidate for closing, which is related to showing timestamps on messages, and I commented on it. That's #3488 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants