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

feat(ui5-datetime-picker): replace time part wheelsliders with clocks #8129

Merged
merged 8 commits into from
Feb 1, 2024

Conversation

NHristov-sap
Copy link
Contributor

Some time ago the ui5-time-picker component was redesigned to use clock dials instead of wheel sliders, but ui5-datetime-picker component wasn't redesigned too.

This PR introduces change of time part - now it also uses clock dials instead of wheel sliders.

image

@@ -210,9 +203,8 @@ class DateTimePicker extends DatePicker {
* @public
*/
async openPicker(): Promise<void> {
await super.openPicker();
this._currentTimeSlider = "hours";
this._previewValues.timeSelectionValue = this.value || this.getFormat().format(new Date());
Copy link
Contributor

Choose a reason for hiding this comment

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

also please check _calendarSelectedDates and _calendarTimestamp. Seems that the fallback is always executed and the overrides seems to be redundant.

Copy link
Contributor Author

@NHristov-sap NHristov-sap Feb 1, 2024

Choose a reason for hiding this comment

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

the mentioned getters _calendarSelectedDates and _calendarTimestamp are necessary, you can check it by commenting them and try to use a picker when there is no date selected already.

Copy link
Contributor

Choose a reason for hiding this comment

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

this._previewValues. _calendarSelectedDates is never set so we always use the value which comes from the parent class. Take this as possible cleanup in separate PR

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 is set in the onSelectedDatesChange method

Copy link
Contributor

@nnaydenow nnaydenow left a comment

Choose a reason for hiding this comment

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

UPDATE: Ignore that comment. We've discussed it offline and the popover should be opened on the view where it was closed.

Also I think that I found an issue with the popover. Could you please confirm it?

When the popover is opened for first time the time picker starts from "hours" view and after that it starts from the "seconds" view.

If it's a issue could you please it here or to open separate issue so we can track the progress?

Screen.Recording.2024-01-31.at.15.17.45.mov

@NHristov-sap NHristov-sap merged commit 9041e16 into main Feb 1, 2024
8 checks passed
@NHristov-sap NHristov-sap deleted the BL_dtp_clocks branch February 1, 2024 07:59
@DMihaylova
Copy link

Fixes #7802

tsanislavgatev pushed a commit that referenced this pull request Feb 20, 2024
…#8129)

* feat(ui5-datetime-picker): replace time part wheelsliders with clocks

* feat(ui5-datetime-picker): fix comments

* feat(ui5-datetime-picker): fix more comments

* feat(ui5-datetime-picker): remove redundant getter

* feat(ui5-datetime-picker): adjust mobile view

* feat(ui5-datetime-picker): fix code reordering in openPicker method
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.

4 participants