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

fix: Localize Date and Time translations using AM/PM #5748

Merged
merged 23 commits into from
Nov 27, 2020
Merged

fix: Localize Date and Time translations using AM/PM #5748

merged 23 commits into from
Nov 27, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Nov 25, 2020

Fixes #5491

Short description of what this resolves:

Time format is changed to 24hr clock whose languages are not supported by moment

Changes proposed in this pull request:

  • 24 hr clock format is shown if AM/PM is not translated to resp language by moment

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Nov 25, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/dskcvxisg
✅ Preview: https://open-event-frontend-git-trans-date-5491.eventyay.now.sh

@codecov
Copy link

codecov bot commented Nov 25, 2020

Codecov Report

Merging #5748 (fe1151a) into development (53ce749) will decrease coverage by 0.05%.
The diff coverage is 57.14%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5748      +/-   ##
===============================================
- Coverage        23.50%   23.45%   -0.06%     
===============================================
  Files              512      511       -1     
  Lines             5445     5449       +4     
  Branches            63       63              
===============================================
- Hits              1280     1278       -2     
- Misses            4149     4155       +6     
  Partials            16       16              
Impacted Files Coverage Δ
app/controllers/admin/messages/list.js 0.00% <ø> (ø)
app/controllers/admin/sessions/list.js 0.00% <ø> (ø)
app/helpers/general-date.js 66.66% <57.14%> (-33.34%) ⬇️
app/components/tabbed-navigation.js 33.33% <0.00%> (-20.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53ce749...fe1151a. Read the comment docs.

@divyamtayal
Copy link
Member Author

Wherever moment was able to convert it into resp language there we are using as it is otherwise 24hr clock.

Some ScreenShots
Screenshot from 2020-11-25 13-48-24
Screenshot from 2020-11-25 13-48-46
Screenshot from 2020-11-25 13-49-10
Screenshot from 2020-11-25 13-49-33
Screenshot from 2020-11-25 13-52-39
Screenshot from 2020-11-25 13-52-52
Screenshot from 2020-11-25 13-53-27
Screenshot from 2020-11-25 13-53-45
Screenshot from 2020-11-25 13-54-00
Screenshot from 2020-11-25 13-54-24
Screenshot from 2020-11-25 13-54-40
Screenshot from 2020-11-25 13-55-10 Screenshot from 2020-11-25 13-55-32

@divyamtayal
Copy link
Member Author

Screenshot from 2020-11-25 14-07-36
Screenshot from 2020-11-25 14-07-54
Screenshot from 2020-11-25 14-08-08

@mariobehling
Copy link
Member

Thank you. Could you change the following, please:

  • no space in front of any commas
  • change 0:00 to 24:00

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 25, 2020

change 0:00 to 24:00

@mariobehling
change as in change date or format (H:mm to HH:mm) ?

@iamareebjamal
Copy link
Member

We won't be able to change 0:00 to 24:00, that is controlled by moment JS AFAIK

@divyamtayal
Copy link
Member Author

We won't be able to change 0:00 to 24:00, that is controlled by moment JS AFAIK

exactly

.env.example Outdated Show resolved Hide resolved
@iamareebjamal
Copy link
Member

I think there will be conflicting changes in this PR and standardize PR, please work with @maze-runnar to finalize that PR and then work on this PR

@divyamtayal
Copy link
Member Author

@iamareebjamal Pls see the changes, improved the format wherever not standardized, and also replaced everything with general-date.
Pls, let me know to make any changes further.

daretobedifferent18 added 2 commits November 27, 2020 16:08
@divyamtayal
Copy link
Member Author

@iamareebjamal This is done now with all the changes said by you.

@@ -12,9 +12,9 @@
<div class="ui padded segment">
<strong>{{t 'At'}} {{this.event.locationName}}</strong>
<br>
<strong>{{t 'From'}}:</strong> {{header-date this.event.startsAt}}
<strong>{{t 'From'}}:</strong> {{general-date this.event.startsAt '' 'dddd, D MMMM, YYYY h:mm A'}}
Copy link
Member

Choose a reason for hiding this comment

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

I don't remember, was this the standard format which was chosen?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

Copy link
Member Author

@divyamtayal divyamtayal Nov 27, 2020

Choose a reason for hiding this comment

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

What I found in #5747 is that at some places still time format is not standardized like format is not consistent

Copy link
Member

Choose a reason for hiding this comment

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

Please create a Pr to fix that as well

@iamareebjamal iamareebjamal changed the title Date and Time translations using AM/PM fix: Localize Date and Time translations using AM/PM Nov 27, 2020
@auto-label auto-label bot added the fix label Nov 27, 2020
@iamareebjamal iamareebjamal merged commit e80b30f into fossasia:development Nov 27, 2020
@divyamtayal divyamtayal deleted the trans-date-5491 branch November 27, 2020 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date and Time translations using AM/PM
4 participants