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

Initializing a datepicker with an input's value #2387

Closed
nickalbrecht opened this issue Sep 1, 2021 · 19 comments
Closed

Initializing a datepicker with an input's value #2387

nickalbrecht opened this issue Sep 1, 2021 · 19 comments

Comments

@nickalbrecht
Copy link

If the input element for a datepicker has a value attribute, the picker should default to that date when opening the picker, but that doesn't seem to be the case from my testing

@Eonasdan Eonasdan added v6 State: Triage Tickets that need to be triaged. Type: Bug and removed State: Triage Tickets that need to be triaged. labels Sep 1, 2021
Eonasdan added a commit that referenced this issue Sep 6, 2021
@Eonasdan
Copy link
Owner

Eonasdan commented Sep 6, 2021

Hi. I think I fixed this in the commit I just made, can you verify?

@nickalbrecht
Copy link
Author

It works fine when I have a date like 4/21/2021. But if I have a date set that is in a format it does not expect, like 21/4/2021, it will give an error in the console about an incorrect date (totally fine), but the control is now broken, and it will not show no matter what I click/focus on.

When you look into fixing this, can you ensure that even if a date is invalid (unexpected format), it's value is left intact as the value of the input until the user interacts with the TempusDominus control to change it. That way there's no unintentional data loss when used on an Edit screen for existing data, and someone submits changes unrelated to that date field. I haven't tested this, it's just something I wanted to callout.

@Eonasdan
Copy link
Owner

Eonasdan commented Sep 8, 2021

Date parsing is done from javascripts native date object. I'll make sure that the control still works in the case and doesn't change the input vlaue.

@Eonasdan
Copy link
Owner

Eonasdan commented Sep 9, 2021

Hmm seemed to work for me. Can you fork the v6 stackblitz and let me know if you can reproduce it?

@nickalbrecht
Copy link
Author

nickalbrecht commented Sep 9, 2021

For sure, I simply forked yours, and added a value to the input element that wasn't in the expected format
https://stackblitz.com/edit/js-3ukhtk?file=index.html

@Eonasdan
Copy link
Owner

Eonasdan commented Sep 9, 2021

