-
Notifications
You must be signed in to change notification settings - Fork 115
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
A user can set a due date #27 #78
A user can set a due date #27 #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately you did not adapt the tests. They are currently failing because of the new changes.
}); | ||
|
||
constructor(@Inject('TaskService') private taskService: TaskService) { } | ||
|
||
onSubmit(): void { | ||
this.taskService.create(this.taskForm.value.name).subscribe(task => { | ||
if(this.taskForm.value.dueDate && this.taskForm.value.time){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use the patchValue method here and move the setting of the time to date into a specific private method. Like:
` onSubmit(): void {
if (this.taskForm.value.dueDate && this.taskForm.value.time) {
this.taskForm.patchValue({dueDate: this.setTimeToDate(this.taskForm.value.dueDate, this.taskForm.value.time)});
}
this.taskService.create(this.taskForm.value.name, this.taskForm.value.dueDate).subscribe(task => {
this.created.emit(task);
this.taskForm.reset();
});
}
private setTimeToDate(date: Date, time: string): string {
const hour = +time.split(':')[0];
const minute = +time.split(':')[1];
date.setHours(hour, minute);
return date.toISOString();
}`
@@ -20,7 +20,7 @@ export interface TaskService { | |||
* @param name the task's name | |||
* @returns an `Observable` holding the created task | |||
*/ | |||
create(name: string): Observable<Task>; | |||
create(name: string, dueDate: Date): Observable<Task>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dueDate should be optional in my opinion.
I implemented the due date as described in the opened issue #27 .
I modified in scss files to be more responsiveness.