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

fix(ui5-date-picker): enable date value strict parsing #4428

Merged
merged 3 commits into from
Dec 9, 2021

Conversation

unazko
Copy link
Contributor

@unazko unazko commented Dec 6, 2021

A validity check is now performed while parsing the date value.

Fixes: #4409

@@ -223,7 +223,7 @@ describe("Date Picker Tests", () => {
const valueHelpIcon = await datepicker.getValueHelpIcon();
await valueHelpIcon.click();

let calendarDate_4_Jan_2019 = await datepicker.getPickerDate(1546560000); //Jan 4, 2019
let calendarDate_4_Jan_2019 = await datepicker.getPickerDate(1638576000); //Jan 4, 2019
await calendarDate_4_Jan_2019.click();

const innerInput = await datepicker.getInnerInput();
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand that part, the date is different now, but the input stays the same.
If the date is different, please change the comments and variable names also.

Copy link
Contributor Author

@unazko unazko Dec 6, 2021

Choose a reason for hiding this comment

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

I've disabled the failing timezone test as it works based on a faked Date.prototype.getDate method.
That way a DateFormat instance with strict parsing enabled is unable to parse the value strings from the input field.
I would suggest to address this test issue in another change.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree

Copy link
Member

Choose a reason for hiding this comment

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

@unazko In case you don't plan to immediately fix it with another change. please create an issue for that not to forget it. If you plan to immediately address it, a new issue is not required.

Copy link
Contributor Author

@unazko unazko Dec 7, 2021

Choose a reason for hiding this comment

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

Additional investigation time would be required, in order to adjust the described test properly.
I've created the following issue in this regard: #4443

Copy link
Member

Choose a reason for hiding this comment

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

Thanks

@ilhan007
Copy link
Member

ilhan007 commented Dec 7, 2021

Could you provide perhaps one sentence what the fix does and we can merge it.
Also it should be "enable" not "enabling' - fix(ui5-date-picker): enable strict parsing

and perhaps you can elaborate a bit - strict parsing of what
fix(ui5-date-picker): enable date value strict parsing
fix(ui5-date-picker): enable strict parsing of the date timestamp

Think this way, someone will read the release notes, and by the title of the commit should be able more or less to understand what the change is doing.

@unazko unazko changed the title fix(ui5-date-picker): enabling strict parsing fix(ui5-date-picker): enable strict parsing Dec 7, 2021
@unazko unazko changed the title fix(ui5-date-picker): enable strict parsing fix(ui5-date-picker): enable date value strict parsing Dec 7, 2021
@ilhan007 ilhan007 merged commit ac5ac2d into SAP:master Dec 9, 2021
eshpak pushed a commit to eshpak/ui5-webcomponents that referenced this pull request Jan 9, 2022
A validity check is now performed while parsing the date value.

Fixes: SAP#4409
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix validation for date picker and date range picker components
3 participants