Feature/internationalize date formatting of charts #3381

deligence-dharmendra · 2018-02-05T11:57:58Z

Closes #2900

Changes

change in format date helper file
add getLocaleDateFormat method

Developer checklist

This checklist is to be completed by the PR developer:

Alternative solutions were compared/discussed before writing code
- trade-offs with this solution are considered acceptable
Code in this PR adheres to the project styleguide
This pull request does not decrease project test coverage
If the code changes existing database collection(s), migration has been written
If UI texts are added or changed, all texts are internationalized

Reviewer checklist

Reviewed by: @marla-singer @matleppa

This list is to be completed by the pull request reviewer:

marla-singer · 2018-02-05T12:57:02Z

apinf_packages/dashboard/client/charts/median_response_time/median_response_time.js

@@ -69,7 +72,7 @@ Template.medianResponseTime.onRendered(function () {

    // Get dates
    const labels = chartData.map(dataset => {
-      return moment(dataset.date).format('MM/DD');
+      return moment(dataset.date).format(getLocaleDateFormat());


Not good enough. Get the locale format outside of loop

marla-singer · 2018-02-05T12:57:08Z

apinf_packages/dashboard/client/charts/requests_number/requests_number.js

@@ -94,7 +97,7 @@ Template.requestTimeline.onRendered(function () {

    // Initialization
    const labels = selectedPathData.dates.map(date => {
-      return moment(date).format('MM/DD');
+      return moment(date).format(getLocaleDateFormat());


Get the locale format outside of loop

marla-singer · 2018-02-05T12:57:14Z

apinf_packages/dashboard/client/charts/requests_over_time/requests_over_time.js

@@ -69,7 +72,7 @@ Template.requestsOverTime.onRendered(function () {

    // Get dates
    const labels = chartData.map(dataset => {
-      return moment(dataset.date).format('MM/DD');
+      return moment(dataset.date).format(getLocaleDateFormat());


Get the locale format outside of loop

marla-singer · 2018-02-05T12:57:21Z

apinf_packages/dashboard/client/charts/response_time/response_time.js

@@ -89,7 +92,7 @@ Template.responseTimeTimeline.onRendered(function () {

    // Create labels value
    const labels = selectedPathData.dates.map(date => {
-      return moment(date).format('MM/DD');
+      return moment(date).format(getLocaleDateFormat());


Get the locale format outside of loop

marla-singer · 2018-02-05T12:57:26Z

apinf_packages/dashboard/client/charts/unique_users/unique_users.js

@@ -69,7 +72,7 @@ Template.uniqueUsersOverTime.onRendered(function () {

    // Get dates
    const labels = chartData.map(dataset => {
-      return moment(dataset.date).format('MM/DD');
+      return moment(dataset.date).format(getLocaleDateFormat());


Get the locale format outside of loop

marla-singer

One remarks

marla-singer · 2018-02-05T16:12:14Z

apinf_packages/core/helper_functions/format_date.js

+
+export function getLocaleDateFormat () {
+  // Get locale
+  const locale = window.navigator.userLanguage || window.navigator.language;


Using another command to cover all user's preferred language:

const locale = navigator.languages ? navigator.languages[0] : (navigator.language || navigator.userLanguage)

And please squash all commits to one

matleppa

One change in date.

matleppa · 2018-02-08T07:35:35Z

apinf_packages/core/helper_functions/format_date.js

+  let format = localeData.longDateFormat('L');
+
+  // Remove year part
+  format = format.replace(/.YYYY/, '');


Remove only the year, not the dot before year.

@matleppa For what? The label looks "07.02" and I don't understand why do we need save the last dot?

The label should look "07.02.", because the last dot is part of the date in this form.

Can't agree with you 👀 let's ask other devs about this?

Honestly, I don't know about Finnish date format, but for me the last dot looks sooo strange in the Date

Current realization

Your suggestions

Keep the dot. In Finnish last dot is an indicator of the date. There's now dates without a fot after the month. "14.2." is a date in Finnish. "14.2" is not a date in Finnish.

@preriasusi, That means we go with keep the dot with finish date.

First if you get a date format from a i18n component/library, it is not a good ide ato start clipping it. And also very bad idea to do now some logic according to the language. Isn't there a format for the date without a year that you could use without doing any mods on it?

@preriasusi Nope. We use moment.js and it doesn't provide locale date w/out year

@marla-singer Ok, it seems that it doesn't. So, ok stripping the year away is in this case accettable. But touching the dot in Finnish date is very, very wrong thing to do.

marla-singer · 2018-03-16T10:02:15Z

@matleppa Add case for Finnish locale. Please check and let's merge it

matleppa

Change in comparison.

matleppa · 2018-03-16T12:48:46Z

apinf_packages/core/helper_functions/format_date.js

  let format = localeData.longDateFormat('L');

-  // Remove year part
-  format = format.replace(/.YYYY/, '');
+  if (locale === 'fi') {


Seems not to work.
Should the comparison be
locale === 'fi-FI'
Or use regex to find 'fi'

I remade it by using regex. Please check again. If everything is fine then not merge it yet, I wanna do rebase

matleppa

Unfortunately still some problem.
Does not fill the charts and
gives lots of exceptions about regex.text.

Exception from Tracker afterFlush function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: regex.text is not a function
at getLocaleDateFormat (format_date.js:41)
at Blaze.View. (requests_over_time.js:74)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1934
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1932
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at viewAutorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1931)
at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
at new Tracker.Computation (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:229)
at Object.Tracker.autorun (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:613)
at Blaze.View.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1944)
at Blaze.TemplateInstance.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3604)
at Blaze.TemplateInstance. (requests_over_time.js:69)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3398
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at fireCallbacks (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3394)
at Blaze.View. (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3487)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1845
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1844
at Object.Tracker._runFlush (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:548)
at onGlobalMessage (meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:398)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker afterFlush function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: regex.text is not a function
at getLocaleDateFormat (format_date.js:41)
at Blaze.View. (median_response_time.js:74)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1934
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1932
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at viewAutorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1931)
at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
at new Tracker.Computation (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:229)
at Object.Tracker.autorun (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:613)
at Blaze.View.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1944)
at Blaze.TemplateInstance.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3604)
at Blaze.TemplateInstance. (median_response_time.js:69)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3398
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at fireCallbacks (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3394)
at Blaze.View. (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3487)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1845
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1844
at Object.Tracker._runFlush (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:548)
at onGlobalMessage (meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:398)
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 Exception from Tracker afterFlush function:
meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:942 TypeError: regex.text is not a function
at getLocaleDateFormat (format_date.js:41)
at Blaze.View. (unique_users.js:74)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1934
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1932
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at viewAutorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1931)
at Tracker.Computation._compute (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:339)
at new Tracker.Computation (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:229)
at Object.Tracker.autorun (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:613)
at Blaze.View.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1944)
at Blaze.TemplateInstance.autorun (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3604)
at Blaze.TemplateInstance. (unique_users.js:69)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3398
at Function.Template._withTemplateInstanceFunc (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3744)
at fireCallbacks (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3394)
at Blaze.View. (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:3487)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1845
at Object.Blaze._withCurrentView (blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:2271)
at blaze.js?hash=f33d3dfed63a491d24e3aa07ad66c24b5fe8c761:1844
at Object.Tracker._runFlush (tracker.js?hash=997515fa2d5b0530ba07741da556c4b36963ef3b:548)
at onGlobalMessage (meteor.js?hash=6d285d84547b3dad9717a7c89c664b61b45ea3d8:398)

- Create an additional case fo Finnish locale. Using regex because different browsers handle it different way

marla-singer · 2018-03-22T08:22:59Z

@matleppa Please check again

matleppa

Works as expected.

ghost assigned deligence-dharmendra Feb 5, 2018

ghost added the in progress label Feb 5, 2018

deligence-dharmendra requested review from marla-singer and matleppa February 5, 2018 12:42

deligence-dharmendra added Ready for review and removed in progress labels Feb 5, 2018

marla-singer suggested changes Feb 5, 2018

View reviewed changes

marla-singer removed the Ready for review label Feb 5, 2018

ghost added the in progress label Feb 5, 2018

deligence-dharmendra requested a review from marla-singer February 5, 2018 13:38

deligence-dharmendra added Ready for review and removed in progress labels Feb 5, 2018

marla-singer suggested changes Feb 5, 2018

View reviewed changes

marla-singer removed the Ready for review label Feb 6, 2018

ghost added the in progress label Feb 8, 2018

deligence-dharmendra force-pushed the feature/internationalize-date-formatting-of-charts branch from 27255d8 to dedaf46 Compare February 8, 2018 05:44

deligence-dharmendra requested a review from marla-singer February 8, 2018 05:55

matleppa requested changes Feb 8, 2018

View reviewed changes

marla-singer approved these changes Feb 8, 2018

View reviewed changes

ghost assigned marla-singer Mar 16, 2018

marla-singer added the Ready for review label Mar 16, 2018

matleppa requested changes Mar 16, 2018

View reviewed changes

matleppa removed the Ready for review label Mar 16, 2018

marla-singer added the Ready for review label Mar 21, 2018

matleppa requested changes Mar 21, 2018

View reviewed changes

matleppa removed the Ready for review label Mar 21, 2018

Add internationalization of date formatting for Dashboard charts

72140a7

- Create an additional case fo Finnish locale. Using regex because different browsers handle it different way

marla-singer force-pushed the feature/internationalize-date-formatting-of-charts branch from d431611 to 72140a7 Compare March 22, 2018 08:14

marla-singer added the Ready for review label Mar 22, 2018

matleppa approved these changes Mar 22, 2018

View reviewed changes

matleppa merged commit b784fb0 into develop Mar 22, 2018

ghost removed the in progress label Mar 22, 2018

preriasusi deleted the feature/internationalize-date-formatting-of-charts branch August 29, 2018 08:00

Feature/internationalize date formatting of charts #3381

Feature/internationalize date formatting of charts #3381

Uh oh!

Conversation

deligence-dharmendra commented Feb 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Developer checklist

Reviewer checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marla-singer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matleppa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matleppa Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marla-singer Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marla-singer Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Current realization

Your suggestions

Uh oh!

preriasusi Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marla-singer commented Mar 16, 2018

Uh oh!

matleppa left a comment

Choose a reason for hiding this comment

Uh oh!

matleppa Mar 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matleppa left a comment

Choose a reason for hiding this comment

Uh oh!

marla-singer commented Mar 22, 2018

Uh oh!

matleppa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deligence-dharmendra commented Feb 5, 2018 •

edited

Loading

matleppa Feb 8, 2018 •

edited

Loading

marla-singer Feb 8, 2018 •

edited

Loading

marla-singer Feb 8, 2018 •

edited

Loading

preriasusi Feb 14, 2018 •

edited

Loading

matleppa Mar 16, 2018 •

edited

Loading