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-time-picker): Redesign the Time Picker component according to the new design #6818

Merged
merged 14 commits into from
Jun 9, 2023

Conversation

NHristov-sap
Copy link
Contributor

@NHristov-sap NHristov-sap commented Mar 29, 2023

The Time Picker component will be redesigned according to the new design with clocks, already provided for UI5 TimePicker control.

Instead of currently existing sliders, new clock dial components whould be used to select time (hours/minutes/seconds, each selected by separate clock). Also, there is AM/PM segmented button that replaces the AM/PM slider.

image

image

In addition to mouse, touch or mouse wheel time selection, there is improved keyboard interaction added - hours/minutes and seconds can be selected by keyboard ([PageUp]/[PageDown] for hours, [Shift]+[PageUp]/[PageDown] for minutes and [Ctrl/Cmd]+[Shift]+[PageUp]/[PageDown] for seconds as well as [A] or [P] for AM/PM selection). Also, direct number typing is accepted.

Accessibility/SR announcements are also covered.

packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
Copy link
Member

@ilhan007 ilhan007 left a comment

Choose a reason for hiding this comment

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

The PR says:
"refactor(ui5-time-picker): Redesign the Time Picker component according to the new design"

but there is no redesign of the TimePicker, it's a PR that adds private web component (that I assume will be used later with another changed in the TimePicker). The PR description needs adjustments.

@NHristov-sap
Copy link
Contributor Author

The PR says: "refactor(ui5-time-picker): Redesign the Time Picker component according to the new design"

but there is no redesign of the TimePicker, it's a PR that adds private web component (that I assume will be used later with another changed in the TimePicker). The PR description needs adjustments.

I will change "refactor" with "wip" - in fact the component that is currently in this PR cannot (and have no idea to) be used separately. There will be at least two more components (also private and useless outside Time Picker context) included in this PR and all of them then will be used in the refactoring.

BR,
Niki

@NHristov-sap NHristov-sap changed the title refactor(ui5-time-picker): Redesign the Time Picker component according to the new design wip(ui5-time-picker): Redesign the Time Picker component according to the new design Mar 30, 2023
@github-actions github-actions bot added the Stale label Apr 21, 2023
@github-actions github-actions bot closed this Apr 28, 2023
@NHristov-sap
Copy link
Contributor Author

Hello, the redesign is still in process, that's why I am reopening this PR.
BR,
Nikolay Hristov

@NHristov-sap NHristov-sap reopened this Apr 28, 2023
@github-actions github-actions bot removed the Stale label Apr 29, 2023
@NHristov-sap NHristov-sap force-pushed the BL_timepicker_redesign branch from 736511c to 7a5e86a Compare May 4, 2023 06:52
@NHristov-sap NHristov-sap requested a review from unazko May 4, 2023 07:03
packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
packages/main/src/TimePickerClock.ts Show resolved Hide resolved
packages/main/src/TimePickerClock.ts Outdated Show resolved Hide resolved
packages/main/src/TimeSelectionClocks.ts Outdated Show resolved Hide resolved
packages/main/src/TimeSelectionClocks.ts Show resolved Hide resolved
packages/main/src/TimeSelectionClocks.ts Show resolved Hide resolved
packages/main/src/TimeSelectionClocks.ts Outdated Show resolved Hide resolved
Copy link
Member

@ilhan007 ilhan007 left a comment

Choose a reason for hiding this comment

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

The change is major, please describe it better in the PR's description

@NHristov-sap NHristov-sap requested review from unazko and ilhan007 May 19, 2023 12:38
Copy link
Contributor

@unazko unazko left a comment

Choose a reason for hiding this comment

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

Tested on different themes and time formats. All seems to be working fine.

Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

When the clock is open and I change the value of the timepicker in the console
$0.value = "15:00:00" f.e., nothing happens. I must close the clock and reopen it for it to update.
I get warnings in the console for texts that don't exist.

packages/main/src/TimePickerClock.hbs Outdated Show resolved Hide resolved
packages/main/src/TimePickerClock.hbs Outdated Show resolved Hide resolved
@NHristov-sap NHristov-sap requested a review from vladitasev May 29, 2023 09:53
@NHristov-sap NHristov-sap dismissed stale reviews from ilhan007 and vladitasev May 29, 2023 10:33

It is already fixed

@NHristov-sap NHristov-sap removed the request for review from tsanislavgatev May 29, 2023 10:35
@NHristov-sap NHristov-sap requested a review from ilhan007 June 2, 2023 11:28
@NHristov-sap NHristov-sap requested a review from ilhan007 June 7, 2023 15:24
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

Some discussions are still in progress with the design team, let's delay the merge of the PR for a bit.

@ilhan007 ilhan007 changed the title wip(ui5-time-picker): Redesign the Time Picker component according to the new design feat(ui5-time-picker): Redesign the Time Picker component according to the new design Jun 9, 2023
@NHristov-sap NHristov-sap merged commit 1d910cf into main Jun 9, 2023
@NHristov-sap NHristov-sap deleted the BL_timepicker_redesign branch June 9, 2023 12:10
nnaydenow pushed a commit that referenced this pull request Jun 12, 2023
…o the new design (#6818)

* WIP(ui5-time-picker-clock): initial implementation

* feat(ui5-time-picker-clock): initial implementation

* feat(ui5-time-picker-clock) mark component as private

* time selection with clocks - updated main

* optimization and time selection methods integration, part 1

* wip(ui5-time-picker): replace sliders with clocks

* feat(ui5-time-picker): fix lint errors

* add initial tests

* feat(ui5-time-picker): fix comments and add tests

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

* feat(ui5-time-picker): fix comments and tests

* feat(ui5-time-picker): fix latest comments

* feat(ui5-time-picker): fix comments for tests and global event
@petyabegovska
Copy link
Collaborator

petyabegovska commented Jun 30, 2023

Related to: #3678

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.

6 participants