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

handle valueAsDate to keep time for unchanged date #539

Open
Mattchewone opened this issue Jul 11, 2019 · 5 comments
Open

handle valueAsDate to keep time for unchanged date #539

Mattchewone opened this issue Jul 11, 2019 · 5 comments

Comments

@Mattchewone
Copy link
Contributor

If you set an input with valueAsDate and read the value back you will get a Date object but it will no longer have the same time as the date object that you gave it. It currently doesn't support datetime only date or time.

Example - https://codepen.io/mattchewone-the-looper/pen/EBMRXR?editors=0010

Spec - https://html.spec.whatwg.org/multipage/input.html#concept-input-apply

We could handle valueAsDate to keep the same date object if un-modified so that you don't lose the time aspect.

@matthewp
Copy link
Contributor

matthewp commented Aug 12, 2019

Interesting issue, where does canjs currently do property special casing? I guess it's probably here... https://github.com/canjs/can-attribute-observable/blob/master/behaviors.js

@phillipskevin
Copy link
Contributor

Yes, that's where it is done.

@nlundquist
Copy link
Contributor

nlundquist commented Aug 27, 2019

I ran into this while updating the guides and had a somewhat different recommendation. Rather than handling one scenario, i.e keeping the time from being dropped from the Date so long as a user doesn't make a change via the input, we should instead let the browser drop the time and warn developers about this behavior.

That way, the developer is fully aware of the consequences of using this binding, rather than leaving a potential pitfall when a user modifies a date that already has a time via the input, unexpectedly dropping the time.

So, when a developer binds two-way or parent-to-child to valueAsDate on an input[type=date], and the value set to valueAsDate includes a time, we should throw a warning informing users that input[type=date] drops the time and that the Dates they set on it shouldn't include a time (i.e should have their time set to midnight).

Similarly, when a developer binds two-way or parent-to-child to valueAsDate on an input[type=time], and the value set to valueAsDate includes a date, we should throw a warning informing users that input[type=time] drops the date and that the Dates they set on it shouldn't include a date (i.e should have their date set to 01-01-1970).

@nlundquist
Copy link
Contributor

nlundquist commented Aug 27, 2019

My interest in this issue was due to my belief it was the cause of a "two-way binding changing or converting its value when set" warning when two way binding to valueAsDate. As it turns out, my suggestion above, while helpful to users to prevent unintended stripping of dates / times, won't prevent that warning. Unfortunately neither would Matthew's suggestion.

This is because when setting valueAsDate a copy of the Date object is made. Thus CanJS will always complain that the child is modifying the value since object equality is done by identity.

A CodePen showing valueAsDate creating a copy: https://codepen.io/stealthwang/pen/KKPmoLm?editors=1111

I suspect we'll have to add smarter equality checking for Objects to the code here: https://github.com/canjs/can-bind/blob/fc2f0cec958c224165857c11b0eae06152b7fedf/can-bind.js#L401

@phillipskevin
Copy link
Contributor

The copying must be happening in the setter for that property:

image

I still think creating a specialAttributes behavior for valueAsDate would work to improve the situation, since it would be hit before the setter is called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants