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

Upgrade d3 time format to expose new formatting options for weekdays, weeks and quarters #5026

Merged
merged 6 commits into from
Jul 30, 2020

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jul 28, 2020

Supersedes #5023 and resolves #5009 by upgrading d3-time-format to the latest version.

@plotly/plotly_js

archmoj added 2 commits July 28, 2020 17:57
- support formatting options e.g. quarters, weeks and weekdays
- update description link to list various time format options
@alexcjohnson
Copy link
Collaborator

The fix looks great. But for low-level non-graphical items like this I'd like us to get away from using image tests. This one should just add a few more test cases to this test:

it('should accept custom formats using d3 specs even for world cals', function() {

That's both faster (in terms of our overall test suite time going forward) and more complete: we can easily add arbitrarily many cases, and it'll also tell us how the new fields play with world calendars - no need to fix anything that might come up there, I just want to know what we do and do not support, like:

'Mesori \'28 WOY:## DOW:##' // world-cals doesn't support U or w

@archmoj archmoj added the bug something broken label Jul 29, 2020
@@ -571,15 +571,19 @@ describe('dates', function() {
],
[
'%B \'%y WOY:%U DOW:%w',
'August \'12 WOY:32 DOW:1',
'August \'12 WOY:33 DOW:1',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed bug reported in d3/d3-time-format#62.

'Mon Aug 13 06:19:34 2012 && 08/13/2012 && .57 .5678',
'Pes Meso 7 06:19:34 1728 && 12/07/1728 && .57 .5678'
'8/13/2012, 6:19:34 AM && 8/13/2012 && .57 .5678',
'Pes Meso 7 6:19:34 AM 1728 && 12/07/1728 && .57 .5678'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://github.com/d3/d3/blob/master/CHANGES.md#time-formats-d3-time-format
The default U.S. English locale now uses 12-hour time and a more concise representation of the date. This aligns with local convention and is consistent with date.toLocaleString in Chrome, Firefox and Node

var now = new Date;
d3.timeFormat("%c")(new Date); // "6/23/2016, 2:01:33 PM"
d3.timeFormat("%x")(new Date); // "6/23/2016"
d3.timeFormat("%X")(new Date); // "2:01:38 PM"

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Excellent! Thanks for the test updates! 💃

@archmoj archmoj merged commit 0426df3 into master Jul 30, 2020
@archmoj archmoj deleted the new-d3-time-format branch July 30, 2020 13:32
@archmoj archmoj mentioned this pull request Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatting for weeks and quarters
2 participants