Well I feel dumb. lol. I swear I tried this locally and it worked. :( I try to fix it sorry and thanks.

@nickalbrecht
Copy link
Author

Haha, no worries, I know that feeling when things like that happen :-D

@nickalbrecht
Copy link
Author

nickalbrecht commented Sep 9, 2021

I've got a few others issues I've found while trying to adapt an existing site using TD5, to the new v6 as well. Can't seem to get custom formatting to apply at all (tried both the inputFormat and the inline options), and it doesn't seem like keepOpen: false works as expected? Would you prefer I open new issues for those issues separately? Would having a shared/individual stackblits for each of them help?

@Eonasdan
Copy link
Owner

I'm currently trying to get this in to an existing angular project of mine as well. Dinner "real world" testing. I know it's tedious but could you create an issue for each unrelated item? A single stackblitz will do just fine.

I really appreciate you try this out and reporting bugs.

@nickalbrecht
Copy link
Author

Done, and no worries! Having a compatible datetime picker is the last roadblock I have before I can look at migrating a project that is currently using Bootstrap4, to instead use Bootstrap5. I started doing the testing/migrating once I saw you had an alpha up, so I'm glad that me finding issues helps you track them down :-)

@Eonasdan
Copy link
Owner

Hi can you please check out the latest release? I think I've resolved this.

@nickalbrecht
Copy link
Author

Still gives an error on some invalid dates. I've updated the stackblitz to [email protected] assuming that's a version that has this fix? But putting in an invalid dates such as '21/4/2021' in the input's value still gives an error, and no date picker control to allow a correct date to be chosen.
Also, it seems like it gives the error when a hard limit is surpassed (more than 12 months, more than 31 days), but silently shifts dates oddly when it's validation is conditional on some logic (leap year, days in a given month). The value appears to stay as the invalid date, which is good in my opinion as it'll prevent accidental data modification, but the displayed date is shifted.
For example try using a date of "Feb 29, 2021", or "Sept 31, 2021" in the stackblitz. "Feb 31, 2021" is even more pronounced result.

@Eonasdan
Copy link
Owner

image

I keep testing this by putting your date into the text field instead of setting the value. Sorry.

@nickalbrecht
Copy link
Author

Hehe, no worries. I chuckled at the commit message when I went looking to update the reference to TD in the stackblitz, and found it was simply "oops :(". :-P
I feel your pain, some days are tougher trying to track down bugs than others :-D

Eonasdan added a commit that referenced this issue Oct 7, 2021
@Eonasdan
Copy link
Owner

Eonasdan commented Oct 7, 2021

Hello. Please try your config with this stackblitz. I think I FINALLY for REAL fixed this.

@nickalbrecht
Copy link
Author

nickalbrecht commented Oct 7, 2021

Almost! No more error at any rate. It still silently shifts invalid dates like Sept 31, 2021, Feb 31, 2021.

I noticed this and mentioned it in one of the other issues but I think what I was getting at, is that there should be different behavior between user entered data, and data in the input's value field (assumedly supplied by the server). I'm not sure if the hook for inputParse can distinguish where its input came from?

To me, information supplied by the user should be more aggressively validated. If the date is in an invalid format (21/4/2021), can't be parsed (Sep 1, 2021), doesn't represent a valid date (Feb 31, 2021), the input should be tossed, and force the user to try again (is feedback to the user when this happens beyond the scope of TD, and left up to the consumer of the package?).

If it's assumedly from the server (since it's the input's value attribute), I would expect it to be more lenient, as refusing the value and wiping it out can unintentionally blank bad data when the form containing the picker is later submitted and the user isn't made aware this happened. Again, possible feedback to the user somehow?

To be fair, I'm not entirely sure if my concerns around this are within the scope of the TD control, or should be for the consumer of the TD package (myself in this case) to deal with, but I don't think there's a way to tell where the input being parsed came from currently, so I don't know how a consumer of the package could have different logic in these two scenarios.

@Eonasdan
Copy link
Owner

Eonasdan commented Oct 8, 2021

Unfortunately that is the nature of js's date parsing. Moment had a "strict" option but that seems like we're going down the path of a full dt lib. The parse hooks I added ought to allow for people to use moment or dayjs or w/e to determine what is valid or provide more complex parsing.

I could potentially look at the input attribute instead of input.value on the first go. That might be a way to check that value was set vs input.

I know removing moment mean relaying on the native functions but that still seems better compared to a direct dependency.

@nickalbrecht
Copy link
Author

nickalbrecht commented Oct 8, 2021

Oh, I didn't realize that. Yeah, I just tried it in a console window in chrome, and sure enough, if I do new Date('feb 31, 2021'), it outputs "Mar 3" for the day. Interesting, yeah, not much you can do with that.

As for the other part, just brainstorming here for some way to distinguish between initializing the control versus parsing user input. Perhaps something passed to the inputParse function to indicate if it's called from the control init versus user input. Actually, it looks like currently, this is the hooks object while executing the inputParse. Could that be set to something to help determine the context of why it's parsing. Maybe the event that triggered it (which should give access to the element via .target I think?). It'd be nice to have some way of getting a refence to the <input> element itself, as that'd let dev's implementing custom functions for things like inputParse and inputFormat, to inspect the <input> tag for data attributes or the like.

@Eonasdan
Copy link
Owner

Eonasdan commented Oct 9, 2021

I added the context to both of those functions. You should be able to get the input or the validation methods or anything the picker provides including the widget itself from there.

hooks: {
    inputParse: (context: TempusDominus, value: any) => DateTime;
    inputFormat: (context: TempusDominus, date: DateTime) => string;
  };

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

2 participants