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

HXCS-3381 fix date widget #9482

Merged
merged 4 commits into from
Mar 29, 2024
Merged

HXCS-3381 fix date widget #9482

merged 4 commits into from
Mar 29, 2024

Conversation

wideLandscape
Copy link
Contributor

@wideLandscape wideLandscape commented Mar 28, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)

https://hyland.atlassian.net/browse/HXCS-3381

What is the new behaviour?

https://hyland.atlassian.net/browse/HXCS-3381

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

To change the timezone in Chrome open the inspector and click the three dots icon on the bottom panel, then select Sensors from the menu

image

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Adriano Costa seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@@ -15,9 +15,16 @@
* limitations under the License.
*/

import { format, parse, parseISO, isValid, isBefore, isAfter, lightFormat } from 'date-fns';
import { format, parse, parseISO, isValid, isBefore, isAfter } from 'date-fns';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lightFormat doesn't return the desired output when dealing with -XX:XXGMT timezones (eg: USA)

@@ -203,18 +210,19 @@ export class DateFnsUtils {
return new Date(date.getUTCFullYear(), date.getUTCMonth(), date.getUTCDate(), date.getUTCHours(), date.getUTCMinutes(), date.getUTCSeconds());
}

static forceLocal(date: string | Date): Date {
if (typeof date === 'string'){
static forceLocal(date: Date | string): Date {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

forceLocal and forceUtc are meant to display the same date over different timezone.
So, giving a UTC date , the output might be a different Date. But since that date is localized automatically by the browser, the text displayed will be the same.
Eg: 03-24-1983T00:00:00Z localized to USA timezone is 03-23-1983T16:00:00 (notice the absence of Z, and different day and hours values). Instead forceUtc/forceLocal will convert the date to 03-24-1983T00:00:00, different value but same text when displayed

@wideLandscape
Copy link
Contributor Author

I have tested it in the Card View of ADF Demo App. I believe there are some other places to check I'm not aware of

@wideLandscape wideLandscape marked this pull request as ready for review March 28, 2024 12:34
Copy link
Contributor

@eromano eromano left a comment

Choose a reason for hiding this comment

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

Any unit test can be adde here?

@wideLandscape
Copy link
Contributor Author

Any unit test can be adde here?

Is it possible to simulate different timezones in the tests? the main issue here is that tests can be green in some tz and fail in others.

@eromano
Copy link
Contributor

eromano commented Mar 28, 2024

const equalTime = (actual: Date, expected: Date) => actual.getTime() === expected.getTime();
maybe here there are some ideas

'09-02-9999 09:10 AM',
'dd-MM-yyyy hh:mm aa'
);
const parsed = DateFnsUtils.parseDate('09-02-9999 09:10 AM', 'dd-MM-yyyy hh:mm aa');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all these format changes has been done automatically by linter when I did the commit

it('should convert UTC dates to have the same day, month, year displayed in different timezone', () => {
const dateUTC = new Date('Wed Jan 01 2020 00:00:00 GMT+0000');
const dateUSA = new Date('Tue Dec 31 2019 16:00:00 GMT-0800');
const dateJapan = new Date('Wed Jan 01 2020 09:00:00 GMT+0900');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same instant in different timezones

Copy link

sonarcloud bot commented Mar 29, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@DenysVuika
Copy link
Contributor

@wideLandscape can you click the CLA button please to make the CI/CD happy

@DenysVuika DenysVuika merged commit ec02c76 into develop Mar 29, 2024
34 of 35 checks passed
@DenysVuika DenysVuika deleted the fix/HXCS-3381-fix-date-widget branch March 29, 2024 13:59
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.

None yet

4 participants