refactor(ui5-slider): slider handle and scale extraction by MapTo0 · Pull Request #13033 · UI5/webcomponents

MapTo0 · 2026-02-05T14:30:02Z

No description provided.

ui5-webcomponents-bot · 2026-02-05T14:38:03Z

🚀 Deployed on https://pr-13033--ui5-webcomponents-preview.netlify.app

packages/main/src/SliderTemplate.tsx

ndeshev · 2026-02-12T10:40:19Z

packages/main/src/SliderTemplate.tsx

Previously the part name was just "handle". This will break the apps that already uses it.

ndeshev · 2026-02-12T10:45:15Z

packages/main/src/SliderScale.ts

+	}
+
+	get _tickmarksCount() {
+		return (this.max - this.min) / this.step;


I think it will be good to return if step is 0 to avoid division by 0, in fact, setting the step property to 0 breaks the code in more than one place. in get _allTickmarks() the script throws an exception and at least my browser crashes. I think we should at least avoid uncaught exceptions in case of unwanted property values.

I've added a check but still, application setting step 0 is problem in the application, guarding this would cover potential application bugs.

packages/main/src/SliderScale.ts

ndeshev · 2026-02-12T10:51:01Z

packages/main/src/Slider.ts

-			this.storePropertyState("labelInterval");
-		}
+	_handleResize() {
+		// TODO: Remove after refactoring Base and RangeSlider


ndeshev · 2026-02-12T10:53:30Z

packages/main/src/SliderScale.ts

+		} else {
+			// Calculate if labels would overlap based on their estimated width
+			const visibleLabelCount = Math.ceil((tickmarksCount - 1) / this.labelInterval) + 1;
+			const estimatedLabelWidth = 32; // Estimated width in pixels for a label (2rem = ~32px)


Could we somehow use a CSS var instead of hardcoded number, in case it changes depending on a theme or theme density?

Lets leave such logic for the new responsive scale implementation

niyap

The accessibility part is broken - slider's value is not announced as well as how to increase or decrease the value.
Now, when an invalid value is set, it is not reset anymore. The problem is that that invalid value is set as value of aria-valuenow of the handle and end-value of the scale. If that change is desired, could you please check if it is appropriate from accessibility perspective
According to the UX specification, the first and the last label should always be visible. In such case, what should happen, if you have the following situation: on the test page, slider with id="slider-tickmarks-labels" enhance with step="3" and label-interval="2" -> the last visible one is 16 as it is the last available in the predefined range. Should we visualised 20 at the end as the end of the interval or leave it without label as there is next valid one in out of range.
Tooltips and inputs values are not updated correctly if you just click somewhere over a tickmark of the slider. Works as expected if you drag the handle.
If you set min value greater than the max one, the slider is broken at all. Labels and tickmarks are not rendered. Handle cound not be dragged, tooltip is not updated and etc. Do you know what should we do in such case, I am not sure if it a valid one but if not, we can add some documentation/note about that
In the documentation we state that if the step is equal to 0, slider handle movement is disabled. When negative number or value other than a number, the component fallbacks to its default value. -> currently if you set the step to 0, the slider is even not rendered and а console error is logged. Please, also check when the step value has been changed programmatically.
If you have editable tooltip and you change its value to a invalid one -> value state is changed to error, then you update it again with a valid one and press Enter, the handle is moved to the correct point of the slider, the tooltip now shows a valid value, but the value state is not updated, still remains error. Even on focusout, if you hover over the handle, you will notice that it is still invalid
In RTL mode, the end dot is placed at the beginning and has blue color.

niyap · 2026-02-13T21:22:41Z

packages/main/src/Slider.ts

 	 *
 	 */
 	onBeforeRendering() {
-		if (!this.isCurrentStateOutdated()) {


Please, update the documentation as it describes the deleted logic.

niyap · 2026-02-16T07:30:30Z

packages/main/cypress/specs/Slider.cy.tsx

-		cy.get("@slider").should("have.value", 1, "The current value should be 'stepified' by 7");
-	});
-
-	it("If the step property or the labelInterval are changed, the tickmarks and labels must be updated also", () => {


Isn't it good to have such a test?

niyap · 2026-02-16T07:49:47Z

packages/main/cypress/specs/Slider.cy.tsx

-			.and('have.length', 6);
-	});
-
-	it("If the min and max properties are changed, the tickmarks and labels must be updated also.", () => {


This one as well

niyap · 2026-02-16T12:53:06Z

packages/main/src/Slider.ts

 	 * @formProperty
 	 * @public
 	 */
 	@property({ type: Number })


If we have negative value both for min and max, for example min="-20" and max="-10" and we do not define value. Value is 0 by default which is out of the range, if we have tooltip, it also shows 0 (the default value)(even not marked as invalid value of the range initially.

packages/main/src/SliderHandle.ts

niyap · 2026-02-16T13:18:43Z

packages/main/src/SliderHandle.ts

Do we need it and use it somewhere, as I noticed that _handlePosition with the same logic is implemented and used in SliderTemplate

niyap · 2026-02-16T14:59:59Z

packages/main/src/SliderScale.ts

+			const visibleLabelCount = Math.ceil((tickmarksCount - 1) / this.labelInterval) + 1;
+			const estimatedLabelWidth = 32; // Estimated width in pixels for a label (2rem = ~32px)
+			const requiredWidth = visibleLabelCount * estimatedLabelWidth;
+			this._labelsOverlapping = containerSize < requiredWidth;


If you have a slider with labels and tickmarks and you update the step programmatically, the labels are not rendered correctly. The last one is not visualised where it should always be. I tested on "Slider with steps, input tooltips, tickmarks and labels" sample, changing the step value on change

packages/main/src/SliderScale.ts

packages/main/src/SliderTemplate.tsx

niyap · 2026-02-16T22:26:20Z

packages/main/src/Slider.ts

@@ -263,7 +232,7 @@

 	_onTooltipOpen() {


Check the sample with the float values. Initially on hover, the tooltip shows the value with the two decimals places precision but if you click couple times over the handle (without changing the value), the tooltip value is shown just 10.

niyap · 2026-02-17T11:15:17Z

packages/main/src/themes/Slider.css

+}
+
+:host([label-interval]:not([label-interval="0"])) {
+	height: 3.75rem;


Isn't that hight expected when we have tooltips, not labels.

niyap · 2026-02-17T11:17:50Z

packages/main/src/themes/sap_horizon/Slider-parameters.css

@@ -0,0 +1,11 @@
+:host {
+	--_slider_root_side_padding: 0 1rem;
+	--_slider_height: 2.75rem;


--_slider_height that variable is defined only for Horizon Light theme, in all other supported theme it resolves to undefined

niyap · 2026-02-17T11:23:14Z

packages/main/src/themes/Slider.css

+	height: 100%;
+	display: flex;
+	align-items: center;
+	padding: var(--_slider_root_side_padding);


Previously, there was a padding in all supported themes, now you define value only for the Horizon Light and Dark. Is there a reason?

niyap · 2026-02-17T11:31:31Z

packages/main/src/themes/SliderScale.css

@@ -0,0 +1,148 @@
+:host {


Previously, the cursor was pointer, now it is not. Do you know which one is expected

niyap · 2026-02-17T11:37:47Z

packages/main/src/themes/SliderHandle.css

redundant empty line

niyap · 2026-02-17T13:22:22Z

packages/main/src/themes/SliderScale.css

+	width: 100%;
+	position: relative;
+	background: var(--ui5_slider_scale_background);
+	border: var(--_ui5_slider_scale_border);


currently border results in 0.857rem instead of 1px.

niyap · 2026-02-17T13:26:31Z

packages/main/src/themes/sap_fiori_3/SliderScale-parameters.css

I think that is redundant, on the next line you define the same variable but with value none which I think is the expected one.

niyap · 2026-02-17T13:26:56Z

packages/main/src/themes/sap_fiori_3_dark/SliderScale-parameters.css

I think that is redundant, on the next line you define the same variable but with value none which I think is the expected one.

niyap · 2026-02-17T13:27:20Z

packages/main/src/themes/sap_fiori_3_hcb/SliderScale-parameters.css

I think that is redundant, on the next line you define the same variable but with value none which I think is the expected one.

niyap · 2026-02-17T13:27:34Z

packages/main/src/themes/sap_fiori_3_hcw/SliderScale-parameters.css

I think that is redundant, on the next line you define the same variable but with value none which I think is the expected one.

niyap · 2026-02-17T13:50:53Z

packages/main/src/themes/SliderScale.css

+	top: 50%;
+	transform: translateY(-50%);
+	display: var(--_ui5_slider_scale_dots_display);
+}


I did not find information about the offset between the scale and the dot, but I think it is bigger then that shown on the screenshots and a little bigger than it was before the change

niyap · 2026-02-17T19:40:16Z

packages/main/src/themes/sap_horizon_hcb/SliderHandle-parameters.css

+@import "../base/SliderHandle-parameters.css";
+
+:host {
+	--ui5_slider_handle_outline_offset: 3px;


I am not sure is that the correct value here, as I did not find it specified anywhere. In the documentation, it is mentioned that the offset is 1px but on the other hand, if we look at the screenshots, it looks bigger. Do you know the reason to be 3px? The sam is valid for HCW as well

refactor(ui5-slider): slider handle extraction

160781b

MapTo0 changed the title ~~refactor(ui5-slider): slider handle extraction~~ refactor(ui5-slider): slider handle and scale extraction Feb 5, 2026

MapTo0 requested review from ndeshev and niyap February 5, 2026 14:30

ui5-webcomponents-bot temporarily deployed to preview February 5, 2026 14:38 Inactive

Merge branch 'main' into slider-refactoring-2

fe25fa2

ui5-webcomponents-bot temporarily deployed to preview February 9, 2026 08:53 Inactive

ndeshev reviewed Feb 12, 2026

View reviewed changes

niyap reviewed Feb 16, 2026

View reviewed changes

niyap reviewed Feb 17, 2026

View reviewed changes

fix: review feedback applied

be21665

ui5-webcomponents-bot temporarily deployed to preview February 18, 2026 22:19 Inactive

Conversation

MapTo0 commented Feb 5, 2026

Uh oh!

ui5-webcomponents-bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ndeshev Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niyap left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

ui5-webcomponents-bot commented Feb 5, 2026 •

edited

Loading

ndeshev Feb 12, 2026 •

edited

Loading

niyap left a comment •

edited

Loading