Skip to content

fix angular material date control selecting the wrong date #2035

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.

Sign up for GitHub

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

Jump to bottom
Closed
TheMicDiet wants to merge 3 commits into from
Closed

fix angular material date control selecting the wrong date #2035

TheMicDiet wants to merge 3 commits into from

Conversation

@TheMicDiet
Copy link

@TheMicDiet TheMicDiet commented Oct 15, 2022

replace toISOString(), which formats dates based on the UTC time zone with Angular DatePipe, which works with local time zones

fix date control selecting the wrong date eclipsesource#2033
178d533 
replace toISOString(), which formats dates based on the UTC time zone with Angular DatePipe, which works with local time zones
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Oct 21, 2022

Coverage Status

Coverage increased (+0.05%) to 84.404% when pulling 178d533 on TheMicDiet:master into 20e9e0f on eclipsesource:master.

lucas-koehler
lucas-koehler requested changes Oct 21, 2022
View reviewed changes
Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

I think this only fixes part of the problem. While the display now correctly works for timezones UTC and UTC+x, it still shows the wrong date for UTC-x timezones (e.g. in the US). As far as I saw it, the storage of the date now works correctly but I guess there might be additional problems converting the date back when showing it.

@sdirix sdirix linked an issue Nov 29, 2022 that may be closed by this pull request
Angular date picker: selected date is off by one day #2033
Closed
@sdirix sdirix changed the title fix angular material date control selecting the wrong date #2033 fix angular material date control selecting the wrong date Nov 29, 2022
@sdirix
Copy link
Member

sdirix commented Dec 5, 2022

@TheMicDiet Will you take a look and handle the review comment by @lucas-koehler ?

@TheMicDiet
Copy link
Author

TheMicDiet commented Dec 5, 2022

@sdirix yes, it should work now

sdirix
sdirix requested changes Jan 5, 2023
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Thanks for the update! However I would prefer to use something else than moment. Moment is very large which is why we removed it recently from the other renderer sets. Can we also use dayjs here?

@netlify
Copy link

netlify bot commented Jan 17, 2023 •
edited
Loading

❌ Deploy Preview for jsonforms-examples failed.

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/63ec9f3dbf2c2e0e1b861827

@TheMicDiet
Copy link
Author

TheMicDiet commented Jan 17, 2023

@sdirix I managed to replace moment with dayjs. However, I had to increase the target level in the tsconfig to es6, otherwise the date adapter wouldn't work. Is that an issue? Also, the date format is currently hard-coded to YYYY-MM-DD. I've seen that it is possible to change the date format via the UI schema in the React implementation. I've tried to implement this for the Angular date picker, but I haven't found a nice way to configure the formatting of an individual Angular date picker component yet :/

@sdirix
Copy link
Member

sdirix commented Feb 9, 2023

Sorry for the late reply! I took a look at the new code.

I managed to replace moment with dayjs.

Looks good!

However, I had to increase the target level in the tsconfig to es6, otherwise the date adapter wouldn't work. Is that an issue?

Do you know any specifics of why it didn't work? Some class problem with es5?

As we are targeting later Angular versions I think this change should be fine. @lucas-koehler what do you think?

Also, the date format is currently hard-coded to YYYY-MM-DD. I've seen that it is possible to change the date format in the React implementation. I've tried to implement this for the Angular date picker, but I haven't found a nice way to configure the formatting of an individual Angular date picker component yet :/

We had the same problems with localizing the data adapter which then influenced the whole app outside of JSON Forms too. Sadly I'm not aware of a good local solution at the moment.

@lucas-koehler
Copy link
Contributor

lucas-koehler commented Feb 13, 2023

However, I had to increase the target level in the tsconfig to es6, otherwise the date adapter wouldn't work. Is that an issue?

Do you know any specifics of why it didn't work? Some class problem with es5?

As we are targeting later Angular versions I think this change should be fine. @lucas-koehler what do you think?

For me, updating to ES6 is fine. It seems to be supported by every reasonably current browser anyway :) https://caniuse.com/?search=es6

@TheMicDiet
Copy link
Author

TheMicDiet commented Feb 13, 2023 •
edited
Loading

Sorry for the late reply! I took a look at the new code.

I managed to replace moment with dayjs.

Looks good!

However, I had to increase the target level in the tsconfig to es6, otherwise the date adapter wouldn't work. Is that an issue?

Do you know any specifics of why it didn't work? Some class problem with es5?

As we are targeting later Angular versions I think this change should be fine. @lucas-koehler what do you think?

Also, the date format is currently hard-coded to YYYY-MM-DD. I've seen that it is possible to change the date format in the React implementation. I've tried to implement this for the Angular date picker, but I haven't found a nice way to configure the formatting of an individual Angular date picker component yet :/

We had the same problems with localizing the data adapter which then influenced the whole app outside of JSON Forms too. Sadly I'm not aware of a good local solution at the moment.

Do you know any specifics of why it didn't work? Some class problem with es5?

If I run the examples with es5, I get the following error message:

ERROR TypeError: Class constructor NativeDateAdapter cannot be invoked without 'new'
    at new DayJsDateAdapter (dayjs-date-adapter.ts:36:1)

Changing the target level to es6 seems to solve this problem.

@sdirix
Copy link
Member

sdirix commented Jul 11, 2023

  1. Closed as we updated to pnpm with Upgrade build stack to pnpm, Node 16, Lerna 6 #2148

@sdirix sdirix closed this Jul 11, 2023
@LukasBoll LukasBoll mentioned this pull request Feb 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucas-koehler lucas-koehler lucas-koehler requested changes

@sdirix sdirix sdirix requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Angular date picker: selected date is off by one day

5 participants

@TheMicDiet @CLAassistant @coveralls @sdirix @lucas-koehler