feat: Add fullDateWithWeekday, weekday and weekdayShort formatters #400

philipbulley · 2020-06-25T14:06:23Z

Addresses #399

I've used the following formatter strings:

Library	`fullDateWithWeekday`	`weekday`	`weekdayShort`	Formatter Docs
date-fns	`PPPP`	`EEEE`	`EEE`	date-fns
moment	`dddd, LL` *	`dddd`	`ddd`	Moment.js
luxon	`DDDD`	`cccc`	`ccc`	Luxon
dayjs	`dddd, LL`	`dddd`	`ddd`	Day.js
jalaali	`jYYYY, jMMMM Do, dddd` *	`dddd`	`ddd`	jalaali/moment-jalaali
hijri	`iYYYY, iMMMM Do, dddd` *	`dddd`	`ddd`	xsoh/moment-hijri

*Moment, jalaali and hijri don't have a preset localised formatter string suitable for fullDateWithWeekday, so I have improvised by adding dddd and documented fullDateWithWeekday as being "Partially localized". This may not be ideal across all locales — I see you've done something similar with keyboardDateTime12h, etc. I imagine this won't be a problem.
I don't understand the output of Hijiri and Jalaali utils, so can't verify. Same applies to the Russian in the formats.test. I've used a 'best guess' and have just copied the received values directly into the tests. Do you have anyone who can verify?
For clarity, I've aligned the examples in the docs with the test assertions (moment en-US) and figured I'd make a couple of grammar improvements on the README whilst I was there (hope you don't mind)
Prettier has reformatted a bunch of lines, not sure why this formatting wasn't in place already? I didn't change any prettier config.

Please let me know if you'd like to see any changes, or feel free to edit.

codecov-commenter · 2020-06-25T14:08:49Z

Codecov Report

Merging #400 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #400   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          692       692           
  Branches        76        76           
=========================================
  Hits           692       692

Impacted Files	Coverage Δ
...kages/date-fns-jalali/src/date-fns-jalali-utils.ts	`100.00% <ø> (ø)`
packages/date-fns/src/date-fns-utils.ts	`100.00% <ø> (ø)`
packages/dayjs/src/dayjs-utils.ts	`100.00% <ø> (ø)`
packages/hijri/src/hijri-utils.ts	`100.00% <ø> (ø)`
packages/jalaali/src/jalaali-utils.ts	`100.00% <ø> (ø)`
packages/luxon/src/luxon-utils.ts	`100.00% <ø> (ø)`
packages/moment/src/moment-utils.ts	`100.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 442965f...a0d1a1a. Read the comment docs.

… + general doc tidy

dmtrKovalenko

Please run yarn prettier --write './packages/**/*.{js,jsx,ts,tsx,json,css,md,mdx}'.
And also what was the motivation of updating example comments?

Russian output is ok. For hijiri and jallali will wait for some native speaker open issue 🤷‍♂️

Or allow changing from maintainers in PR settings.

README.md

__tests__/jalaali.test.ts

packages/jalaali/src/jalaali-utils.ts

Co-authored-by: Dmitriy Kovalenko <[email protected]>

Co-authored-by: Shahab <[email protected]>

philipbulley · 2020-07-01T10:11:52Z

@c0m1t thanks for taking a look at the Jalaali! Unfortunately, the test is currently failing based on the new formatter string. Would you be able to take a quick look and either update the formatter string or the test? Much appreciated :)

…sertions

philipbulley · 2020-07-01T11:00:15Z

@dmtrKovalenko, thanks for reviewing!

And also what was the motivation of updating example comments?

The previous examples were inaccurate so (in most cases) I have aligned the examples with the test assertions. For example:

Previous fullDate example docs:

date-io/packages/core/IUtils.d.ts

Lines 2 to 3 in e8ce35c

    
           /** Localized full date, useful for accessibility @example "January 1st, 2019" */ 
        
           fullDate: TLibFormatToken;

Test assertion (unchanged):

date-io/__tests__/formats.test.ts

Line 40 in e8ce35c

    
               ${"fullDate"}            | ${"Feb 1, 2020"}          | ${"1 февр. 2020 г."}

Note that the month abbreviation is used, whereas the docs show that the full month is used. This inaccuracy is a show stopper for a11y text-to-speech, hence the update.

I'll comment on each change with the reason for the change (as some vary). Please let me know if there are any issues with any of the individual changes. Thanks!

philipbulley

Rationale for individual doc changes...

philipbulley · 2020-07-01T10:29:10Z

packages/core/IUtils.d.ts

 export interface DateIOFormats<TLibFormatToken = string> {
-  /** Localized full date, useful for accessibility @example "January 1st, 2019" */
+  /** Localized full date @example "Jan 1, 2019" */


Prev example was inaccurate and did not match test assertion

philipbulley · 2020-07-01T10:32:14Z

packages/core/IUtils.d.ts

  normalDate: TLibFormatToken;
-  /** Date format string with weekday, month and day of month @example "Wed, Jan 1st" */
+  /** Date format string with weekday, month and day of month @example "Wed, Jan 1" */


Prev example was inaccurate and did not match test assertion

philipbulley · 2020-07-01T10:32:47Z

packages/core/IUtils.d.ts

  normalDateWithWeekday: TLibFormatToken;
-  /** Shorter day format @example "1 January" */
+  /** Shorter day format @example "Jan 1" */


Prev example was inaccurate and did not match test assertion

philipbulley · 2020-07-01T10:36:59Z

packages/core/IUtils.d.ts

@@ -15,37 +21,41 @@ export interface DateIOFormats<TLibFormatToken = string> {
  monthShort: TLibFormatToken;
  /** Short month format string @example "January 2018" */
  monthAndYear: TLibFormatToken;
-  /** Month with date format string @example "January 1st" */
+  /** Month with date format string @example "January 1" */


Prev example was inaccurate and did not match test assertion

philipbulley · 2020-07-01T10:37:03Z

packages/core/IUtils.d.ts

  dayOfMonth: TLibFormatToken;
  /** Hours format string @example "11" */
  hours12h: TLibFormatToken;
  /** Hours format string @example "23" */
  hours24h: TLibFormatToken;
-  /** Minutes format string @example "59" */
+  /** Minutes format string @example "44" */


Aligned with test assertion (no real need in this case, but why not 😄 ).

philipbulley · 2020-07-01T10:50:20Z

packages/core/IUtils.d.ts

  fullTime24h: TLibFormatToken;
-  /** Date & time format string with localized time @example "2018, Jan 1st 11:44 PM" */
+  /** Date & time format string with localized time @example "Jan 1, 2018 11:44 PM" */


Prev example was inaccurate and did not match test assertion

philipbulley · 2020-07-01T10:52:17Z

packages/core/IUtils.d.ts

  fullDateTime24h: TLibFormatToken;
-  /** Localized keyboard input friendly date format @example "2019/01/01" */
+  /** Localized keyboard input friendly date format @example "02/13/2020 */


Prev example was inaccurate for en-US. However, I've deviated from the test assertion to better imply the mm/dd/yyyy order here (ie. 13 implies dd as cannot be mm).

philipbulley · 2020-07-01T10:52:22Z

packages/core/IUtils.d.ts

  keyboardDate: TLibFormatToken;
-  /** Localized keyboard input friendly date/time format @example "2019/01/01 23:44" */
+  /** Localized keyboard input friendly date/time format @example "02/13/2020 23:44" */


Prev example was inaccurate for en-US. However, I've deviated from the test assertion to better imply the mm/dd/yyyy order here (ie. 13 implies dd as cannot be mm).

philipbulley · 2020-07-01T10:53:59Z

packages/core/IUtils.d.ts

  keyboardDateTime: TLibFormatToken;
-  /** Not Localized keyboard input friendly date/time 12h format @example "2019/01/01 11:44 PM" */
+  /** Partially localized keyboard input friendly date/time 12h format @example "02/13/2020 11:44 PM" */


Prev example was inaccurate for en-US. However, I've deviated from the test assertion to better imply the mm/dd/yyyy order here (ie. 13 implies dd as cannot be mm).

I've also changed Not Localized to Partially localized to better reflect the use of the localized formatter for the date part.

philipbulley · 2020-07-01T10:54:16Z

packages/core/IUtils.d.ts

  keyboardDateTime12h: TLibFormatToken;
-  /** Not localized keyboard input friendly date/time 24h format @example "2019/01/01 23:44" */
+  /** Partially localized keyboard input friendly date/time 24h format @example "02/13/2020 23:44" */


Prev example was inaccurate for en-US. However, I've deviated from the test assertion to better imply the mm/dd/yyyy order here (ie. 13 implies dd as cannot be mm).

I've also changed Not Localized to Partially localized to better reflect the use of the localized formatter for the date part.

dmtrKovalenko · 2020-07-01T15:12:34Z

What the status of this PR? CI is failing. Would you be able to finish it?
If no please allow changes from maintainers and I will finish it by myself, thx!

philipbulley · 2020-07-01T22:37:01Z

@dmtrKovalenko the failing test is due to the Jalaali change by @c0m1t (I've requested they take a look). If you're confident enough to fix it, please do, either:

The formatter string should be changed to match the assertion
OR the assertion should be updated to match the formatter string

As I don't read the language, I can't tell which of these is correct.

ALSO:
@c0m1t suggests that the already existing fullDate and fullDateTime are incorrect too but correcting it would represent a breaking change. It's worth noting that after @c0m1t's change, fullDate and fullDateWithWeekday don't use the same ordering:

fullDate: "jYYYY, jMMMM Do",  // year at beginning with comma
fullDateWithWeekday: "dddd Do jMMMM jYYYY",  // year at end, no comma

🤷‍♂️ 🤷‍♂️ 🤷‍♂️ 😄

(Btw... Allow edits by maintainers has been checked since PR creation, is it not allowing you to commit?)

c0m1t · 2020-07-02T06:27:24Z

@c0m1t thanks for taking a look at the Jalaali! Unfortunately, the test is currently failing based on the new formatter string. Would you be able to take a quick look and either update the formatter string or the test? Much appreciated :)

I forgot , after day name, I updated the formatter string.

I don't know if people use the fullDate format as it is in code (month before day of month), but as a native speaker I never heard it in that order, it is always day of month before month. If there has not been any issues or pull requests about this format yet, it probably means it is fine (it might be better to avoid breaking changes).

But this is now how we made date-io/date-fns-jalali, we always put day of month before month.

dmtrKovalenko · 2020-07-06T10:49:26Z

I am not speaking those languages too, so I`d prefer just fix the tests as is (already done it) and wait for somebody (cc @c0m1t 🙏) open a PR that fixes translations...

dmtrKovalenko · 2020-07-06T10:59:05Z

Thanks @philipbulley for your awesome input! ❤️

philipbulley · 2020-07-08T14:23:36Z

@dmtrKovalenko no probs, it's my pleasure 👍 Thanks also to @c0m1t !

philipbulley added 2 commits June 25, 2020 14:42

feat: Add fullDateWithWeekday, weekday and weekdayShort formatters

1486cc1

docs: Add fullDateWithWeekday, weekday and weekdayShort formatters

34c388c

philipbulley mentioned this pull request Jun 25, 2020

Add formatters: fullDateWithWeekday, weekday and weekdayShort #399

Closed

docs: aligning DateIOFormats docs with test assertions (moment en-US)…

273d031

… + general doc tidy

dmtrKovalenko requested changes Jun 27, 2020

View reviewed changes

dmtrKovalenko reviewed Jun 27, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

c0m1t reviewed Jun 29, 2020

View reviewed changes

__tests__/jalaali.test.ts Outdated Show resolved Hide resolved

packages/jalaali/src/jalaali-utils.ts Outdated Show resolved Hide resolved

philipbulley and others added 3 commits July 1, 2020 10:59

Update README.md

fa7146a

Co-authored-by: Dmitriy Kovalenko <[email protected]>

Update Jalaali fullDateWithWeekday

23090d4

Co-authored-by: Shahab <[email protected]>

chore: run prettier across all files

642e46d

philipbulley added 2 commits July 1, 2020 11:41

docs: remove ordinal from fullDateTime example, aligning with test as…

b8c7e91

…sertions

docs: update keyboardDate* examples to better imply mm/dd/yyyy

48721be

philipbulley commented Jul 1, 2020

View reviewed changes

Fix jalaali tests

c9329de

dmtrKovalenko added 2 commits July 6, 2020 13:50

Merge branch 'master' into add-a11y-formats

c5d249b

Add support of new format to date-fns-jalaali

a0d1a1a

dmtrKovalenko merged commit 6c02179 into dmtrKovalenko:master Jul 6, 2020

This was referenced Mar 15, 2021

Bump @date-io/moment from 1.3.13 to 2.10.8 in /nms/app bramwelt/magma#32

Closed

Bump @date-io/date-fns from 1.3.13 to 2.10.8 in /client shadoo77/cv-creator#8

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add fullDateWithWeekday, weekday and weekdayShort formatters #400

feat: Add fullDateWithWeekday, weekday and weekdayShort formatters #400

philipbulley commented Jun 25, 2020 •

edited

Loading

codecov-commenter commented Jun 25, 2020 •

edited

Loading

dmtrKovalenko left a comment •

edited

Loading

philipbulley commented Jul 1, 2020

philipbulley commented Jul 1, 2020

philipbulley left a comment •

edited

Loading

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

philipbulley Jul 1, 2020

dmtrKovalenko commented Jul 1, 2020

philipbulley commented Jul 1, 2020 •

edited

Loading

c0m1t commented Jul 2, 2020 •

edited

Loading

dmtrKovalenko commented Jul 6, 2020

dmtrKovalenko commented Jul 6, 2020

philipbulley commented Jul 8, 2020

feat: Add fullDateWithWeekday, weekday and weekdayShort formatters #400

feat: Add fullDateWithWeekday, weekday and weekdayShort formatters #400

Conversation

philipbulley commented Jun 25, 2020 • edited Loading

codecov-commenter commented Jun 25, 2020 • edited Loading

Codecov Report

dmtrKovalenko left a comment • edited Loading

Choose a reason for hiding this comment

philipbulley commented Jul 1, 2020

philipbulley commented Jul 1, 2020

philipbulley left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmtrKovalenko commented Jul 1, 2020

philipbulley commented Jul 1, 2020 • edited Loading

c0m1t commented Jul 2, 2020 • edited Loading

dmtrKovalenko commented Jul 6, 2020

dmtrKovalenko commented Jul 6, 2020

philipbulley commented Jul 8, 2020

philipbulley commented Jun 25, 2020 •

edited

Loading

codecov-commenter commented Jun 25, 2020 •

edited

Loading

dmtrKovalenko left a comment •

edited

Loading

philipbulley left a comment •

edited

Loading

philipbulley commented Jul 1, 2020 •

edited

Loading

c0m1t commented Jul 2, 2020 •

edited

Loading