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

Time picker returns 00:00 when clear the value. #8548

Closed
cleverdog opened this issue Nov 10, 2020 · 6 comments · Fixed by #9160
Closed

Time picker returns 00:00 when clear the value. #8548

cleverdog opened this issue Nov 10, 2020 · 6 comments · Fixed by #9160
Assignees

Comments

@cleverdog
Copy link

Description

Describe the issue.

  • igniteui-angular version: 10.2.1
  • browser: Chrome

Steps to reproduce

  1. Open the stackblitz, also open the console panel.
    https://stackblitz.com/edit/timepicker-null?file=src%2Fapp%2Ftimepicker-sample-1%2Ftimepicker-sample-1.component.ts
  2. Set some time to the time picker.
  3. Click the close icon to clear the set time.

Result

Time picker returns 00:00 as value.

Expected result

Time picker returns null the same as the initial value.

@cleverdog cleverdog added the 🐛 bug Any issue that describes a bug label Nov 10, 2020
@Lipata Lipata assigned Lipata and jackofdiamond5 and unassigned Lipata Nov 19, 2020
@kdinev
Copy link
Member

kdinev commented Nov 25, 2020

The ngModel bound to the timepicker is undefined, which technically breaks your sample before you even begin working with the component (turn on strict template checks and this won't even compile). If you define public time = null in the component, you will see you get console errors before any editing happens, as the value is not a Date object, which is the type expected by the time picker, and regardless of what edits you do, the ngModel won't change from null. This is not a bug, instead the component is not used as intended.

@jackofdiamond5
Copy link
Member

The TimePicker was initially designed with the mindset of never having a falsy value. This is the reason why if you clear the editor, the picker will emit a value with hours, minutes and seconds set to 0.

@kdinev While I agree that setting the TimePicker's ngModel to null, undefined or InvalidDate does not count as valid values (ignoring the fact that not declaring time in this case breaks the sample), I think the TimePicker should be capable of gracefully handling such values as they are not completely out of the question in an app. If you add the member public time: Date = null; and attempt to type something in the editor, the TimePicker throws Cannot read property X of null. This happens on each keystroke until the editor is filled in with a valid time string that the picker can parse. I don't think this should be happening. Instead, I think it should be updating the model with whatever new value is presented (providing it's a full and valid date). This means that it should not even attempt to parse the input and update the model until the editor is completely filled in. Furthermore, if you have the exact same scenario but you attempt to spin values in the editor, the picker does not throw. To me, it seems like we have an issue related to the fact that, at the moment, the TimePicker does not properly handle all possible initial values for ngModel. And that it attempts to parse the partially filled in value in its editor. While these may not be necessarily related to this issue in particular, I believe they are issues in their own right. And could be one of the things that we have to discuss in #6482 What do you think?

@kdinev
Copy link
Member

kdinev commented Nov 26, 2020

@jackofdiamond5 I would actually disagree with you. Since the timepicker is a component we created with the intention of modifying an existing Date object's time portion. Without a bound date, the timepicker does not present a UI where you can select the date portion of the object and thus should not be responsible for creating the Date object itself. The timepicker itself doesn't have sufficient context for determining what the date portion of a Date object should be and without it the object is incomplete.

The usual use cases are either the timepicker is presented after a date is picked from a datepicker, for modification of the time portion of the value coming from the datepicker, or a timepicker is presented in a UI with an already existing Date object that it;s bound to, populated from a service or created in another application view.

@jackofdiamond5
Copy link
Member

@kdinev

Without a bound date, the timepicker does not present a UI where you can select the date portion of the object and thus should not be responsible for creating the Date object itself.

As obvious as it is I hadn't thought about that. So I guess it's an expected behavior that during typing the TimePicker throws if a falsy value is bound to its model. It still spawns a Date object when you spin, though, I suppose this is better than throwing errors. Either way I think it's something that has to be resolved on application level.

@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Jan 26, 2021
@jackofdiamond5 jackofdiamond5 removed the status: inactive Used to stale issues and pull requests label Jan 26, 2021
@github-actions
Copy link

There has been no recent activity and this issue has been marked inactive.

@github-actions github-actions bot added the status: inactive Used to stale issues and pull requests label Mar 28, 2021
@Lipata Lipata removed the status: inactive Used to stale issues and pull requests label Mar 29, 2021
@Lipata Lipata mentioned this issue Apr 14, 2021
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants