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.

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

fix: (core) Datepicker and DateTimePicker: Respect disabled/block function by inputs #1787

Merged
merged 6 commits into from
Jan 13, 2020
Merged

fix: (core) Datepicker and DateTimePicker: Respect disabled/block function by inputs #1787

merged 6 commits into from
Jan 13, 2020

Conversation

JKMarkowski
Copy link
Contributor

@JKMarkowski JKMarkowski commented Jan 8, 2020 •
edited

Please provide a link to the associated issue.

fixes: #1627

Please provide a brief summary of this pull request.

There is special behaviour for Date Picker and Datetime Picker, which doesn't allow to pick disabled or blocked date by writing it in input. Before it was completely allowed.
I also tried to follow behaviour from https://sapui5.hana.ondemand.com/#/entity/sap.m.DatePicker/sample/sap.m.sample.DatePicker

I also removed useless inputs form DateTimePicker

Please check whether the PR fulfills the following requirements

Documentation checklist:

@JKMarkowski JKMarkowski added this to the Sprint 28 - Sydney milestone Jan 8, 2020
@JKMarkowski JKMarkowski added this to In progress in Development via automation Jan 8, 2020
@netlify
Copy link

netlify bot commented Jan 8, 2020 •
edited

Deploy preview for fundamental-ngx ready!

Built with commit 0529d35

https://deploy-preview-1787--fundamental-ngx.netlify.com

InnaAtanasova
InnaAtanasova reviewed Jan 8, 2020
View reviewed changes
apps/docs/src/app/core/component-docs/date-picker/date-picker-docs.component.html Outdated
@@ -74,7 +74,9 @@
<fd-docs-section-title [id]="'combobox-reactiveForm'" [componentName]="'datePicker'">
Date Picker in Reactive Form
</fd-docs-section-title>
<description>The date-picker component may also be used within Angular Reactive Forms.</description>
<description>The date-picker component may also be used within Angular Reactive Forms. There is used
<code>[disableFunction]</code>, which controls the <code>valid</code> flag
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe
"[disableFunction] is used to control the valid flag"

Copy link
Contributor

Choose a reason for hiding this comment

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

the [disableFunction] controls the valid flag

@InnaAtanasova
Copy link
Contributor

LGTM (I will not approve now to give a chance to the others to also take a look at it)

@InnaAtanasova
Copy link
Contributor

@mpienkowski could you take a look at the fix for the issue you opened? Thank you!

stefanoScalzo
stefanoScalzo reviewed Jan 9, 2020
View reviewed changes
libs/core/src/lib/date-picker/date-picker.component.ts Outdated
selectedRangeDate.start = this.invalidDate();
}

/** If end date is invalid, cause of format, block or disable function, there is invalidDate forced */
Copy link
Contributor

Choose a reason for hiding this comment

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

do you mean because "because of format, block..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's changed to because

@stefanoScalzo
Copy link
Contributor

Is there any example of showing a disabled range of dates?

@JKMarkowski
Copy link
Contributor Author

Hi @stefanoScalzo, there is example of disabledFunction on range mode.

@salarenko
Copy link
Contributor

salarenko commented Jan 10, 2020 •
edited

Hi @stefanoScalzo, there is example of disabledFunction on range mode.

I think we should create separate example focusing only disabling dates. It took me a while until I found the one using disableFunction even with your hint.
We have two examples of datepicker with reactive forms:

  • Date Picker in Reactive Form
  • Date Picker in Reactive Form with a Range of Dates.

Maybe lets leave one more complex example of reactive forms and transform the other one into "Disabling datepicker and custom dates" section.

Development automation moved this from In progress to Review in progress Jan 10, 2020
salarenko
salarenko suggested changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@salarenko salarenko 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 you also should update your code to fill recent arrangement on private fields and methods naming convention (_ prefix)

libs/core/src/lib/datetime-picker/datetime-picker.component.ts Outdated
@@ -453,7 +417,15 @@ export class DatetimePickerComponent implements OnInit, OnDestroy, ControlValueA
private isModelValid(fdDateTime: FdDatetime): boolean {
return fdDateTime &&
fdDateTime instanceof FdDatetime &&
fdDateTime.isDateValid() && fdDateTime.isTimeValid();
this.isDateValid(fdDateTime) && fdDateTime.isTimeValid();
Copy link
Contributor

Choose a reason for hiding this comment

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

New line

Suggested change
this.isDateValid(fdDateTime) && fdDateTime.isTimeValid();
this.isDateValid(fdDateTime) &&
fdDateTime.isTimeValid();

@JKMarkowski
Copy link
Contributor Author

JKMarkowski commented Jan 10, 2020 •
edited

@salarenko All of the changes are included, I added _ prefix to private fields and methods on date/datetime picker component.
Also there is new example of range disable date picker and for single date picker.

@stefanoScalzo
Copy link
Contributor

Hi @stefanoScalzo, there is example of disabledFunction on range mode.

I think we should create separate example focusing only disabling dates. It took me a while until I found the one using disableFunction even with your hint.
We have two examples of datepicker with reactive forms:

  • Date Picker in Reactive Form
  • Date Picker in Reactive Form with a Range of Dates.

Maybe lets leave one more complex example of reactive forms and transform the other one into "Disabling datepicker and custom dates" section.

I couldn't agree more! We need to clarify to the users which example is for what in a quick glance

Development automation moved this from Review in progress to Reviewer approved Jan 13, 2020
@JKMarkowski JKMarkowski merged commit acb4688 into master Jan 13, 2020
Development automation moved this from Reviewer approved to Done Jan 13, 2020
@JKMarkowski JKMarkowski deleted the fix/1627-blocked-datetimedate branch January 13, 2020 15:57
Lokanathan-k pushed a commit that referenced this pull request Feb 3, 2020
@JKMarkowski @Lokanathan-k
fix: (core) Datepicker and DateTimePicker: Respect disabled/block fun…
b9522a2 
…ction by inputs (#1787)

* (core) Datepicker and DateTimePicker: Respect disabled/block function on input type, in datepicker and datetimepicker

* Fix Typo

* Revert Changes on FdDate model, remove unused code on example

* Fix typos, change comments

* Add new example with disabled, apply PR comments

* Add example for single datepicker and change range datepicker example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@salarenko salarenko salarenko requested changes

@InnaAtanasova InnaAtanasova InnaAtanasova approved these changes

@droshev droshev Awaiting requested review from droshev

@fkolar fkolar Awaiting requested review from fkolar

@stefanoScalzo stefanoScalzo Awaiting requested review from stefanoScalzo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Sprint 28 - Sydney
Development

Successfully merging this pull request may close these issues.

[DatePicker] allows to type in a blocked date
4 participants
@JKMarkowski @InnaAtanasova @stefanoScalzo @salarenko