fix(ui5-slider): add more tests, add cozy styles, fix input event, fix tickmarks display (#2508)

New tests are added to cover using 'effective' properties for
calculation instead of normaliziations of the real ones.
Added cozy styles and set as default while the compact css vars are now
moved to sizes-parameters.css files.
Input event is now fixed and fired only after the current value is
changed by user interaction.
Updating the labels based on the other properties is fixed - labels are
now getting re-created if the one of the other component's properties is updated.
Tick-marks bug when sometimes the tick-mars are hidden under the parent's
background is fixed, the tick mark DOM element no longer has a z-index
set to -1.

Author: ndeshev

packages/main/src/RangeSlider.js

@@ -142,7 +142,7 @@ class RangeSlider extends SliderBase {
 		}
 
 		// Calculate the new value from the press position of the event
-		const newValue = this.handleDownBase(event, this._effectiveMin, this._effectiveMax);
+		const newValue = this.handleDownBase(event);
 
 		// Determine the rest of the needed details from the start of the interaction.
 		this._saveInteractionStartData(event, newValue);

packages/main/src/Slider.js

@@ -118,7 +118,7 @@ class Slider extends SliderBase {
 			return;
 		}
 
-		const newValue = this.handleDownBase(event, this._effectiveMin, this._effectiveMax);
+		const newValue = this.handleDownBase(event);
 		this._valueOnInteractionStart = this.value;
 
 		// Do not yet update the Slider if press is over a handle. It will be updated if the user drags the mouse.

packages/main/src/SliderBase.hbs

@@ -7,10 +7,6 @@
 	dir="{{effectiveDir}}"
 >
 	<div class="ui5-slider-inner">
-		<div class="ui5-slider-progress-container">
-			<div class="ui5-slider-progress" style="{{styles.progress}}"></div>
-		</div>
-
 		{{#if step}}
 			{{#if showTickmarks}}
 				<div class="ui5-slider-tickmarks" style="{{styles.tickmarks}}"></div>
@@ -23,6 +19,10 @@
 				{{/if}}
 			{{/if}}
 		{{/if}}
+
+		<div class="ui5-slider-progress-container">
+			<div class="ui5-slider-progress" style="{{styles.progress}}"></div>
+		</div>
 		{{> handles}}
 	</div>
 </div>

packages/main/src/SliderBase.js

@@ -166,6 +166,7 @@ class SliderBase extends UI5Element {
 			step: null,
 			min: null,
 			max: null,
+			labelInterval: null,
 		};
 	}
 
@@ -301,20 +302,26 @@ class SliderBase extends UI5Element {
 	 *
 	 * @protected
 	 */
-	handleDownBase(event, min, max) {
+	handleDownBase(event) {
+		const min = this._effectiveMin;
+		const max = this._effectiveMax;
+		const domRect = this.getBoundingClientRect();
+		const directionStart = this.directionStart;
+		const step = this._effectiveStep;
+		const newValue = SliderBase.getValueFromInteraction(event, step, min, max, domRect, directionStart);
+
 		if (isPhone() && this.showTooltip) {
 			this._tooltipVisibility = "visible";
 		}
 
+		// Mark start of a user interaction
+		this._isUserInteraction = true;
 		// Only allow one type of move event to be listened to (the first one registered after the down event)
 		this._moveEventType = !this._moveEventType ? SliderBase.MOVE_EVENT_MAP[event.type] : this._moveEventType;
 
 		SliderBase.UP_EVENTS.forEach(upEventType => window.addEventListener(upEventType, this._upHandler));
 		window.addEventListener(this._moveEventType, this._moveHandler);
 
-		this._boundingClientRect = this.getBoundingClientRect();
-		const newValue = SliderBase.getValueFromInteraction(event, this.step, min, max, this._boundingClientRect, this.directionStart);
-
 		return newValue;
 	}
 
@@ -333,6 +340,7 @@ class SliderBase extends UI5Element {
 		window.removeEventListener(this._moveEventType, this._moveHandler);
 
 		this._moveEventType = null;
+		this._isUserInteraction = false;
 	}
 
 	/**
@@ -344,7 +352,9 @@ class SliderBase extends UI5Element {
 	updateValue(valueType, value) {
 		this[valueType] = value;
 		this.storePropertyState(valueType);
-		this.fireEvent("input");
+		if (this._isUserInteraction) {
+			this.fireEvent("input");
+		}
 	}
 
 	/**
@@ -446,7 +456,6 @@ class SliderBase extends UI5Element {
 		// Recalculate the tickmarks and labels and update the stored state.
 		if (this.isPropertyUpdated("min", "max", ...values)) {
 			this.storePropertyState("min", "max");
-			this._createLabels();
 
 			// Here the value props are changed programatically (not by user interaction)
 			// and it won't be "stepified" (rounded to the nearest step). 'Clip' them within
@@ -457,6 +466,16 @@ class SliderBase extends UI5Element {
 				this.storePropertyState(valueType);
 			});
 		}
+
+		// Labels must be updated if any of the min/max/step/labelInterval props are changed
+		if (this.labelInterval && this.showTickmarks) {
+			this._createLabels();
+		}
+
+		// Update the stored state for the labelInterval, if changed
+		if (this.isPropertyUpdated("labelInterval")) {
+			this.storePropertyState("labelInterval");
+		}
 	}
 
 	/**

packages/main/src/themes/SliderBase.css

@@ -37,6 +37,7 @@
 	border: var(--_ui5_slider_progress_border);
 	border-radius: var(--_ui5_slider_progress_border_radius);
 	height: var(--_ui5_slider_inner_height);
+	position: relative;
 }
 
 .ui5-slider-progress {
@@ -53,7 +54,6 @@
 	width: 100%;
 	height: 1rem;
 	top: var(--_ui5_slider_tickmark_top);
-	z-index: -1;
 }
 
 .ui5-slider-handle {

packages/main/src/themes/base/SliderBase-parameters.css

@@ -5,13 +5,13 @@
 	--_ui5_slider_inner_height: 0.25rem;
 	--_ui5_slider_progress_border_radius: 0.25rem;
 	--_ui5_slider_progress_background: var(--sapActiveColor);
-	--_ui5_slider_handle_height: 1.25rem;
-	--_ui5_slider_handle_width: 1.25rem;
+	--_ui5_slider_handle_height: 1.625rem;
+	--_ui5_slider_handle_width: 1.625rem;
 	--_ui5_slider_handle_border: solid 0.125rem var(--sapField_BorderColor);
 	--_ui5_slider_handle_background: var(--sapButton_Background);
 	--_ui5_range_slider_handle_background: rgba(var(--sapButton_Background), 0.25);
-	--_ui5_slider_handle_top: -0.6425rem;
-	--_ui5_slider_handle_margin_left: -0.8125rem;
+	--_ui5_slider_handle_top: -0.825rem;
+	--_ui5_slider_handle_margin_left: -0.9725rem;
 	--_ui5_slider_handle_hover_background: var(--sapButton_Hover_Background);
 	--_ui5_slider_handle_hover_border: var(--sapButton_Hover_BorderColor);
 	--_ui5_range_slider_handle_hover_background: rgba(var(--sapButton_Background), 0.25);
@@ -23,11 +23,11 @@
 	--_ui5_slider_tooltip_background: var(--sapField_Background);
 	--_ui5_slider_tooltip_border_radius: var(--sapElement_BorderCornerRadius);
 	--_ui5_slider_tooltip_border_color: var(--sapField_BorderColor);
-	--_ui5_slider_tooltip_padding: 0.25rem;
+	--_ui5_slider_tooltip_padding: 0.4125rem;
 	--_ui5_slider_tooltip_height: 1rem;
 	--_ui5_slider_tooltip_box_shadow: none;
 	--_ui5_slider_tooltip_min_width: 2rem;
-	--_ui5_slider_tooltip_bottom: 1.825rem;
+	--_ui5_slider_tooltip_bottom: 2rem;
 	--_ui5_slider_label_fontsize: var(--sapFontSmallSize);
 	--_ui5_slider_label_color: var(--sapContent_LabelColor);
 }

packages/main/src/themes/base/sizes-parameters.css

@@ -199,6 +199,15 @@
 	/* Side Navigation */
 	--ui5_side_navigation_item_height: 2rem;
 
+	/* Slider */
+	--_ui5_slider_handle_height: 1.25rem;
+	--_ui5_slider_handle_width: 1.25rem;
+	--_ui5_slider_handle_top: -0.6425rem;
+	--_ui5_slider_handle_margin_left: -0.7825rem;
+	--_ui5_slider_tooltip_height: 1rem;
+	--_ui5_slider_tooltip_padding: 0.25rem;
+	--_ui5_slider_tooltip_bottom: 1.825rem;
+
 	/* Tree */
 	--_ui5-tree-indent-step: 0.5rem;
 	--_ui5-tree-toggle-box-width: 2rem;

packages/main/src/themes/sap_belize/SliderBase-parameters.css

@@ -5,18 +5,18 @@
 	--_ui5_slider_inner_height: 0.1875rem;
 	--_ui5_slider_progress_border-radius: 0;
 	--_ui5_slider_progress_background: var(--sapField_Active_BorderColor);
-	--_ui5_slider_handle_height: 1.375rem;
-	--_ui5_slider_handle_width: 1.375rem;
+	--_ui5_slider_handle_height: 1.75rem;
+	--_ui5_slider_handle_width: 1.75rem;
 	--_ui5_slider_handle_border: solid 0.125rem #999;
 	--_ui5_slider_handle_background: rgba(var(--sapField_Background), 0.1);
-	--_ui5_slider_handle_top: -0.725rem;
+	--_ui5_slider_handle_top: -0.8725rem;
 	--_ui5_slider_handle_margin_left: -1rem;
 	--_ui5_slider_handle_hover_background: rgba(217,217,217,0.6);
 	--_ui5_slider_handle_hover_border: #999;
 	--_ui5_slider_tickmark_color: #bfbfbf;
 	--_ui5_slider_disabled_opacity: 0.5;
-	--_ui5_slider_tooltip_padding: 0.25rem 0.50rem;
-	--_ui5_slider_tooltip_height: 1.325rem;
+	--_ui5_slider_tooltip_padding: 0.50rem;
+	--_ui5_slider_tooltip_height: 1rem;
 	--_ui5_slider_tooltip_box_shadow: 0 0.625rem 2rem 0  rgba(0, 0, 0, 0.15);
 	--_ui5_slider_tooltip_border_radius: 0;
 }

packages/main/test/pages/RangeSlider.html

@@ -59,5 +59,25 @@ <h2>Disabled Range Slider</h2>
 			<h2 style="text-align: center;">Range Slider with steps, tooltips, tickmarks and labels</h2>
 			<ui5-range-slider id="range-slider-tickmarks-labels" min="0" max="44" step="2" start-value="2" end-value="12" show-tooltip label-interval="2" show-tickmarks style="width: 60%; margin-left: 20%; margin-top: 50px;"></ui5-range-slider>
 		</section>
+
+		<section class="group">
+			<h2>Event Testing Slider</h2>
+			<ui5-range-slider id="test-slider" min="0" max="10"  start-value="1" end-value="2"></ui5-range-slider>
+			<h2>Event Testing Result Slider</h2>
+			<ui5-range-slider id="test-result-slider" start-value="1" end-value="2"></ui5-range-slider>
+		</section>
+
+		<script>
+			const eventTargetSlider = document.getElementById("test-slider");
+			const eventResultSlider = document.getElementById("test-result-slider");
+		
+			eventTargetSlider.addEventListener("ui5-input", () => {
+				eventResultSlider.setAttribute("end-value", parseInt(eventResultSlider.getAttribute("end-value")) + 1);
+			});
+
+			eventTargetSlider.addEventListener("ui5-change", () => {
+				eventResultSlider.setAttribute("end-value", parseInt(eventResultSlider.getAttribute("end-value")) + 1);
+			});
+		</script>
 	</body>
 </html>

packages/main/test/pages/Slider.html

@@ -66,5 +66,25 @@ <h2 style="text-align: center;">Slider with steps, tooltips, tickmarks and label
 			<h2>Slider with float number step (1.25), tooltips, tickmarks and labels between 3 steps (3.75 value)</h2>
 			<ui5-slider id="slider-tickmarks-tooltips-labels" min="-12.5" max="47.5" step="1.25" value="10" show-tooltip label-interval="3" show-tickmarks></ui5-slider>
 		</section>
+
+		<section class="group">
+			<h2>Event Testing Slider</h2>
+			<ui5-slider id="test-slider" min="0" max="10"></ui5-slider>
+			<h2>Event Testing Result Slider</h2>
+			<ui5-slider id="test-result-slider" value="1"></ui5-slider>
+		</section>
+
+		<script>
+			const eventTargetSlider = document.getElementById("test-slider");
+			const eventResultSlider = document.getElementById("test-result-slider");
+		
+			eventTargetSlider.addEventListener("ui5-input", () => {
+				eventResultSlider.setAttribute("value", parseInt(eventResultSlider.getAttribute("value")) + 1);
+			});
+
+			eventTargetSlider.addEventListener("ui5-change", () => {
+				eventResultSlider.setAttribute("value", parseInt(eventResultSlider.getAttribute("value")) + 1);
+			});
+		</script>
 	</body>
 </html>

packages/main/test/specs/RangeSlider.spec.js

@@ -170,6 +170,30 @@ describe("Properties synchronization and normalization", () => {
 		assert.strictEqual(rangeSlider.getProperty("step"), 1, "Step value should be its default value");
 	});
 
+	it("If a negative number is set to the step property its positive equivalent should be used as effective value", () => {
+		const rangeSlider = browser.$("#range-slider-tickmarks-labels");
+
+		rangeSlider.setProperty("step", -7);
+
+		assert.strictEqual(rangeSlider.getProperty("step"), -7, "Step value should be a positive number");
+
+		rangeSlider.click();
+
+		assert.strictEqual(rangeSlider.getProperty("endValue"), 21, "The current value should be 'stepified' by 7");
+	});
+
+	it("If min property is set to a greater number than the max property their effective values should be swapped, their real ones - not", () => {
+		const rangeSlider = browser.$("#range-slider-tickmarks-labels");
+
+		rangeSlider.setProperty("startValue", 2);
+		rangeSlider.setProperty("max", 10);
+		rangeSlider.setProperty("min", 100);
+
+		assert.strictEqual(rangeSlider.getProperty("min"), 100, "min property itself should not be normalized");
+		assert.strictEqual(rangeSlider.getProperty("max"), 10, "max property itself should not be normalized");
+		assert.strictEqual(rangeSlider.getProperty("startValue"), 10, "startValue property should be within the boundaries of the effective (swapped) min and max props");
+	});
+
 	it("Should keep the current values between the boundaries of min and max properties", () => {
 		const rangeSlider = browser.$("#range-slider-tickmarks-labels");
 
@@ -196,6 +220,45 @@ describe("Properties synchronization and normalization", () => {
 
 		assert.strictEqual(rangeSlider.getProperty("startValue"), 14, "startValue should not be stepped to the next step (15)");
 		assert.strictEqual(rangeSlider.getProperty("endValue"), 24, "endValue should not be stepped to the next step (25)");
+	});	
+
+	it("If the step property or the labelInterval are changed, the tickmarks and labels must be updated also", () => {
+		const rangeSlider = browser.$("#range-slider-tickmarks-labels");
+
+		rangeSlider.setProperty("max", 0);
+		rangeSlider.setProperty("min", 40);
+		rangeSlider.setProperty("step", 1);
+
+		assert.strictEqual(rangeSlider.getProperty("_labels").length, 21, "Labels must be 21 - 1 for every 2 tickmarks (and steps)");
+
+		rangeSlider.setProperty("step", 2);
+
+		assert.strictEqual(rangeSlider.getProperty("_labels").length, 11, "Labels must be 12 - 1 for every 2 tickmarks (and 4 current value points)");
+		
+		rangeSlider.setProperty("labelInterval", 4);
+
+		assert.strictEqual(rangeSlider.getProperty("_labels").length, 6, "Labels must be 6 - 1 for every 4 tickmarks (and 8 current value points)");
+	});
+});
+
+describe("Testing events", () => {
+
+	it("Should fire input event on use interaction and change event after user interaction finish", () => {
+		const rangeSlider = browser.$("#test-slider");
+		const eventResultRangeSlider = browser.$("#test-result-slider");
+
+		rangeSlider.click();
+
+		assert.strictEqual(eventResultRangeSlider.getProperty("endValue") , 4, "Both input event and change event are fired after user interaction");
+	});
+
+	it("Should not fire change event after user interaction is finished if the current value is the same as the one at the start of the action", () => {
+		const rangeSlider = browser.$("#test-slider");
+		const eventResultRangeSlider = browser.$("#test-result-slider");
+
+		rangeSlider.click();
+
+		assert.strictEqual(eventResultRangeSlider.getProperty("endValue") , 4, "Change event is not fired if the value is the same as before the start of the action");
 	});
 });
 

packages/main/test/specs/Slider.spec.js

@@ -63,6 +63,71 @@ describe("Slider basic interactions", () => {
 	});
 });
 
+describe("Properties synchronization and normalization", () => {
+
+	it("If a negative number is set to the step property its positive equivalent should be used as effective value", () => {
+		const slider = browser.$("#slider-tickmarks-labels");
+
+		slider.setProperty("step", -7);
+
+		assert.strictEqual(slider.getProperty("step"), -7, "The step property should not be changed itself");
+
+		slider.click();
+
+		assert.strictEqual(slider.getProperty("value"), 1, "The current value should be 'stepified' by 7");
+	});
+
+	it("If step property is not a valid number its value should fallback to 1", () => {
+		const slider = browser.$("#slider-tickmarks-labels");
+
+		slider.setProperty("step", 2);
+		slider.setProperty("step", "String value");
+		slider.click();
+
+		assert.strictEqual(slider.getProperty("step"), 1, "Step property should be set to its defaut value");
+		assert.strictEqual(slider.getProperty("value"), 0, "The current value should be 'stepified' by 1");
+	});
+
+	it("If the step property or the labelInterval are changed, the tickmarks and labels must be updated also", () => {
+		const slider = browser.$("#slider-tickmarks-labels");
+
+		assert.strictEqual(slider.getProperty("_labels").length, 21, "Labels must be 21 - 1 for every 2 tickmarks (and steps)");
+
+		slider.setProperty("step", 2);
+
+		assert.strictEqual(slider.getProperty("_labels").length, 11, "Labels must be 12 - 1 for every 2 tickmarks (and 4 current value points)");
+		
+		slider.setProperty("labelInterval", 4);
+
+		assert.strictEqual(slider.getProperty("_labels").length, 6, "Labels must be 6 - 1 for every 4 tickmarks (and 8 current value points)");
+	});
+
+	it("If min property is set to a greater number than the max property their effective values should be swapped, their real ones - not", () => {
+		const slider = browser.$("#basic-slider");
+
+		slider.setProperty("value", 2);
+		slider.setProperty("max", 10);
+		slider.setProperty("min", 100);
+
+		assert.strictEqual(slider.getProperty("max"), 10, "min property itself should not be normalized");
+		assert.strictEqual(slider.getProperty("min"), 100, "max property itself should not be normalized");
+		assert.strictEqual(slider.getProperty("value"), 10, "value property should be within the boundaries of the normalized 'effective' min and max values");
+	});
+
+	it("Should keep the current value between the boundaries of min and max properties", () => {
+		const slider = browser.$("#basic-slider");
+
+		slider.setProperty("min", 100);
+		slider.setProperty("max", 200);
+		slider.setProperty("value", 300);
+
+		assert.strictEqual(slider.getProperty("value"), 200, "value prop should always be lower than the max value");
+
+		slider.setProperty("value", 99);
+
+		assert.strictEqual(slider.getProperty("value"), 100, "value prop should always be greater than the min value");
+	});
+});
 
 describe("Slider elements - tooltip, step, tickmarks, labels", () => {
 
@@ -89,9 +154,7 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => {
 
 		assert.strictEqual(numberOfLabels, 17, "17 labels should be rendered, 1 between each 3 tickmarks");
 	});
-});
 
-describe("Properties synchronization and normalization", () => {
 	it("Should not 'stepify' current value if it is not in result of user interaction", () => {
 		const slider = browser.$("#tickmarks-slider");
 
@@ -101,6 +164,27 @@ describe("Properties synchronization and normalization", () => {
 	});
 });
 
+describe("Testing events", () => {
+
+	it("Should fire input event on use interaction and change event after user interaction finish", () => {
+		const slider = browser.$("#test-slider");
+		const eventResultSlider = browser.$("#test-result-slider");
+
+		slider.click();
+
+		assert.strictEqual(eventResultSlider.getProperty("value") , 3, "Both input event and change event are fired after user interaction");
+	});
+
+	it("Should not fire change event after user interaction is finished if the current value is the same as the one at the start of the action", () => {
+		const slider = browser.$("#test-slider");
+		const eventResultSlider = browser.$("#test-result-slider");
+
+		slider.click();
+
+		assert.strictEqual(eventResultSlider.getProperty("value") , 3, "Change event is not fired if the value is the same as before the start of the action");
+	});
+});
+
 describe("Testing resize handling and RTL support", () => {
 	it("Testing RTL support", () => {
 		const slider = browser.$("#basic-slider");
@@ -118,7 +202,6 @@ describe("Testing resize handling and RTL support", () => {
 
 		assert.strictEqual(sliderHandle.getAttribute("style"), "right: 30%;", "Slider handle should be 30% from the right");
 
-		slider.moveTo();
 		slider.click();
 
 		assert.strictEqual(sliderHandle.getAttribute("style"), "right: 50%;", "Slider handle should be in the middle of the slider");