Skip to content

Commit

Permalink
Add more tests, add cozy styles, fix input event
Browse files Browse the repository at this point in the history
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.

Tickmarks bug when sometimes the tickmars are hidden under the parent's
background is fixed, the tickmark DOM element no longer has a z-index
set to -1.
  • Loading branch information
ndeshev committed Nov 23, 2020
1 parent 7d24572 commit 0e4e9dd
Show file tree
Hide file tree
Showing 13 changed files with 261 additions and 27 deletions.
20 changes: 20 additions & 0 deletions packages/main/debug.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[1122/193243.673:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193243.673:ERROR:exception_snapshot_win.cc(99)] thread ID 26820 not found in process
[1122/193243.703:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193243.704:ERROR:exception_snapshot_win.cc(99)] thread ID 24172 not found in process
[1122/193335.204:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193335.215:ERROR:exception_snapshot_win.cc(99)] thread ID 24184 not found in process
[1122/193335.232:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193335.232:ERROR:exception_snapshot_win.cc(99)] thread ID 30428 not found in process
[1122/193748.390:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193748.391:ERROR:exception_snapshot_win.cc(99)] thread ID 15436 not found in process
[1122/193748.414:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1122/193748.414:ERROR:exception_snapshot_win.cc(99)] thread ID 22404 not found in process
[1123/092211.513:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1123/092211.521:ERROR:exception_snapshot_win.cc(99)] thread ID 7236 not found in process
[1123/092211.537:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1123/092211.537:ERROR:exception_snapshot_win.cc(99)] thread ID 9040 not found in process
[1123/092757.530:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1123/092757.532:ERROR:exception_snapshot_win.cc(99)] thread ID 3596 not found in process
[1123/092757.557:ERROR:process_reader_win.cc(123)] NtOpenThread: {Access Denied} A process has requested access to an object, but has not been granted those access rights. (0xc0000022)
[1123/092757.557:ERROR:exception_snapshot_win.cc(99)] thread ID 17488 not found in process
2 changes: 1 addition & 1 deletion packages/main/src/RangeSlider.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,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);
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,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.
Expand Down
8 changes: 4 additions & 4 deletions packages/main/src/SliderBase.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
31 changes: 25 additions & 6 deletions packages/main/src/SliderBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ class SliderBase extends UI5Element {
step: null,
min: null,
max: null,
labelInterval: null,
};
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -333,6 +340,7 @@ class SliderBase extends UI5Element {
window.removeEventListener(this._moveEventType, this._moveHandler);

this._moveEventType = null;
this._isUserInteraction = false;
}

/**
Expand All @@ -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");
}
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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");
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/main/src/themes/SliderBase.css
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -53,7 +54,6 @@
width: 100%;
height: 1rem;
top: var(--_ui5_slider_tickmark_top);
z-index: -1;
}

.ui5-slider-handle {
Expand Down
12 changes: 6 additions & 6 deletions packages/main/src/themes/base/SliderBase-parameters.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
9 changes: 9 additions & 0 deletions packages/main/src/themes/base/sizes-parameters.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions packages/main/src/themes/sap_belize/SliderBase-parameters.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
20 changes: 20 additions & 0 deletions packages/main/test/pages/RangeSlider.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
20 changes: 20 additions & 0 deletions packages/main/test/pages/Slider.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
63 changes: 63 additions & 0 deletions packages/main/test/specs/RangeSlider.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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");
});
});

Expand Down
Loading

0 comments on commit 0e4e9dd

Please sign in to comment.