Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

feature/COR-1369-date-formatting-adjustments #4609

Merged
merged 2 commits into from
Jan 31, 2023

Conversation

VWSCoronaDashboard26
Copy link
Contributor

Summary

  • added new WeekdayLong formatting option for appending the year to dates, resulting in 'day daynumber month year' format;
  • adjusted all files/locations that was either using WeekdayMedium formatting or using no formatting (for examples when dates were part of a text within Sanity);

Notes

  • I know I have touched a plethora of files and that these might need further readjustments based on magic numbers or other conventions, but since there are already some pull requests for this I decided to not pick this up (aside from the fact that I personally think that would be well OOS for this feature);
  • Main changes are either adding the weekday-long format or changing existing format styles to weekday-long; other changes in files are mainly due to Prettier applying its formatting rules;

Screenshots

The screenshots only cover bits of the dashboard and give an impression more so than a full overview of everything that has changed. This is because, as you can see, quite a few files have changed (and some files impact even more files).

Before

Toggle before screenshots

COR-1369_before_1_NL
COR-1369_before_2_NL
COR-1369_before_1_EN
COR-1369_before_2_EN

After

Toggle after screenshots

COR-1369_after_1_NL
COR-1369_after_2_NL
COR-1369_after_1_EN
COR-1369_after_2_EN

Copy link
Contributor

@APW26 APW26 left a comment

Choose a reason for hiding this comment

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

Are we sure that this is desired?
image

Seems a bit repetitive.

In my opinion, I would imagine if it's the same year it would be something like 2nd January - 8th January 2023.

Copy link
Contributor

@APW26 APW26 left a comment

Choose a reason for hiding this comment

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

Approved but please see my comments.

@VWSCoronaDashboard26
Copy link
Contributor Author

Are we sure that this is desired? image

Seems a bit repetitive.

In my opinion, I would imagine if it's the same year it would be something like 2nd January - 8th January 2023.

While I (partly) agree, I would argue that we would want to keep this as verbose as possible due to the fact that would have to introduce some logic that checks in date X is in a different year than date Y, which is not mentioned or required according to the acceptance criteria of the feature: it simply states that all dates (from Sanity) should show in a certain format: 'day day-number month year'.

…ng the year to dates, resulting in 'day daynumber month year'; adjusted all files/locations that was either using WeekdayMedium formatting or using no formatting (for examples when dates were part of a text within Sanity);
@VWSCoronaDashboard26 VWSCoronaDashboard26 force-pushed the feature/COR-1369-date-formatting-adjustments branch from 543733b to def1d6b Compare January 19, 2023 11:08
Copy link
Contributor

@Jorrik-Klijnsma-Work Jorrik-Klijnsma-Work left a comment

Choose a reason for hiding this comment

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

See my comments

Copy link
Contributor

@Amber-Taal-Work Amber-Taal-Work left a comment

Choose a reason for hiding this comment

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

LGTM! Nothing blocking, small nitpicky comments.

@APW26 APW26 merged commit 22eee97 into develop Jan 31, 2023
@APW26 APW26 deleted the feature/COR-1369-date-formatting-adjustments branch January 31, 2023 13:17
@APW26 APW26 mentioned this pull request Jan 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants