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

[HOLD for payment 2022-03-22] Always display timezone of the other person in the chat window - Reported by @Santhosh-Sellavel #7832

Closed
mvtglobally opened this issue Feb 20, 2022 · 19 comments
Assignees
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Feb 20, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open Any User Chat with the same timezone as yours. OR (Create two test accounts User A & B with the same timezone settings. And Open User B's chat logged in as User A )
  2. Navigate to User Details by clicking on the chat header

Expected Result:

Timezone should be displayed at bottom of chat in all 1:1 chats.

Actual Result:

Currently timezone is displayed at bottom of chat only if your timezone doesn't match the timezone of the other person.

Eg. Sonia is in PST, Vic is in AEDT. So in the Sonia-Vic chat both see each other's local time at the bottom of the chat window:

image

Sonia is in PST, and Francois is in PST. So the Sonia-Francois chat doesn't display local time at the bottom of the chat:

image

Showing timezone in some cases but not in others feels unintentional. Let's make it consistent by showing the other person's local time at the bottom of a 1:1 chat in all cases regardless of the participants' timezones.

Workaround:

Click on the profile icon of the person you're chatting with to open their Details panel which lists their timezone and local time.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.38-0
Reproducible in staging?: y
Reproducible in production?: y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screenshot 2022-02-04 at 10 11 28 PM

Expensify/Expensify Issue URL:
Issue reported by: @Santhosh-Sellavel
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1643993593833769

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Feb 20, 2022
@MelvinBot
Copy link

Triggered auto assignment to @sonialiap (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Feb 20, 2022
@parasharrajat
Copy link
Member

This is not a bug.

@iwiznia
Copy link
Contributor

iwiznia commented Feb 21, 2022

I know this is on purpose and we don't display the timezone if they are the same. But to me as a user, it looks broken. I don't know if the other person is in my same timezone or not. I rather see the timezone in every chat.

@sonialiap
Copy link
Contributor

I don't think I've ever seen any chat/email systems display sender's timezone on the message timestamp. If we were to be the first to do that, it would be confusing to users since there isn't a precedent for this.

Currently we show the message timestamp in your timezone. If the other person in the chat is in a different timezone then their timezone is displayed at the bottom of the window.

Victoria is in AEDT, I am in PST so I'm shown her current time:

image

If they're in the same timezone as you, then their time isn't displayed. Nikki and I are both in PST:

image

I think it makes sense to either leave it as it is or display local time at the bottom of every chat (so my chat with Nikki would show her time too). @iwiznia do you think we should make this change?

@iwiznia
Copy link
Contributor

iwiznia commented Feb 22, 2022

I have no idea where some people are or if where they are it's the same time zone as where I am. I went to a chat and wanted to know if they could be online based on their time and did not see it there, to me it looked like a bug because all other chats seemingly have it.
So yeah, I think we should always show the other user's timezone, no matter if they are in the same timezone as you or not.

@sonialiap sonialiap changed the title We display timezone on the User Details Page and but we don't on the chat window - Reported by @Santhosh-Sellavel Always display timezone of the other person in the chat window - Reported by @Santhosh-Sellavel Feb 22, 2022
@sonialiap
Copy link
Contributor

Made some tweaks to the original post

@sonialiap sonialiap removed their assignment Feb 22, 2022
@MelvinBot
Copy link

Triggered auto assignment to @chiragsalian (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Feb 22, 2022

Proposal

Here is the restriction we make to hide the other user's time, where we check the other user is from a different timezone.
Removing this check would be enough to handle this issue.

&& moment().tz(currentUserTimezone.selected).utcOffset() !== moment().tz(reportRecipientTimezone.selected).utcOffset();

@chiragsalian chiragsalian added the External Added to denote the issue can be worked on by a contributor label Feb 22, 2022
@MelvinBot
Copy link

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@kadiealexander
Copy link
Contributor

Posted to Upwork: https://www.upwork.com/jobs/~01fb9158fbedf74de3

@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 23, 2022
@MelvinBot
Copy link

Current assignee @chiragsalian is eligible for the Exported assigner, not assigning anyone new.

@koushal141
Copy link

koushal141 commented Feb 23, 2022

Proposal from Upwork

Hi!
This seems like it's not a bug, but the way the front end is coded.
I.e:- When time zones are same, they are rendered at the top and when they are different, they render at the bottom.

This can be fixed by merging the different timezone logic from bottom with the one at the top.

Eg: { timezoneOfUser1==timezoneOfUser2? < RenderLocalTime /> : < Render International Time />}

Or remove the logic of checking timezones completely and just use new Date(timestamp) which will return the Date & Time object based on the users device timezone

@parasharrajat
Copy link
Member

I like @Santhosh-Sellavel 's proposal.

cc: @chiragsalian

🎀 👀 🎀 C+ reviewed

@chiragsalian
Copy link
Contributor

Yup it lgtm too.

Feel free to create the PR @Santhosh-Sellavel.

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 23, 2022
@MelvinBot
Copy link

📣 @Santhosh-Sellavel You have been assigned to this job by @chiragsalian!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@kadiealexander
Copy link
Contributor

@Santhosh-Sellavel please don't forget to apply in Upwork!

@Santhosh-Sellavel
Copy link
Collaborator

Done @kadiealexander !

@kadiealexander kadiealexander added the Reviewing Has a PR in review label Mar 7, 2022
@MelvinBot MelvinBot removed the Overdue label Mar 7, 2022
@kadiealexander kadiealexander changed the title Always display timezone of the other person in the chat window - Reported by @Santhosh-Sellavel [HOLD for payment 2022-03-22] Always display timezone of the other person in the chat window - Reported by @Santhosh-Sellavel Mar 21, 2022
@kadiealexander
Copy link
Contributor

Paid @Santhosh-Sellavel for his fix + reporting bonus, and paid @parasharrajat for C+. Thanks team!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants