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

Profile - GMT time is not displayed #2097 #2756

Closed
wants to merge 2 commits into from
Closed

Profile - GMT time is not displayed #2097 #2756

wants to merge 2 commits into from

Conversation

kaushiktd
Copy link
Contributor

@kaushiktd kaushiktd commented May 10, 2021

Details

This issue is resolved by just modifying moment syntax because in the moment library, some timezone abbreviation, not support. so if return numeric value I've put modified syntax to show GMT.

Fixed Issues

Fixes #2097

Tests

  1. Pull code from GitHub and install its dependencies via the below command: npm install
  2. Run this project locally via npm run web / npm run android
  3. login via existing credentials.
  4. Go to the settings page then open the profile tab.
  5. Set desired timezone.
  6. log out and log in with another account.
  7. search for the first account and check his/her profile and check on the local time option. It will show GMT or any other timezone.

QA Steps

I've already QA this issue and it's working properly.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

2021-05-16

Mobile Web

Desktop

iOS

2021-05-17

Android

2021-05-17

@kaushiktd kaushiktd requested a review from a team as a code owner May 10, 2021 08:17
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team May 10, 2021 08:17
@github-actions
Copy link
Contributor

github-actions bot commented May 10, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@stitesExpensify
Copy link
Contributor

Hi @kaushiktd it looks like you didn't use our pull request template which has a lot of important info on it. Could you update your PR to follow it?
This is an example #2746

This is the template

<!-- If necessary, assign reviewers that know the area or changes well. Feel free to tag any additional reviewers you see fit. -->

### Details
<!-- Explanation of the change or anything fishy that is going on -->

### Fixed Issues
<!-- Please replace GH_LINK with the link to the GitHub issue this Pull Request is fixing -->
Fixes GH_LINK

### Tests
<!--- 
Add a numbered list of manual tests you performed that validates your changes work on all platforms, and that there are no regressions present.
Add any additional test steps if test steps are unique to a particular platform.
Manual test steps should be written so that your reviewer can repeat and verify one or more expected outcomes in the development environment.

For example:
1. Click on the text input to bring it into focus
2. Upload an image via copy paste
3. Verify a modal appears displaying a preview of that image 
--->

### QA Steps
<!--- 
Add a numbered list of manual tests that can be performed by our QA engineers on the staging environment to validate that your changes work on all platforms, and that there are no regressions present.
Add any additional QA steps if test steps are unique to a particular platform.
Manual test steps should be written so that the QA engineer can repeat and verify one or more expected outcomes in the staging environment.

For example:
1. Click on the text input to bring it into focus
2. Upload an image via copy paste
3. Verify a modal appears displaying a preview of that image 
--->

### Tested On

- [ ] Web
- [ ] Mobile Web
- [ ] Desktop
- [ ] iOS
- [ ] Android

### Screenshots
<!-- Add screenshots for all platforms tested. Pull requests won't be merged unless the screenshots show the app was tested on all platforms.-->

#### Web
<!-- Insert screenshots of your changes on the web platform-->

#### Mobile Web
<!-- Insert screenshots of your changes on the web platform (from a mobile browser)-->

#### Desktop
<!-- Insert screenshots of your changes on the desktop platform-->

#### iOS
<!-- Insert screenshots of your changes on the iOS platform-->

#### Android
<!-- Insert screenshots of your changes on the Android platform-->

@stitesExpensify
Copy link
Contributor

@kaushiktd can you please update this PR?

@kaushiktd
Copy link
Contributor Author

@kaushiktd can you please update this PR?

@stitesExpensify please check I've updated PR request.

@@ -101,7 +101,15 @@ const DetailsPage = ({personalDetails, route}) => {
<Text style={[styles.textP]} numberOfLines={1}>
{moment().tz(details.timezone.selected).format('LT')}
{' '}
{moment().tz(details.timezone.selected).zoneAbbr()}
{isNaN(moment().tz(details.timezone.selected).zoneAbbr()) ? (
Copy link
Contributor

@stitesExpensify stitesExpensify May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind moving the logic here up above the return? So basically make a currentTime var and then here it will just be <Text>{currentTime}</Text>

Copy link
Contributor

@stitesExpensify stitesExpensify left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One quick change, and there appear to be some style problems as well :)

@kaushiktd kaushiktd closed this May 25, 2021
@kaushiktd
Copy link
Contributor Author

duplicate pull request

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

Successfully merging this pull request may close these issues.

Profile - GMT time is not displayed
2 participants