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-range-slider): Add Range Slider component #2310

Merged
merged 56 commits into from
Nov 20, 2020

Conversation

ndeshev
Copy link
Contributor

@ndeshev ndeshev commented Oct 7, 2020

Introduce ui5-range-slider component that represents a numerical interval and two handles to select a sub-range within it.
The purpose of the component is to enable visual selection of sub-ranges within a given interval.

<ui5-range-slider start-value="10" end-value="20"></ui5-range-slider>

Overview

ui5-range-slider extends the SliderBase base class. It contains most of the component's API as well as it's CSS and the core part of the template file:

Properties:

  • min - The minimum value of the slider range
  • max - The maximum value of the slider range
  • value - The current value of the slider
  • step - Determines the increments in which the slider will move
  • showTickmarks - Displays a visual divider between the step values
  • showToolTip - Determines if a tooltip should be displayed above the handle
  • labelInterval - Labels some or all of the tickmarks with their values.
  • disabled- Defines whether the Slider is in disabled state.

Events:

  • change - Fired when the value changes and the user has finished interacting with the slider.
  • input - Fired when the value changes due to user interaction that is not yet finished - during mouse/touch dragging.

ui5-range-slider extends the API above with its specific properties:

  • startValue - Defines start point of a selection - position of a first handle on the slider.
  • endValue - Defines end point of a selection - position of a second handle on the slider.

Main features:
The Range Slider allows users to:

  • Change the startValue or endValue by clicking on the RangeSlider progress tracker or moving the handles.
  • Drag the whole selected range to increase or decrease the start and end values.
  • Handles (and values) can move across each other. If the start-handle passes over the end-handle the 'startValue' and 'endValue' properties are swapped, in order the start-handle to always represent the 'startValue' and so for the endValue/handle.
  • define custom "min" and "max" values of the Slider, they can be positive or negative numbers, round or decimal ones.
  • define a custom step - the step is the number with which the Slider value is increased/decreased. It also can be a round number or a decimal one.
  • the startValue and endValue properties are always kept within the boundaries of the min and max they are synchronized between each other, along with their visual UI representation.

@CLAassistant
Copy link

CLAassistant commented Oct 7, 2020

CLA assistant check
All committers have signed the CLA.

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.

Niki, Thanks for the PR.

I tried it briefly and I looked at the code, it works fine to me and conceptually I don't find major things.

The way you apply styles to the DOM element is something that you should refactor. We do it more declaratively so to say, compared to the imperative approach you have. You will find more details in the inlined comment.

Regarding the API, I would say we should discuss the "value" and "endValue' thing within the team, otherwise it seems fine.

@ndeshev
Copy link
Contributor Author

ndeshev commented Oct 8, 2020

Hey Ilhan,

Thank you for your comments and kind feedback.

Just want to say I am working on the range selection concept (with the endValue prop) at the moment and it is not yet functional. I will write an update when I have progress later, in order to have something like an implementation example (..proof of concept) when discussing the property.

Overall the PR is still in an early, draft stage, the CSS is there for now just to visualise the POC samples, the code will be also changed and refactored frequently.

@ndeshev ndeshev changed the title feat(ui5-slider): Slider essentials POC, core functionality (WIP) feat(ui5-slider): Slider & Range Slider essentials, base functionality (WIP, POC) Oct 12, 2020
@ndeshev ndeshev changed the title feat(ui5-slider): Slider & Range Slider essentials, base functionality (WIP, POC) feat(ui5-range-slider): Range Slider essentials, base functionality (WIP, POC) Oct 12, 2020
@ndeshev
Copy link
Contributor Author

ndeshev commented Oct 13, 2020

I separated the Slider and Range Slider functionality and introduced a new common base class - SliderBase.
I created a new draft pull request for the Slider and will continue to update this one as a stand alone RangeSlider component.

@ndeshev ndeshev changed the title feat(ui5-range-slider): Range Slider essentials, base functionality (WIP, POC) feat(ui5-range-slider): Range Slider Oct 26, 2020
@ndeshev ndeshev changed the title feat(ui5-range-slider): Range Slider feat(ui5-range-slider): Add Range Slider component Oct 26, 2020
@ndeshev ndeshev marked this pull request as ready for review October 28, 2020 07:38
@ndeshev
Copy link
Contributor Author

ndeshev commented Nov 13, 2020

Feature Request: #1242

ndshv and others added 2 commits November 14, 2020 11:47
- startValue and endValue are swapped when the start-handle moves pass
  the end-handle

- when click is within the selected range no values are changed

- if the selected range is dragged - both values are updated and the
whole selection is moved

- Implemented property synchronizations and normalizations

- Added RTL support and resize handling for the Range Slider elements

- Added test page, tests, and samples

- Added support by specifications  for Fiori 3, Fiori 3 Dark, Fiori 3
  HCW and HCB variants and Belize variants
@ndeshev
Copy link
Contributor Author

ndeshev commented Nov 16, 2020

Hello, I rebased this PR based on the Slider's branch.

Current progress overview:

  • Range Slider includes the features of the Slider component, plus specific Range features like swapping the values when one handle come across the other, and moving the whole range selection when dragging it.
  • The code review comments changes are done (including the ones from the Slider's pull request that are relevant)
  • Theme styles are added according to the specifications, including HCW and HCB variants.
  • RTL support is implemented
  • Playground samples
  • Test page and unit tests
  • Accessibility and keyboard handling will be added on a later stage.
  • The branch is synced with the updated UI5 Web Components master branch.

@ndeshev
Copy link
Contributor Author

ndeshev commented Nov 18, 2020

Hi colleagues, everything we discussed about the Slider's implementation is now adapted and applied here to the Range Slider and it is ready to be reviewed again.

This PR is based on the Slider's one, the only additional files are RangeSlider.js, the playground sample, the test page and the unit tests for it.

@ndeshev ndeshev force-pushed the range-slider branch 2 times, most recently from 62c49bf to 68bfb3f Compare November 18, 2020 12:14
packages/main/src/RangeSlider.js Outdated Show resolved Hide resolved
packages/main/src/RangeSlider.js Outdated Show resolved Hide resolved
packages/main/src/RangeSlider.js Outdated Show resolved Hide resolved
@ilhan007 ilhan007 closed this Nov 20, 2020
@ilhan007 ilhan007 reopened this Nov 20, 2020
@ilhan007 ilhan007 merged commit 9dea3b3 into SAP:master Nov 20, 2020
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.

5 participants