-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: timezone shown for our own email #9642
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
Conversation
mananjadhav
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Puneet-here I've posted the comment and also suggestion of the function.
I would also request:
- Mark every item in the checklist. If it applies then test and mark, if it doesn't apply then just mark so that we know we've gone through it and adhere to it.
- QA Steps says go to
#announce-room, can we post atleast in one of the steps how to create an Announce with participants?
|
For some reason the commit to merge main branch wasn't signed so I had to reset it and force push. |
thienlnam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is getting there! Just a few more comments
thienlnam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also see a video for:
Chat with concierge (No timezone should be shown)
Chat with 1:1 (Timezone should be shown)
Chat with another user and concierge ( Timezone should be shown)
|
Removed the function. And here's the video - Screen.Recording.2022-07-15.at.11.05.35.PM.mov |
|
@Puneet-here Did we make the null handling change? I can't find it. |
|
@mananjadhav, no I didn't. As It's not needed.
|
|
Sweet, @mananjadhav please go ahead and complete testing for this PR |
|
@thienlnam Tests well and changes look good. 🎀 👀 🎀 Quick comment: The timezone text is aligned a bit on the right. It is right aligned in the main chat as well and doesn't need to be changed here, but I thought I'll just highlight it here.
|
This doesn't seem related to any changes made here, should be happening on prod as well so feel free to report in the #expensify-open-source channel for us to create a new issue |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@Puneet-here I just took the latest cc - @thienlnam |
|
@thienlnam IMP: Please check this PR for the login crash on this report - #9966 I still couldn't understand why it worked fine beforehand (especially with the typo in the prop name) and now its crashing. |
|
Yup i am seeing the same issue and dev is broken for me. Was going to push the same fix but you got to it sooner. Checking your code. |
|
🚀 Deployed to production by @luacmartins in version: 1.1.85-8 🚀
|



Details
Fixed timezone showing up for our own email at rooms when there is no user except you
Fixed Issues
$ #9238
Tests
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followed/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Screenshots
Web
Screen.Recording.2022-06-30.at.10.38.54.PM.mov
Screen.Recording.2022-06-30.at.10.40.45.PM.mov
Screen.Recording.2022-06-30.at.10.40.17.PM.mov
Mobile Web
Screen.Recording.2022-06-30.at.10.43.29.PM.mov
Desktop
Screen.Recording.2022-06-30.at.10.59.21.PM.mov
iOS
Screen.Recording.2022-06-30.at.10.47.52.PM.mov
Android
Screen.Recording.2022-06-30.at.10.56.30.PM.mov