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

[Heatmap] Localize Tooltip Date formatting #162

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

kimili
Copy link
Contributor

@kimili kimili commented Apr 22, 2018

Explanation About What Code Achieves:

The current date formatting in the heat map tooltips is hard coded in a way that makes it odd for some locales. This patch makes the date formatting in the heat map tooltips nicer. Specifically:

  • It displays the date using the end user's browser locale. This is achieved using the Date object's toLocaleDateString() method.
  • The format is configurable when initializing a chart, based on the toLocaleDateString() options parameter.
Screenshots/GIFs:

The default formatting in the en-US locale

screenshot 2018-04-22 01 02 21

The default formatting in the en-GB locale

screenshot 2018-04-22 01 06 57

@frappe frappe deleted a comment from coveralls Apr 22, 2018
@frappe frappe deleted a comment from coveralls Apr 22, 2018
@pratu16x7 pratu16x7 changed the base branch from master to develop April 23, 2018 09:16
@pratu16x7 pratu16x7 changed the base branch from develop to master April 23, 2018 09:17
@pratu16x7
Copy link
Contributor

@kimili Could you send this to develop?

@kimili kimili changed the base branch from master to develop April 23, 2018 10:57
@kimili kimili force-pushed the feature/localize-date-heatmap-tooltip branch 2 times, most recently from 80390c7 to 750be23 Compare April 23, 2018 13:37
kimili added 2 commits April 23, 2018 09:38
# Conflicts:
#	dist/frappe-charts.min.cjs.js
#	dist/frappe-charts.min.cjs.js.map
#	dist/frappe-charts.min.esm.js
#	dist/frappe-charts.min.esm.js.map
#	dist/frappe-charts.min.iife.js
#	dist/frappe-charts.min.iife.js.map
#	docs/assets/js/frappe-charts.min.js
#	docs/assets/js/frappe-charts.min.js.map
#	docs/assets/js/index.min.js
#	docs/assets/js/index.min.js.map
@kimili
Copy link
Contributor Author

kimili commented Apr 23, 2018

@pratu16x7 - Done. I was having some trouble with conflicts in the compiled files, but I ended up resolving them with the changes that are currently in the develop branch.

@ghost
Copy link

ghost commented Jan 22, 2019

Is there any new progress here? I'm also interested in this adaptation because I don't like American formatting.

@scmmishra scmmishra self-assigned this May 22, 2019
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.

5 participants