From d071fe9c6ebc60ea05b62cfe2b9b5d9e3a583f04 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 23 Jul 2024 16:21:35 +0300 Subject: [PATCH 01/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip wip --- packages/main/src/RangeSlider.hbs | 24 +++++++++++++++++++++--- packages/main/src/RangeSlider.ts | 15 +++++++++++++++ packages/main/src/Slider.hbs | 18 +++++++++++++----- packages/main/src/Slider.ts | 23 ++++++++++++++++++++--- packages/main/src/SliderBase.hbs | 2 +- packages/main/src/SliderBase.ts | 20 ++++++++++++++++++-- packages/main/src/themes/SliderBase.css | 22 ++++++++++++++++++++-- packages/main/test/pages/Slider.html | 2 +- 8 files changed, 109 insertions(+), 17 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 9e16c1637fe2..440a1f8a4ce3 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -45,9 +45,18 @@ > - {{#if showTooltip}} + {{#if showTooltip}}
- {{tooltipStartValue}} + {{#if inputTooltip}} + + {{else}} + {{tooltipStartValue}} + {{/if}}
{{/if}} @@ -70,7 +79,16 @@ {{#if showTooltip}}
- {{tooltipEndValue}} + {{#if inputTooltip}} + + {{else}} + {{tooltipEndValue}} + {{/if}}
{{/if}} diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 19acb0b8df25..9335ae48075b 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -540,6 +540,21 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._endValueAtBeginningOfAction = undefined; } + _onInputChange(e: Event) { + const ctor = this.constructor as typeof RangeSlider; + + const input = e.target as HTMLInputElement; + const value = ctor.clipValue(parseFloat(input.value), this._effectiveMin, this._effectiveMax); + + if (input.hasAttribute("start-value")) { + this.update("startValue", value, undefined); + } + + if (input.hasAttribute("end-value")) { + this.update("endValue", value, undefined); + } + } + /** * Determines where the press occured and which values of the Range Slider * handles should be updated on further interaction. diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 999cc08cffd5..95edca243424 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -33,10 +33,18 @@ part="handle" > - {{#if showTooltip}} -
- {{tooltipValue}} -
- {{/if}} + {{#if showTooltip}} +
+ {{#if inputTooltip}} + + {{else}} + {{tooltipValue}} + {{/if}} +
+ {{/if}} {{/inline}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index cf4a23d10fa3..f42bbbfda3ac 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -6,6 +6,7 @@ import { isEscape } from "@ui5/webcomponents-base/dist/Keys.js"; import type { IFormInputElement } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js"; import SliderBase from "./SliderBase.js"; import Icon from "./Icon.js"; +import Input from "./Input.js"; // Template import SliderTemplate from "./generated/templates/SliderTemplate.lit.js"; @@ -74,7 +75,7 @@ import { languageAware: true, formAssociated: true, template: SliderTemplate, - dependencies: [Icon], + dependencies: [Icon, Input], }) class Slider extends SliderBase implements IFormInputElement { /** @@ -193,9 +194,11 @@ class Slider extends SliderBase implements IFormInputElement { if (this.showTooltip) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.VISIBLE; } + + this.focused = true; } - _onfocusout() { + _onfocusout(e: FocusEvent) { // Prevent focusout when the focus is getting set within the slider internal // element (on the handle), before the Slider' customElement itself is finished focusing if (this._isFocusing()) { @@ -207,9 +210,11 @@ class Slider extends SliderBase implements IFormInputElement { // value that was saved when it was first focused in this._valueInitial = undefined; - if (this.showTooltip) { + if (this.showTooltip && !(e.relatedTarget as HTMLInputElement)?.hasAttribute("ui5-input")) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; } + + this.focused = false; } /** @@ -245,6 +250,14 @@ class Slider extends SliderBase implements IFormInputElement { this._valueOnInteractionStart = undefined; } + _onInputChange(e: Event) { + const ctor = this.constructor as typeof Slider; + const input = e.target as HTMLInputElement; + const value = parseFloat(input.value); + + this.value = ctor.clipValue(value, this._effectiveMin, this._effectiveMax); + } + /** Determines if the press is over the handle * @private */ @@ -280,6 +293,10 @@ class Slider extends SliderBase implements IFormInputElement { } } + get inputValue() { + return this.value.toString(); + } + get styles() { return { progress: { diff --git a/packages/main/src/SliderBase.hbs b/packages/main/src/SliderBase.hbs index f4795d150338..8f254185ebf5 100644 --- a/packages/main/src/SliderBase.hbs +++ b/packages/main/src/SliderBase.hbs @@ -44,4 +44,4 @@ {{#*inline "handlesAriaText"}}{{/inline}} {{#*inline "progressBar"}}{{/inline}} -{{#*inline "handles"}}{{/inline}} +{{#*inline "handles"}}{{/inline}} \ No newline at end of file diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index c401cc6c2ae1..5c3442dacd61 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -7,6 +7,7 @@ import { isPhone, supportsTouch } from "@ui5/webcomponents-base/dist/Device.js"; import type { ResizeObserverCallback } from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js"; import type { PassiveEventListenerObject } from "@ui5/webcomponents-base/dist/types.js"; import "@ui5/webcomponents-icons/dist/direction-arrows.js"; +import Input from "./Input.js"; import { isEscape, isHome, isEnd, isUp, isDown, isRight, isLeft, isUpCtrl, isDownCtrl, isRightCtrl, isLeftCtrl, isPlus, isMinus, isPageUp, isPageDown, } from "@ui5/webcomponents-base/dist/Keys.js"; @@ -108,6 +109,14 @@ abstract class SliderBase extends UI5Element { @property({ type: Boolean }) showTooltip = false; + /** + * Enables handle tooltip displaying the current value. + * @default false + * @public + */ + @property({ type: Boolean }) + inputTooltip = false; + /** * Defines whether the slider is in disabled state. * @default false @@ -125,6 +134,12 @@ abstract class SliderBase extends UI5Element { @property() accessibleName?: string; + /** + * @private + */ + @property({ type: Boolean }) + focused = false; + /** * @private */ @@ -403,9 +418,10 @@ abstract class SliderBase extends UI5Element { * @private */ _handleFocusOnMouseDown(e: TouchEvent | MouseEvent) { - const focusedElement = this.shadowRoot!.activeElement; + const currentlyFocusedElement = this.shadowRoot!.activeElement; + const elementToBeFocused = e.target as HTMLElement; - if (!focusedElement || focusedElement !== e.target) { + if ((!currentlyFocusedElement || currentlyFocusedElement !== elementToBeFocused) && !elementToBeFocused.hasAttribute("ui5-input")) { this._preserveFocus(true); this.focusInnerElement(); } diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index c3bcb2053b9e..33d5915ab7ec 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -153,8 +153,6 @@ justify-content: center; align-items: center; visibility: hidden; - pointer-events: none; - line-height: 1rem; position: absolute; left: 50%; transform: translate(-50%); @@ -171,6 +169,21 @@ box-sizing: var(--_ui5_slider_tooltip_border_box); } +:host([input-tooltip]) .ui5-slider-tooltip { + padding: 0; +} + +.ui5-slider-tooltip ui5-input { + background: transparent; + border: none; + box-shadow: none; + height: 100%; + width: 100%; + margin: 0; + padding: 0; + text-align: center; +} + .ui5-slider-tooltip-value { position: relative; display: flex; @@ -237,3 +250,8 @@ border-radius: var(--_ui5_slider_handle_border_radius); pointer-events: none; } + +/* .ui5-slider-popover-input { + min-width: 20px; + min-height: 2px; +} */ diff --git a/packages/main/test/pages/Slider.html b/packages/main/test/pages/Slider.html index 28f27c13d6a6..2f282a648954 100644 --- a/packages/main/test/pages/Slider.html +++ b/packages/main/test/pages/Slider.html @@ -45,7 +45,7 @@

Inactive slider with no steps and tooltip

Slider with steps, tooltips, tickmarks and labels

- +

Slider with float number step (1.25), tooltips, tickmarks and labels between 3 steps (3.75 value)

From 76b9df28d6df246aa46ef6ea0cb9049027b41f0f Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 24 Jul 2024 12:56:06 +0300 Subject: [PATCH 02/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix change bug --- packages/main/src/RangeSlider.ts | 10 +++++----- packages/main/test/pages/RangeSlider.html | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 9335ae48075b..f6e645545a27 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -11,6 +11,7 @@ import { import SliderBase from "./SliderBase.js"; import Icon from "./Icon.js"; import RangeSliderTemplate from "./generated/templates/RangeSliderTemplate.lit.js"; +import Input from "./Input.js"; // Texts import { @@ -88,7 +89,7 @@ type AffectedValue = "startValue" | "endValue"; languageAware: true, formAssociated: true, template: RangeSliderTemplate, - dependencies: [Icon], + dependencies: [Icon, Input], styles: [SliderBase.styles, rangeSliderStyles], }) class RangeSlider extends SliderBase implements IFormInputElement { @@ -547,12 +548,11 @@ class RangeSlider extends SliderBase implements IFormInputElement { const value = ctor.clipValue(parseFloat(input.value), this._effectiveMin, this._effectiveMax); if (input.hasAttribute("start-value")) { - this.update("startValue", value, undefined); + this.startValue = value; + return; } - if (input.hasAttribute("end-value")) { - this.update("endValue", value, undefined); - } + this.endValue = value; } /** diff --git a/packages/main/test/pages/RangeSlider.html b/packages/main/test/pages/RangeSlider.html index 57f1b929379b..d7f79361901d 100644 --- a/packages/main/test/pages/RangeSlider.html +++ b/packages/main/test/pages/RangeSlider.html @@ -41,7 +41,7 @@

Disabled Range Slider

Range Slider with steps, tooltips, tickmarks and labels

- +
From fc4c1452817a43ffa1d18cb30852fd77ad2b1464 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 25 Jul 2024 13:30:59 +0300 Subject: [PATCH 03/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix mouse and shortcuts movement bugs --- packages/main/src/RangeSlider.ts | 10 +++++++--- packages/main/src/Slider.ts | 12 ++++++++++-- packages/main/src/SliderBase.ts | 6 +++--- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index f6e645545a27..4dbf96824ccd 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -429,7 +429,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { _onmousedown(e: TouchEvent | MouseEvent) { // If step is 0 no interaction is available because there is no constant // (equal for all user environments) quantitative representation of the value - if (this.disabled || this._effectiveStep === 0) { + if (this.disabled || this._effectiveStep === 0 || (e.target as HTMLElement).hasAttribute("ui5-input")) { return; } @@ -484,7 +484,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { e.preventDefault(); // If 'step' is 0 no interaction is available as there is no constant quantitative representation of the value - if (this.disabled || this._effectiveStep === 0) { + if (this.disabled || this._effectiveStep === 0 || (e.target as HTMLElement).hasAttribute("ui5-input")) { return; } @@ -525,7 +525,11 @@ class RangeSlider extends SliderBase implements IFormInputElement { this.update(undefined, newValues[0], newValues[1]); } - _handleUp() { + _handleUp(e: MouseEvent) { + if ((e.target as HTMLElement).hasAttribute("ui5-input")) { + return; + } + this._setAffectedValueByFocusedElement(); this._setAffectedValue(undefined); diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index f42bbbfda3ac..dc196904e6ff 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -162,7 +162,7 @@ class Slider extends SliderBase implements IFormInputElement { _onmousedown(e: TouchEvent | MouseEvent) { // If step is 0 no interaction is available because there is no constant // (equal for all user environments) quantitative representation of the value - if (this.disabled || this.step === 0) { + if (this.disabled || this.step === 0 || (e.target as HTMLElement).hasAttribute("ui5-input")) { return; } @@ -222,6 +222,10 @@ class Slider extends SliderBase implements IFormInputElement { * @private */ _handleMove(e: TouchEvent | MouseEvent) { + if ((e.target as HTMLElement).hasAttribute("ui5-input")) { + return; + } + e.preventDefault(); // If step is 0 no interaction is available because there is no constant @@ -241,7 +245,11 @@ class Slider extends SliderBase implements IFormInputElement { /** Called when the user finish interacting with the slider * @private */ - _handleUp() { + _handleUp(e: TouchEvent | MouseEvent) { + if ((e.target as HTMLElement).hasAttribute("ui5-input")) { + return; + } + if (this._valueOnInteractionStart !== this.value) { this.fireEvent("change"); } diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 5c3442dacd61..e038d8e70bda 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -154,7 +154,7 @@ abstract class SliderBase extends UI5Element { _resizeHandler: ResizeObserverCallback; _moveHandler: (e: TouchEvent | MouseEvent) => void; - _upHandler: () => void; + _upHandler: (e: TouchEvent | MouseEvent) => void; _stateStorage: StateStorage; _ontouchstart: PassiveEventListenerObject; notResized = false; @@ -195,7 +195,7 @@ abstract class SliderBase extends UI5Element { _handleMove(e: TouchEvent | MouseEvent) {} // eslint-disable-line - _handleUp() {} + _handleUp(e: TouchEvent | MouseEvent) {} // eslint-disable-line _onmousedown(e: TouchEvent | MouseEvent) {} // eslint-disable-line @@ -296,7 +296,7 @@ abstract class SliderBase extends UI5Element { } _onkeydown(e: KeyboardEvent) { - if (this.disabled || this._effectiveStep === 0) { + if (this.disabled || this._effectiveStep === 0 || (e.target as HTMLElement).hasAttribute("ui5-input")) { return; } From f54169f5594fa8fc343f8fc7e8ea570dd709884f Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 25 Jul 2024 13:35:24 +0300 Subject: [PATCH 04/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip remove the 'focused' property --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/SliderBase.ts | 7 ------- packages/main/src/themes/SliderBase.css | 7 +------ 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 440a1f8a4ce3..7a446eb6d9e6 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -45,7 +45,7 @@ > - {{#if showTooltip}} + {{#if showTooltip}}
{{#if inputTooltip}} Date: Thu, 25 Jul 2024 15:00:02 +0300 Subject: [PATCH 05/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip and F2 shortcut support and acc attributes to the ui5-slider --- packages/main/src/Slider.hbs | 4 ++++ packages/main/src/Slider.ts | 4 ---- packages/main/src/SliderBase.hbs | 3 +++ packages/main/src/SliderBase.ts | 26 ++++++++++++++++++++++++-- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 95edca243424..ac91143462d3 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -29,6 +29,9 @@ aria-valuenow="{{value}}" aria-labelledby="{{_ariaLabelledByHandleRefs}}" aria-disabled="{{_ariaDisabled}}" + aria-keyshortcuts="F2" + aria-describedby="{{_ariaDescribedByHandleRefs}}" + data-sap-focus-ref part="handle" > @@ -40,6 +43,7 @@ type="Number" value="{{value}}" @change="{{_onInputChange}}" + aria-labelledby={{_ariaLabelledByInputRefs}} > {{else}} {{tooltipValue}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index dc196904e6ff..3e27a9b47ea5 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -194,8 +194,6 @@ class Slider extends SliderBase implements IFormInputElement { if (this.showTooltip) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.VISIBLE; } - - this.focused = true; } _onfocusout(e: FocusEvent) { @@ -213,8 +211,6 @@ class Slider extends SliderBase implements IFormInputElement { if (this.showTooltip && !(e.relatedTarget as HTMLInputElement)?.hasAttribute("ui5-input")) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; } - - this.focused = false; } /** diff --git a/packages/main/src/SliderBase.hbs b/packages/main/src/SliderBase.hbs index 8f254185ebf5..6648a00d9dcc 100644 --- a/packages/main/src/SliderBase.hbs +++ b/packages/main/src/SliderBase.hbs @@ -39,6 +39,9 @@ {{accessibleName}} {{_ariaLabelledByText}} + {{_ariaDescribedByInputText}} + {{_ariaLabelledByInputText}} +
diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 9170df8485ac..688864bdaed2 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -8,7 +8,7 @@ import type { ResizeObserverCallback } from "@ui5/webcomponents-base/dist/delega import type { PassiveEventListenerObject } from "@ui5/webcomponents-base/dist/types.js"; import "@ui5/webcomponents-icons/dist/direction-arrows.js"; import { - isEscape, isHome, isEnd, isUp, isDown, isRight, isLeft, isUpCtrl, isDownCtrl, isRightCtrl, isLeftCtrl, isPlus, isMinus, isPageUp, isPageDown, + isEscape, isHome, isEnd, isUp, isDown, isRight, isLeft, isUpCtrl, isDownCtrl, isRightCtrl, isLeftCtrl, isPlus, isMinus, isPageUp, isPageDown, isF2, } from "@ui5/webcomponents-base/dist/Keys.js"; // Styles @@ -289,7 +289,13 @@ abstract class SliderBase extends UI5Element { } _onkeydown(e: KeyboardEvent) { - if (this.disabled || this._effectiveStep === 0 || (e.target as HTMLElement).hasAttribute("ui5-input")) { + const target = (e.target as HTMLElement); + + if (isF2(e) && target.querySelector("ui5-input")) { + (target.querySelector("ui5-input") as HTMLElement).focus(); + } + + if (this.disabled || this._effectiveStep === 0 || target.hasAttribute("ui5-input")) { return; } @@ -737,6 +743,22 @@ abstract class SliderBase extends UI5Element { get _ariaLabelledByHandleRefs() { return [`${this._id}-accName`, `${this._id}-sliderDesc`].join(" ").trim(); } + + get _ariaDescribedByHandleRefs() { + return [`${this._id}-accName`, `${this._id}-sliderInputDesc`].join(" ").trim(); + } + + get _ariaLabelledByInputRefs() { + return [`${this._id}-accName`, `${this._id}-sliderInputLabel`].join(" ").trim(); + } + + get _ariaDescribedByInputText() { + return "Press F2 to enter a value"; + } + + get _ariaLabelledByInputText() { + return "Current value"; + } } export default SliderBase; From f1dc9c7d9ffd57c44eb6146153205ce1d08ff66d Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 26 Jul 2024 12:11:28 +0300 Subject: [PATCH 06/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip improve acc --- packages/main/src/Slider.hbs | 2 +- packages/main/src/Slider.ts | 10 ++++++++++ packages/main/src/SliderBase.ts | 8 -------- packages/main/src/i18n/messagebundle.properties | 6 ++++++ 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index ac91143462d3..fcd4ac05b9ad 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -42,8 +42,8 @@ {{else}} {{tooltipValue}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index 3e27a9b47ea5..f49497592dd8 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -14,6 +14,8 @@ import SliderTemplate from "./generated/templates/SliderTemplate.lit.js"; // Texts import { SLIDER_ARIA_DESCRIPTION, + SLIDER_TOOLTIP_INPUT_DESCRIPTION, + SLIDER_TOOLTIP_INPUT_LABEL, } from "./generated/i18n/i18n-defaults.js"; /** @@ -341,6 +343,14 @@ class Slider extends SliderBase implements IFormInputElement { return Slider.i18nBundle.getText(SLIDER_ARIA_DESCRIPTION); } + get _ariaDescribedByInputText() { + return Slider.i18nBundle.getText(SLIDER_TOOLTIP_INPUT_DESCRIPTION); + } + + get _ariaLabelledByInputText() { + return Slider.i18nBundle.getText(SLIDER_TOOLTIP_INPUT_LABEL); + } + static async onDefine() { Slider.i18nBundle = await getI18nBundle("@ui5/webcomponents"); } diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 688864bdaed2..e4111f0ec201 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -751,14 +751,6 @@ abstract class SliderBase extends UI5Element { get _ariaLabelledByInputRefs() { return [`${this._id}-accName`, `${this._id}-sliderInputLabel`].join(" ").trim(); } - - get _ariaDescribedByInputText() { - return "Press F2 to enter a value"; - } - - get _ariaLabelledByInputText() { - return "Current value"; - } } export default SliderBase; diff --git a/packages/main/src/i18n/messagebundle.properties b/packages/main/src/i18n/messagebundle.properties index 5686b61ed2ff..0cb6c66a23ca 100644 --- a/packages/main/src/i18n/messagebundle.properties +++ b/packages/main/src/i18n/messagebundle.properties @@ -490,6 +490,12 @@ MONTH_PICKER_DESCRIPTION = Month Picker #XACT: ARIA description for year picker YEAR_PICKER_DESCRIPTION = Year Picker +#XACT: ARIA description for slider tooltip input +SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value + +#XACT: ARIA label for slider tooltip input +SLIDER_TOOLTIP_INPUT_LABEL = Current value + #XTOL: tooltip for decrease button of the StepInput STEPINPUT_DEC_ICON_TITLE=Decrease From b32dfddd5c7965d3d9aa97f484096a824e2d5961 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 31 Jul 2024 09:45:40 +0300 Subject: [PATCH 07/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix accessibility issues --- packages/main/src/RangeSlider.hbs | 2 + packages/main/src/RangeSlider.ts | 10 +++ packages/main/src/Slider.hbs | 63 ++++++++++--------- packages/main/src/SliderBase.ts | 16 +++-- .../main/src/i18n/messagebundle.properties | 6 ++ packages/main/src/themes/SliderBase.css | 12 ++-- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 7a446eb6d9e6..185c89f97adb 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -51,6 +51,7 @@ @@ -83,6 +84,7 @@ diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 4dbf96824ccd..1cc5143309d2 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -18,6 +18,8 @@ import { RANGE_SLIDER_ARIA_DESCRIPTION, RANGE_SLIDER_START_HANDLE_DESCRIPTION, RANGE_SLIDER_END_HANDLE_DESCRIPTION, + RANGE_SLIDER_TOOLTIP_INPUT_LABEL, + RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION, } from "./generated/i18n/i18n-defaults.js"; // Styles @@ -860,6 +862,14 @@ class RangeSlider extends SliderBase implements IFormInputElement { return [`${this._id}-accName`, `${this._id}-sliderDesc`].join(" ").trim(); } + get _ariaLabelledByInputText() { + return RangeSlider.i18nBundle.getText(RANGE_SLIDER_TOOLTIP_INPUT_LABEL); + } + + get _ariaDescribedByInputText() { + return RangeSlider.i18nBundle.getText(RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION); + } + get styles() { return { progress: { diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index fcd4ac05b9ad..691686a4150d 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -17,38 +17,39 @@ {{/inline}} {{#*inline "handles"}} -
+
- - {{#if showTooltip}} -
- {{#if inputTooltip}} - - {{else}} - {{tooltipValue}} - {{/if}} -
+ data-sap-focus-ref + part="handle" + > + +
+ {{#if showTooltip}} +
+ {{#if inputTooltip}} + + {{else}} + {{tooltipValue}} {{/if}} +
+ {{/if}}
{{/inline}} diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index e4111f0ec201..b036defe9b05 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -289,13 +289,13 @@ abstract class SliderBase extends UI5Element { } _onkeydown(e: KeyboardEvent) { - const target = (e.target as HTMLElement); + const target = e.target as HTMLElement; - if (isF2(e) && target.querySelector("ui5-input")) { - (target.querySelector("ui5-input") as HTMLElement).focus(); + if (isF2(e) && target.classList.contains("ui5-slider-handle")) { + (this.shadowRoot?.querySelector("ui5-input") as HTMLElement).focus(); } - if (this.disabled || this._effectiveStep === 0 || target.hasAttribute("ui5-input")) { + if (this.disabled || this._effectiveStep === 0 || target.hasAttribute("ui5-slider-handle")) { return; } @@ -751,6 +751,14 @@ abstract class SliderBase extends UI5Element { get _ariaLabelledByInputRefs() { return [`${this._id}-accName`, `${this._id}-sliderInputLabel`].join(" ").trim(); } + + get _ariaDescribedByInputText() { + return ""; + } + + get _ariaLabelledByInputText() { + return ""; + } } export default SliderBase; diff --git a/packages/main/src/i18n/messagebundle.properties b/packages/main/src/i18n/messagebundle.properties index 0cb6c66a23ca..d8ea32271b4b 100644 --- a/packages/main/src/i18n/messagebundle.properties +++ b/packages/main/src/i18n/messagebundle.properties @@ -496,6 +496,12 @@ SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value #XACT: ARIA label for slider tooltip input SLIDER_TOOLTIP_INPUT_LABEL = Current value +#XACT: ARIA label for range slider tooltip input +RANGE_SLIDER_TOOLTIP_INPUT_LABEL = Current value + +#XACT: ARIA description for range slider tooltip input +RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value + #XTOL: tooltip for decrease button of the StepInput STEPINPUT_DEC_ICON_TITLE=Decrease diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index 928b81220cb5..09d84df1f8c5 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -100,9 +100,7 @@ background: var(--_ui5_slider_handle_background); border: var(--_ui5_slider_handle_border); border-radius: var(--_ui5_slider_handle_border_radius); - margin-inline-start: calc(-1 * var(--_ui5_slider_handle_width) / 2); - top: var(--_ui5_slider_handle_top); - position: absolute; + position: relative; outline: none; height: var(--_ui5_slider_handle_height); width: var(--_ui5_slider_handle_width); @@ -148,7 +146,13 @@ border: var(--_ui5_slider_handle_focus_border); } -.ui5-slider-tooltip { +.ui5-slider-handle-container { + position: absolute; + margin-inline-start: calc(-1 * var(--_ui5_slider_handle_width) / 2); + top: var(--_ui5_slider_handle_top); +} + +.ui5-slider-handle-container .ui5-slider-tooltip { display: flex; justify-content: center; align-items: center; From 5e2f4b4fd12b79adf0d11aeef30fe7c3c1e8dc83 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 31 Jul 2024 11:27:28 +0300 Subject: [PATCH 08/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip apply the accessibility improvements to the range slider --- packages/main/src/RangeSlider.hbs | 101 +++++++++++++++--------------- packages/main/src/Slider.hbs | 1 + 2 files changed, 53 insertions(+), 49 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 185c89f97adb..2f807274b518 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -29,56 +29,58 @@ {{/inline}} {{#*inline "handles"}} -
- +
- {{#if showTooltip}} -
- {{#if inputTooltip}} - - {{else}} - {{tooltipStartValue}} - {{/if}} -
- {{/if}} -
+
+ -
- + {{#if showTooltip}} +
+ {{#if inputTooltip}} + + {{else}} + {{tooltipStartValue}} + {{/if}} +
+ {{/if}} +
+
+
+
+ - {{#if showTooltip}} + {{#if showTooltip}}
{{#if inputTooltip}} {{else}} {{tooltipEndValue}} {{/if}}
- {{/if}} + {{/if}} +
{{/inline}} diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 691686a4150d..a3753e72ed70 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -45,6 +45,7 @@ type="Number" accessible-name-ref="{{_ariaLabelledByInputRefs}}" @change="{{_onInputChange}}" + data-sap-ui-end-value > {{else}} {{tooltipValue}} From 3f0a96d74db11a751fa515d8483eed9e94884b2d Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 12 Aug 2024 13:30:17 +0300 Subject: [PATCH 09/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip middle commit --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 4 ++-- packages/main/src/themes/SliderBase.css | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 2f807274b518..c163d29f347b 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -87,7 +87,7 @@ type="Number" value="{{endValue}}" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @change="{{_onInputChange}}" + @ui5-change="{{_onInputChange}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index a3753e72ed70..eeda08f298a1 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -44,7 +44,7 @@ value="{{value}}" type="Number" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @change="{{_onInputChange}}" + @ui5-change="{{_onInputChange}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index b036defe9b05..98f606b03860 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -745,11 +745,11 @@ abstract class SliderBase extends UI5Element { } get _ariaDescribedByHandleRefs() { - return [`${this._id}-accName`, `${this._id}-sliderInputDesc`].join(" ").trim(); + return ["ui5-slider-accName", `ui5-slider-sliderInputDesc`].join(" ").trim(); } get _ariaLabelledByInputRefs() { - return [`${this._id}-accName`, `${this._id}-sliderInputLabel`].join(" ").trim(); + return ["ui5-slider-accName", "ui5-slider-InputLabel"].join(" ").trim(); } get _ariaDescribedByInputText() { diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index 09d84df1f8c5..781497cde35f 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -177,7 +177,7 @@ padding: 0; } -.ui5-slider-tooltip ui5-input { +.ui5-slider-tooltip [ui5-input] { background: transparent; border: none; box-shadow: none; From 9389fafb2a204d0c1f22d146576a20c39d955dd1 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 12 Aug 2024 13:30:17 +0300 Subject: [PATCH 10/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip middle commit --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 4 ++-- packages/main/src/themes/SliderBase.css | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 2f807274b518..c163d29f347b 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -87,7 +87,7 @@ type="Number" value="{{endValue}}" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @change="{{_onInputChange}}" + @ui5-change="{{_onInputChange}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index a3753e72ed70..eeda08f298a1 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -44,7 +44,7 @@ value="{{value}}" type="Number" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @change="{{_onInputChange}}" + @ui5-change="{{_onInputChange}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index b036defe9b05..98f606b03860 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -745,11 +745,11 @@ abstract class SliderBase extends UI5Element { } get _ariaDescribedByHandleRefs() { - return [`${this._id}-accName`, `${this._id}-sliderInputDesc`].join(" ").trim(); + return ["ui5-slider-accName", `ui5-slider-sliderInputDesc`].join(" ").trim(); } get _ariaLabelledByInputRefs() { - return [`${this._id}-accName`, `${this._id}-sliderInputLabel`].join(" ").trim(); + return ["ui5-slider-accName", "ui5-slider-InputLabel"].join(" ").trim(); } get _ariaDescribedByInputText() { diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index 09d84df1f8c5..781497cde35f 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -177,7 +177,7 @@ padding: 0; } -.ui5-slider-tooltip ui5-input { +.ui5-slider-tooltip [ui5-input] { background: transparent; border: none; box-shadow: none; From 917a8d9fcfa1c6470ba49ed0f58ae065ab0b0e3e Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 12 Aug 2024 13:42:24 +0300 Subject: [PATCH 11/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip rename the property --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index c163d29f347b..728a860c5d40 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -82,7 +82,7 @@ {{#if showTooltip}}
- {{#if inputTooltip}} + {{#if editableTooltip}} {{#if showTooltip}}
- {{#if inputTooltip}} + {{#if editableTooltip}} Date: Mon, 12 Aug 2024 14:30:37 +0300 Subject: [PATCH 12/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix merging --- packages/main/src/RangeSlider.hbs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index cbe3276a576b..728a860c5d40 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -29,25 +29,7 @@ {{/inline}} {{#*inline "handles"}} -<<<<<<< HEAD
-======= -
- ->>>>>>> main
Date: Mon, 12 Aug 2024 14:49:51 +0300 Subject: [PATCH 13/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix the test pages --- packages/main/test/pages/RangeSlider.html | 2 +- packages/main/test/pages/Slider.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/test/pages/RangeSlider.html b/packages/main/test/pages/RangeSlider.html index d7f79361901d..24a3da0ae5bf 100644 --- a/packages/main/test/pages/RangeSlider.html +++ b/packages/main/test/pages/RangeSlider.html @@ -41,7 +41,7 @@

Disabled Range Slider

Range Slider with steps, tooltips, tickmarks and labels

- +
diff --git a/packages/main/test/pages/Slider.html b/packages/main/test/pages/Slider.html index 2f282a648954..d3f51a9240ea 100644 --- a/packages/main/test/pages/Slider.html +++ b/packages/main/test/pages/Slider.html @@ -45,7 +45,7 @@

Inactive slider with no steps and tooltip

Slider with steps, tooltips, tickmarks and labels

- +

Slider with float number step (1.25), tooltips, tickmarks and labels between 3 steps (3.75 value)

From f01cb06226156ab36ae0554d2a6a570564b217d1 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 14 Aug 2024 13:00:42 +0300 Subject: [PATCH 14/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix bugs --- packages/main/src/RangeSlider.hbs | 56 +++++++++++++------------ packages/main/src/RangeSlider.ts | 14 ++++--- packages/main/src/Slider.hbs | 1 + packages/main/src/Slider.ts | 4 ++ packages/main/src/SliderBase.hbs | 8 ++-- packages/main/src/SliderBase.ts | 2 +- packages/main/src/themes/SliderBase.css | 6 +-- 7 files changed, 50 insertions(+), 41 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 728a860c5d40..34caf9009fd0 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -46,23 +46,24 @@ aria-disabled="{{_ariaDisabled}}" > + {{#if showTooltip}} -
- {{#if inputTooltip}} - - {{else}} - {{tooltipStartValue}} - {{/if}} -
+
+ {{#if editableTooltip}} + + {{else}} + {{tooltipStartValue}} {{/if}}
+ {{/if}}
+
- {{#if showTooltip}} -
- {{#if editableTooltip}} - - {{else}} - {{tooltipEndValue}} - {{/if}} -
+ {{#if showTooltip}} +
+ {{#if editableTooltip}} + + {{else}} + {{tooltipEndValue}} {{/if}}
+ {{/if}}
{{/inline}} diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 1cc5143309d2..dd960095f1dc 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -281,7 +281,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { * Resets the stored Range Slider's initial values saved when it was first focused * @private */ - _onfocusout() { + _onfocusout(e: FocusEvent) { if (this._isFocusing()) { this._preventFocusOut(); return; @@ -291,11 +291,15 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._startValueInitial = undefined; this._endValueInitial = undefined; - if (this.showTooltip) { + if (this.showTooltip && !(e.relatedTarget as HTMLInputElement)?.hasAttribute("ui5-input")) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; } } + _onInputFocusOut() { + this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + } + /** * Handles keyup logic. If one of the handles came across the other * swap the start and end values. Reset the affected value by the finished @@ -851,15 +855,15 @@ class RangeSlider extends SliderBase implements IFormInputElement { } get _ariaLabelledByStartHandleRefs() { - return [`${this._id}-accName`, `${this._id}-startHandleDesc`].join(" ").trim(); + return ["ui5-slider-accName", "ui5-slider-startHandleDesc"].join(" ").trim(); } get _ariaLabelledByEndHandleRefs() { - return [`${this._id}-accName`, `${this._id}-endHandleDesc`].join(" ").trim(); + return ["ui5-slider-accName", "ui5-slider-endHandleDesc"].join(" ").trim(); } get _ariaLabelledByProgressBarRefs() { - return [`${this._id}-accName`, `${this._id}-sliderDesc`].join(" ").trim(); + return ["ui5-slider-accName", "ui5-slider-sliderDesc"].join(" ").trim(); } get _ariaLabelledByInputText() { diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index ab16064f0a17..8ba61812f8e6 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -45,6 +45,7 @@ type="Number" accessible-name-ref="{{_ariaLabelledByInputRefs}}" @ui5-change="{{_onInputChange}}" + @focusout="{{_onInputFocusOut}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index f49497592dd8..f010ae6e858e 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -264,6 +264,10 @@ class Slider extends SliderBase implements IFormInputElement { this.value = ctor.clipValue(value, this._effectiveMin, this._effectiveMax); } + _onInputFocusOut() { + this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + } + /** Determines if the press is over the handle * @private */ diff --git a/packages/main/src/SliderBase.hbs b/packages/main/src/SliderBase.hbs index 6648a00d9dcc..8f3b5b4e1405 100644 --- a/packages/main/src/SliderBase.hbs +++ b/packages/main/src/SliderBase.hbs @@ -37,10 +37,10 @@ {{> handles}} - {{accessibleName}} - {{_ariaLabelledByText}} - {{_ariaDescribedByInputText}} - {{_ariaLabelledByInputText}} + {{accessibleName}} + {{_ariaLabelledByText}} + {{_ariaDescribedByInputText}} + {{_ariaLabelledByInputText}} diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 7c51ff73a4f0..66c365c28999 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -292,7 +292,7 @@ abstract class SliderBase extends UI5Element { const target = e.target as HTMLElement; if (isF2(e) && target.classList.contains("ui5-slider-handle")) { - (this.shadowRoot?.querySelector("ui5-input") as HTMLElement).focus(); + (target.parentNode!.querySelector(".ui5-slider-handle-container ui5-input") as HTMLElement).focus(); } if (this.disabled || this._effectiveStep === 0 || target.hasAttribute("ui5-slider-handle")) { diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index 781497cde35f..08404195024b 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -173,14 +173,12 @@ box-sizing: var(--_ui5_slider_tooltip_border_box); } -:host([input-tooltip]) .ui5-slider-tooltip { +:host([editable-tooltip]) .ui5-slider-tooltip { padding: 0; + box-shadow: none; } .ui5-slider-tooltip [ui5-input] { - background: transparent; - border: none; - box-shadow: none; height: 100%; width: 100%; margin: 0; From a8e080875766b4a8dd22ed1ece272fcddb00d90c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 14 Aug 2024 16:27:29 +0300 Subject: [PATCH 15/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix and add tests for ui5-slider --- packages/main/test/specs/Slider.spec.js | 99 ++++++++++++++++++++----- 1 file changed, 81 insertions(+), 18 deletions(-) diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index 4780c391ee2d..2526f942a78c 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -6,33 +6,34 @@ describe("Slider basic interactions", () => { await browser.url(`test/pages/Slider.html`); const slider = await browser.$("#basic-slider"); + const sliderHandleContainer = await slider.shadow$(".ui5-slider-handle-container"); const sliderHandle = await slider.shadow$(".ui5-slider-handle"); - assert.strictEqual((await sliderHandle.getCSSProperty("left")).value, "0px", "Initially if no value is set, the Slider handle is at the beginning of the Slider"); + assert.strictEqual((await sliderHandleContainer.getCSSProperty("left")).value, "0px", "Initially if no value is set, the Slider handle is at the beginning of the Slider"); await browser.setWindowSize(1257, 2000); await slider.setProperty("value", 3); - assert.strictEqual(await sliderHandle.getAttribute("style"), "left: 30%;", "Slider handle should be 30% from the start"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "left: 30%;", "Slider handle should be 30% from the start"); await slider.click(); - assert.strictEqual(await sliderHandle.getAttribute("style"), "left: 50%;", "Slider handle should be in the middle of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "left: 50%;", "Slider handle should be in the middle of the slider"); assert.strictEqual(await slider.getProperty("value"), 5, "Slider current value should be 5"); await sliderHandle.dragAndDrop({ x: 300, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "left: 80%;", "Slider handle should be 80% from the start of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "left: 80%;", "Slider handle should be 80% from the start of the slider"); assert.strictEqual(await slider.getProperty("value"), 8, "Slider current value should be 8"); await sliderHandle.dragAndDrop({ x: 100, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "left: 90%;", "Slider handle should be 90% from the start"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "left: 90%;", "Slider handle should be 90% from the start"); assert.strictEqual(await slider.getProperty("value"), 9, "Slider current value should be 9"); await sliderHandle.dragAndDrop({ x:-100, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "left: 80%;", "Slider handle should be at the end of the slider and not beyond its boundaries"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "left: 80%;", "Slider handle should be at the end of the slider and not beyond its boundaries"); assert.strictEqual(await slider.getProperty("value"), 8, "Slider current value should be 8"); }); @@ -157,6 +158,16 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await sliderTooltipValue.getText(), "2", "Slider tooltip should display value of 2"); }); + it("Tooltip input is displayed showing the current value", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + + await slider.setProperty("value", 8); + const sliderTooltipInputValue = await sliderTooltipInput.getProperty("value"); + + assert.strictEqual(await sliderTooltipInputValue, "8", "Slider input has the correct value"); + }); + it("Slider Tooltip should become visible when slider is focused", async () => { const slider = await browser.$("#basic-slider-with-tooltip"); const sliderTooltip = await slider.shadow$(".ui5-slider-tooltip"); @@ -175,6 +186,34 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual((await sliderTooltip.getCSSProperty("visibility")).value, "visible", "Slider tooltip should be shown"); }); + it("Slider Tooltip should not be closed on focusout if input tooltip is clicked", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + + await slider.click(); + assert.strictEqual(await slider.getProperty("_tooltipVisibility"), "visible", "Slider tooltip visibility property should be 'visible'"); + + await sliderTooltipInput.click(); + + assert.strictEqual(await sliderTooltipInput.getProperty("focused"), true, "The tooltip is not closed and the input is focused"); + }); + + it("Input tooltip value change should change the slider's value", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + + await slider.setProperty("value", 16); + await sliderTooltipInput.setProperty("value", ""); + + await sliderHandle.click(); + await sliderTooltipInput.click(); + await browser.keys(["1", "2"]); + await browser.keys("Enter"); + + assert.strictEqual(await slider.getProperty("value"), 12, "The input value is reflected in the slider"); + }); + it("Slider Tooltip should stay visible when slider is focused and mouse moves away", async () => { const slider = await browser.$("#basic-slider-with-tooltip"); const sliderTooltip = await slider.shadow$(".ui5-slider-tooltip"); @@ -210,6 +249,21 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual((await sliderTooltip.getCSSProperty("visibility")).value, "hidden", "Slider tooltip should be shown"); }); + it("Input tooltip should become hidden when input is looses focus", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const anotherSlider = await browser.$("#basic-slider"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + + await slider.click(); + await sliderTooltipInput.click(); + + assert.strictEqual(await slider.getProperty("_tooltipVisibility"), "visible", "Slider tooltip visibility property should be 'visible'"); + + await anotherSlider.click(); + + assert.strictEqual(await slider.getProperty("_tooltipVisibility"), "hidden", "Slider tooltip visibility property should be 'visible'"); + }); + it("Slider have correct number of labels and tickmarks based on the defined step and labelInterval properties", async () => { const slider = await browser.$("#slider-tickmarks-tooltips-labels"); const labelsContainer = await slider.shadow$(".ui5-slider-labels"); @@ -252,10 +306,9 @@ describe("Accessibility", async () => { it("Aria attributes are set correctly", async () => { const slider = await browser.$("#basic-slider"); const sliderHandle = await slider.shadow$(".ui5-slider-handle"); - const sliderId = await slider.getProperty("_id"); assert.strictEqual(await sliderHandle.getAttribute("aria-labelledby"), - `${sliderId}-accName ${sliderId}-sliderDesc`, "aria-labelledby is set correctly"); + "ui5-slider-accName ui5-sliderDesc", "aria-labelledby is set correctly"); assert.strictEqual(await sliderHandle.getAttribute("aria-valuemin"), `${await slider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await sliderHandle.getAttribute("aria-valuemax"), @@ -264,6 +317,15 @@ describe("Accessibility", async () => { `${await slider.getProperty("value")}`, "aria-valuenow is set correctly"); }); + it("Aria attributes are set correctly to the tooltip input", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + + assert.strictEqual(await sliderTooltipInput.getAttribute("accessible-name-ref"), + "ui5-slider-accName ui5-slider-InputLabel"); + }); + it("Click anywhere in the Slider should focus the Slider's handle", async () => { await browser.url(`test/pages/Slider.html`); @@ -437,49 +499,50 @@ describe("Testing resize handling and RTL support", () => { it("Testing RTL support", async () => { const slider = await browser.$("#basic-slider-rtl"); const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const sliderHandleContainer = await slider.shadow$(".ui5-slider-handle-container"); - assert.strictEqual((await sliderHandle.getCSSProperty("right")).value, "0px", "Initially if no value is set, the Slider handle is at the right of the Slider"); + assert.strictEqual((await sliderHandleContainer.getCSSProperty("right")).value, "0px", "Initially if no value is set, the Slider handle is at the right of the Slider"); await slider.setProperty("value", 3); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 30%;", "Slider handle should be 30% from the right"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 30%;", "Slider handle should be 30% from the right"); await slider.click(); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 50%;", "Slider handle should be in the middle of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 50%;", "Slider handle should be in the middle of the slider"); assert.strictEqual(await slider.getProperty("value"), 5, "Slider current value should be 5"); await sliderHandle.dragAndDrop({ x: -300, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 80%;", "Slider handle should be 80% from the right of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 80%;", "Slider handle should be 80% from the right of the slider"); assert.strictEqual(await slider.getProperty("value"), 8, "Slider current value should be 8"); await sliderHandle.dragAndDrop({ x: -100, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 90%;", "Slider handle should be 90% from the right"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 90%;", "Slider handle should be 90% from the right"); assert.strictEqual(await slider.getProperty("value"), 9, "Slider current value should be 9"); await sliderHandle.dragAndDrop({ x: -150, y: 1 }); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 100%;", "Slider handle should be at the left of the slider and not beyond its boundaries"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 100%;", "Slider handle should be at the left of the slider and not beyond its boundaries"); assert.strictEqual(await slider.getProperty("value"), 10, "Slider current value should be 10"); }); it("Testing RTL KBH support", async () => { const slider = await browser.$("#basic-slider-rtl"); - const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const sliderHandleContainer = await slider.shadow$(".ui5-slider-handle-container"); await slider.setProperty("value", 0); - assert.strictEqual((await sliderHandle.getCSSProperty("right")).value, "0px", "Initially if no value is set, the Slider handle is at the right of the Slider"); + assert.strictEqual((await sliderHandleContainer.getCSSProperty("right")).value, "0px", "Initially if no value is set, the Slider handle is at the right of the Slider"); await slider.keys("ArrowLeft"); await slider.keys("ArrowLeft"); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 20%;", "Slider handle should be 20% from the right of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 20%;", "Slider handle should be 20% from the right of the slider"); assert.strictEqual(await slider.getProperty("value"), 2, "Slider current value should be 2"); await slider.keys("ArrowRight"); - assert.strictEqual(await sliderHandle.getAttribute("style"), "right: 10%;", "Slider handle should be 10% from the right of the slider"); + assert.strictEqual(await sliderHandleContainer.getAttribute("style"), "right: 10%;", "Slider handle should be 10% from the right of the slider"); assert.strictEqual(await slider.getProperty("value"), 1, "Slider current value should be 1"); }); From 791c0356ebf173a32a1d98f93c16632199f496fb Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 16 Aug 2024 11:29:26 +0300 Subject: [PATCH 16/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip add handle-container part, fix ui5-color-picker --- packages/main/src/RangeSlider.hbs | 4 ++-- packages/main/src/Slider.hbs | 2 +- packages/main/src/themes/ColorPicker.css | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 34caf9009fd0..3c435f81c492 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -29,7 +29,7 @@ {{/inline}} {{#*inline "handles"}} -
+
{{/if}}
-
+
+
Date: Tue, 20 Aug 2024 08:29:35 +0300 Subject: [PATCH 17/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip apply code review feedback --- packages/main/src/RangeSlider.hbs | 2 ++ packages/main/src/Slider.hbs | 1 + packages/main/src/Slider.ts | 6 +++++- packages/main/src/SliderBase.ts | 19 ++++++++++++++++++- packages/main/src/themes/ColorPicker.css | 5 +++++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 3c435f81c492..8432653c9ff1 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -57,6 +57,7 @@ accessible-name-ref="{{_ariaLabelledByInputRefs}}" @ui5-change="{{_onInputChange}}" @focusout="{{_onInputFocusOut}}" + @keydown="{{_onInputKeydown}}" start-value > {{else}} @@ -91,6 +92,7 @@ accessible-name-ref="{{_ariaLabelledByInputRefs}}" @ui5-change="{{_onInputChange}}" @focusout="{{_onInputFocusOut}}" + @keydown="{{_onInputKeydown}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index e6a1e3c02b9e..b568848c4f61 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -46,6 +46,7 @@ accessible-name-ref="{{_ariaLabelledByInputRefs}}" @ui5-change="{{_onInputChange}}" @focusout="{{_onInputFocusOut}}" + @keydown="{{_onInputKeydown}}" data-sap-ui-end-value > {{else}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index f010ae6e858e..d997159fb59c 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -261,7 +261,11 @@ class Slider extends SliderBase implements IFormInputElement { const input = e.target as HTMLInputElement; const value = parseFloat(input.value); - this.value = ctor.clipValue(value, this._effectiveMin, this._effectiveMax); + if (!this._isArrowChange) { + this.value = ctor.clipValue(value, this._effectiveMin, this._effectiveMax); + } + + this._isArrowChange = false; } _onInputFocusOut() { diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 66c365c28999..4804f4e245e4 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -145,6 +145,9 @@ abstract class SliderBase extends UI5Element { @property({ type: Boolean }) _hiddenTickmarks = false; + @property({ type: Boolean }) + _isArrowChange = false; + _resizeHandler: ResizeObserverCallback; _moveHandler: (e: TouchEvent | MouseEvent) => void; _upHandler: (e: TouchEvent | MouseEvent) => void; @@ -299,13 +302,27 @@ abstract class SliderBase extends UI5Element { return; } - if (SliderBase._isActionKey(e)) { + if (SliderBase._isActionKey(e) && target && !target.hasAttribute("ui5-input")) { e.preventDefault(); this._isUserInteraction = true; this._handleActionKeyPress(e); } } + _onInputKeydown(e: KeyboardEvent) { + const target = e.target as HTMLElement; + + const isUpAction = SliderBase._isIncreaseValueAction(e); + const isDownAction = SliderBase._isDecreaseValueAction(e); + + if (isUpAction || isDownAction) { + this._isArrowChange = true; + } + + if (isF2(e) && target.hasAttribute("ui5-input")) { + (target.parentNode!.parentNode!.querySelector(".ui5-slider-handle") as HTMLElement).focus(); + } + } _onkeyup() { if (this.disabled) { diff --git a/packages/main/src/themes/ColorPicker.css b/packages/main/src/themes/ColorPicker.css index 23d285c3917d..5bc5a98ae79b 100644 --- a/packages/main/src/themes/ColorPicker.css +++ b/packages/main/src/themes/ColorPicker.css @@ -70,6 +70,11 @@ margin-top: var(--_ui5_color_picker_slider_handle_margin_top); } +[ui5-slider]::part(handle-container):focus-within { + margin-inline-start: unset; + margin-top: unset; +} + [ui5-slider]::part(handle)::after { content: ""; border: 2px solid #fff; From c5c45a285686b3db6a2c25deb009fc476142c110 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 20 Aug 2024 15:40:01 +0300 Subject: [PATCH 18/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip apply code review feedback --- packages/main/src/RangeSlider.hbs | 8 ++++---- packages/main/src/RangeSlider.ts | 34 ++++++++++++++++++++++++++++--- packages/main/src/Slider.hbs | 2 +- packages/main/src/Slider.ts | 27 +++++++++++++++++++----- packages/main/src/SliderBase.ts | 17 +++++++--------- 5 files changed, 65 insertions(+), 23 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 8432653c9ff1..7ec51468aeb9 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -48,17 +48,17 @@
- {{#if showTooltip}} + {{#if showTooltip}}
{{#if editableTooltip}} {{else}} {{tooltipStartValue}} @@ -90,10 +90,10 @@ type="Number" value="{{endValue}}" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @ui5-change="{{_onInputChange}}" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" data-sap-ui-end-value + tabindex="-1" > {{else}} {{tooltipEndValue}} diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index dd960095f1dc..b27a06b7ed02 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -131,6 +131,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { _secondHandlePositionFromStart?: number; _selectedRange?: number; _reversedValues = false; + _lastValidStartValue: string; + _lastValidEndValue: string; static i18nBundle: I18nBundle; @@ -151,6 +153,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { super(); this._stateStorage.startValue = undefined; this._stateStorage.endValue = undefined; + this._lastValidStartValue = this.min.toString(); + this._lastValidEndValue = this.max.toString(); } get tooltipStartValue() { @@ -204,6 +208,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { + this._updateInputValues(); + if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -296,8 +302,9 @@ class RangeSlider extends SliderBase implements IFormInputElement { } } - _onInputFocusOut() { + _onInputFocusOut(e: FocusEvent) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + this._updateValueFromInput(e); } /** @@ -551,13 +558,18 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._endValueAtBeginningOfAction = undefined; } - _onInputChange(e: Event) { + _updateValueFromInput(e: Event) { const ctor = this.constructor as typeof RangeSlider; const input = e.target as HTMLInputElement; const value = ctor.clipValue(parseFloat(input.value), this._effectiveMin, this._effectiveMax); + const isValueValid = value >= this._effectiveMin && value <= this._effectiveMax; - if (input.hasAttribute("start-value")) { + if (!isValueValid) { + return; + } + + if (input.hasAttribute("data-sap-ui-start-value")) { this.startValue = value; return; } @@ -774,6 +786,22 @@ class RangeSlider extends SliderBase implements IFormInputElement { } } + _updateInputValues() { + const startValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]") as Input; + const endValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]") as Input; + + if (this.editableTooltip && startValueInput && endValueInput) { + const isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; + const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; + + startValueInput.value = startValueInput.value ? startValueInput.value : this._lastValidStartValue; + endValueInput.value = endValueInput.value ? endValueInput.value : this._lastValidEndValue; + + this._lastValidStartValue = (!!startValueInput.value && isStartValueValid) ? startValueInput.value : this._lastValidStartValue; + this._lastValidEndValue = (!!endValueInput.value && isEndValueValid) ? endValueInput.value : this._lastValidEndValue; + } + } + /** * Swaps the start and end values of the handles if one came accros the other: * - If the start value is greater than the endValue swap them and their handles diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index b568848c4f61..6a68623565f9 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -44,10 +44,10 @@ value="{{value}}" type="Number" accessible-name-ref="{{_ariaLabelledByInputRefs}}" - @ui5-change="{{_onInputChange}}" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" data-sap-ui-end-value + tabindex="-1" > {{else}} {{tooltipValue}} diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index d997159fb59c..9cf6259f5582 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -94,6 +94,7 @@ class Slider extends SliderBase implements IFormInputElement { _valueOnInteractionStart?: number; _progressPercentage = 0; _handlePositionFromStart = 0; + _lastValidInputValue: string; get formFormattedValue() { return this.value.toString(); @@ -104,6 +105,7 @@ class Slider extends SliderBase implements IFormInputElement { constructor() { super(); this._stateStorage.value = undefined; + this._lastValidInputValue = this.min.toString(); } /** @@ -118,6 +120,8 @@ class Slider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { + this._updateInputValue(); + if (!this.isCurrentStateOutdated()) { return; } @@ -256,20 +260,33 @@ class Slider extends SliderBase implements IFormInputElement { this._valueOnInteractionStart = undefined; } - _onInputChange(e: Event) { + _updateValueFromInput(e: Event) { const ctor = this.constructor as typeof Slider; const input = e.target as HTMLInputElement; const value = parseFloat(input.value); + const isValueValid = value >= this._effectiveMin && value <= this._effectiveMax; - if (!this._isArrowChange) { - this.value = ctor.clipValue(value, this._effectiveMin, this._effectiveMax); + if (!isValueValid) { + return; } - this._isArrowChange = false; + this.value = value; } - _onInputFocusOut() { + _onInputFocusOut(e: FocusEvent) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + this._updateValueFromInput(e); + } + + _updateInputValue() { + const tooltipInput = this.shadowRoot!.querySelector("ui5-input") as Input; + + if (this.editableTooltip && tooltipInput) { + const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + + tooltipInput.value = tooltipInput.value ? tooltipInput.value : this._lastValidInputValue; + this._lastValidInputValue = (!!tooltipInput.value && isTooltipInputValueValid) ? tooltipInput.value : this._lastValidInputValue; + } } /** Determines if the press is over the handle diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 4804f4e245e4..5a01d4a28eab 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -9,6 +9,7 @@ import type { PassiveEventListenerObject } from "@ui5/webcomponents-base/dist/ty import "@ui5/webcomponents-icons/dist/direction-arrows.js"; import { isEscape, isHome, isEnd, isUp, isDown, isRight, isLeft, isUpCtrl, isDownCtrl, isRightCtrl, isLeftCtrl, isPlus, isMinus, isPageUp, isPageDown, isF2, + isEnter, } from "@ui5/webcomponents-base/dist/Keys.js"; // Styles @@ -145,9 +146,6 @@ abstract class SliderBase extends UI5Element { @property({ type: Boolean }) _hiddenTickmarks = false; - @property({ type: Boolean }) - _isArrowChange = false; - _resizeHandler: ResizeObserverCallback; _moveHandler: (e: TouchEvent | MouseEvent) => void; _upHandler: (e: TouchEvent | MouseEvent) => void; @@ -195,6 +193,8 @@ abstract class SliderBase extends UI5Element { _onmousedown(e: TouchEvent | MouseEvent) {} // eslint-disable-line + _updateValueFromInput(e: KeyboardEvent | FocusEvent) {} // eslint-disable-line + _handleActionKeyPress(e: Event) {} // eslint-disable-line // used in base template, but implemented in subclasses @@ -312,16 +312,13 @@ abstract class SliderBase extends UI5Element { _onInputKeydown(e: KeyboardEvent) { const target = e.target as HTMLElement; - const isUpAction = SliderBase._isIncreaseValueAction(e); - const isDownAction = SliderBase._isDecreaseValueAction(e); - - if (isUpAction || isDownAction) { - this._isArrowChange = true; - } - if (isF2(e) && target.hasAttribute("ui5-input")) { (target.parentNode!.parentNode!.querySelector(".ui5-slider-handle") as HTMLElement).focus(); } + + if (isEnter(e)) { + this._updateValueFromInput(e); + } } _onkeyup() { From 13d14aeee03cc1a81915381372e1dbefe5aefdb5 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 20 Aug 2024 18:17:04 +0300 Subject: [PATCH 19/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix tests --- packages/main/src/RangeSlider.hbs | 1 - packages/main/src/Slider.ts | 1 - packages/main/test/specs/RangeSlider.spec.js | 21 +++++++++----------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 7ec51468aeb9..ab72cab4ee5b 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -33,7 +33,6 @@
= this._effectiveMin && value <= this._effectiveMax; diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index b836a929fdab..7ce2c39e28f9 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -7,7 +7,7 @@ describe("Testing Range Slider interactions", () => { await browser.setWindowSize(1257, 2000); const rangeSlider = await browser.$("#range-slider-tickmarks"); - const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const startHandle = await rangeSlider.shadow$(".ui5-slider-handle-container"); assert.strictEqual((await startHandle.getAttribute("style")).replace(" ", ""), "left:0%;", "Initially if no value is set, the Range Slider start-handle is at the beginning of the Range Slider"); @@ -28,7 +28,7 @@ describe("Testing Range Slider interactions", () => { it("Changing the endValue is reflected", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks"); - const endHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + const endHandle = await rangeSlider.shadow$$(".ui5-slider-inner .ui5-slider-handle-container")[1]; assert.strictEqual((await endHandle.getAttribute("style")).replace(" ", ""), "left:50%;", "Range Slider end-handle is should be 50% from the start the Range Slider"); await rangeSlider.setProperty("endValue", 10); @@ -67,9 +67,6 @@ describe("Testing Range Slider interactions", () => { it("Dragging the selected range should change both values and handles", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks"); - const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); - const endHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); - await rangeSlider.dragAndDrop({ x: 90, y: 1 }); assert.strictEqual(await rangeSlider.getProperty("startValue"), 8, "startValue should be 8"); @@ -359,7 +356,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-labelledby"), - `${rangeSliderId}-accName ${rangeSliderId}-sliderDesc`, "aria-labelledby is set correctly"); + "ui5-slider-accName ui5-slider-sliderDesc", "aria-labelledby is set correctly"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-valuemax"), @@ -375,7 +372,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await startHandle.getAttribute("aria-labelledby"), - `${rangeSliderId}-accName ${rangeSliderId}-startHandleDesc`, "aria-labelledby is set correctly"); + "ui5-slider-accName ui5-slider-startHandleDesc", "aria-labelledby is set correctly"); assert.strictEqual(await startHandle.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await startHandle.getAttribute("aria-valuemax"), @@ -390,7 +387,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await endHandle.getAttribute("aria-labelledby"), - `${rangeSliderId}-accName ${rangeSliderId}-endHandleDesc`, "aria-labelledby is set correctly"); + "ui5-slider-accName ui5-slider-endHandleDesc", "aria-labelledby is set correctly"); assert.strictEqual(await endHandle.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await endHandle.getAttribute("aria-valuemax"), @@ -956,8 +953,8 @@ describe("Accessibility: Testing keyboard handling", async () => { describe("Testing resize handling and RTL support", () => { it("Testing RTL support", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); - const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); - const endHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + const startHandle = await rangeSlider.shadow$(".ui5-slider-handle-container"); + const endHandle = await rangeSlider.shadow$$(".ui5-slider-inner .ui5-slider-handle-container")[1]; await rangeSlider.setAttribute("dir", "rtl"); await rangeSlider.setProperty("min", 0); @@ -996,8 +993,8 @@ describe("Testing resize handling and RTL support", () => { it("Testing RTL KBH support", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); - const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); - const endHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + const startHandle = await rangeSlider.shadow$(".ui5-slider-handle-container"); + const endHandle = await rangeSlider.shadow$$(".ui5-slider-inner .ui5-slider-handle-container")[1]; const rangeSliderSelection = await rangeSlider.shadow$(".ui5-slider-progress"); await rangeSlider.setAttribute("dir", "rtl"); From bc0d25b2218544769d0169cc5a9f1d260a16bb6a Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 22 Aug 2024 22:02:14 +0300 Subject: [PATCH 20/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix range slider test, add new ones --- packages/main/test/specs/RangeSlider.spec.js | 72 ++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 7ce2c39e28f9..39cec215e28b 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -149,7 +149,67 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSliderEndTooltipValue.getText(), "115", "Range Slider end tooltip should display value of 65"); }); + it("Tooltip input is displayed showing the current value", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderEndTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + + await rangeSlider.setProperty("startValue", 8); + const rangeSliderStartTooltipInputValue = await rangeSliderStartTooltipInput.getProperty("value"); + + await rangeSlider.setProperty("endValue", 12); + const rangeSliderEndTooltipInputValue = await rangeSliderEndTooltipInput.getProperty("value"); + + assert.strictEqual(await rangeSliderStartTooltipInputValue, "8", "Slider input has the correct value"); + assert.strictEqual(await rangeSliderEndTooltipInputValue, "12", "Slider input has the correct value"); + }); + + it("Tooltip input should not be closed on focusout if input tooltip is clicked", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + + await rangeSlider.click(); + assert.strictEqual(await rangeSlider.getProperty("_tooltipVisibility"), "visible", "Slider tooltip visibility property should be 'visible'"); + + await rangeSliderStartTooltipInput.click(); + + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("focused"), true, "The tooltip is not closed and the input is focused"); + }); + + it("Input tooltips value change should change the range slider's value", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + + await rangeSlider.setProperty("startValue", 4); + + await rangeSliderHandle.click(); + await rangeSliderStartTooltipInput.click(); + await rangeSliderStartTooltipInput.setProperty("value", "8"); + + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("startValue"), 8, "The input value is reflected in the slider"); + }); + + it("Input tooltip should become hidden when input is looses focus", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const anotherSlider = await browser.$("#basic-range-slider"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + + await rangeSlider.click(); + await rangeSliderStartTooltipInput.click(); + + assert.strictEqual(await rangeSlider.getProperty("_tooltipVisibility"), "visible", "Slider tooltip visibility property should be 'visible'"); + + await anotherSlider.click(); + + assert.strictEqual(await rangeSlider.getProperty("_tooltipVisibility"), "hidden", "Slider tooltip visibility property should be 'visible'"); + }); + it("Range Slider tooltips should become visible when range slider is focused", async () => { + await browser.url(`test/pages/RangeSlider.html`); + const rangeSlider = await browser.$("#basic-range-slider-with-tooltip"); const rangeSliderStartTooltip = await rangeSlider.shadow$(".ui5-slider-tooltip--start"); const rangeSliderEndTooltip = await rangeSlider.shadow$(".ui5-slider-tooltip--end"); @@ -396,6 +456,14 @@ describe("Accessibility", async () => { `${await rangeSlider.getProperty("endValue")}`, "aria-valuenow is set correctly"); }); + it("Aria attributes are set correctly to the tooltip input", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + + assert.strictEqual(await rangeSliderStartTooltipInput.getAttribute("accessible-name-ref"), + "ui5-slider-accName ui5-slider-InputLabel"); + }); + it("Aria-labelledby text is mapped correctly when values are swapped", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks"); const rangeSliderId = await rangeSlider.getProperty("_id"); @@ -992,12 +1060,16 @@ describe("Testing resize handling and RTL support", () => { }); it("Testing RTL KBH support", async () => { + await browser.url(`test/pages/RangeSlider.html`); + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); const startHandle = await rangeSlider.shadow$(".ui5-slider-handle-container"); const endHandle = await rangeSlider.shadow$$(".ui5-slider-inner .ui5-slider-handle-container")[1]; const rangeSliderSelection = await rangeSlider.shadow$(".ui5-slider-progress"); await rangeSlider.setAttribute("dir", "rtl"); + await rangeSlider.scrollIntoView(); + await rangeSlider.setProperty("min", 0); await rangeSlider.setProperty("max", 10); await rangeSlider.setProperty("step", 1); From 5de1e4dd9f5391f90f251d39adae6d3cbb7006a0 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 23 Aug 2024 09:38:32 +0300 Subject: [PATCH 21/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip implement value state for slider and range slider inputs when the value is invalid --- packages/main/src/RangeSlider.ts | 52 ++++++++++++++------ packages/main/src/Slider.ts | 29 +++++++++-- packages/main/src/SliderBase.ts | 3 ++ packages/main/test/specs/RangeSlider.spec.js | 16 ++++++ packages/main/test/specs/Slider.spec.js | 19 ++++++- 5 files changed, 97 insertions(+), 22 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index b27a06b7ed02..c2ed9b6fb1c4 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -208,7 +208,9 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { - this._updateInputValues(); + if (this.editableTooltip) { + this._updateInputValue(); + } if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -303,8 +305,18 @@ class RangeSlider extends SliderBase implements IFormInputElement { } _onInputFocusOut(e: FocusEvent) { + const tooltipInput = e.target as Input; + this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; this._updateValueFromInput(e); + this._updateInputValue(); + + const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + + if (!isTooltipInputValueValid) { + tooltipInput.value = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this._lastValidStartValue : this._lastValidEndValue; + tooltipInput.valueState = "None"; + } } /** @@ -559,22 +571,20 @@ class RangeSlider extends SliderBase implements IFormInputElement { } _updateValueFromInput(e: Event) { - const ctor = this.constructor as typeof RangeSlider; - const input = e.target as HTMLInputElement; - const value = ctor.clipValue(parseFloat(input.value), this._effectiveMin, this._effectiveMax); - const isValueValid = value >= this._effectiveMin && value <= this._effectiveMax; + const inputValue = parseFloat(input.value); + const isValueValid = inputValue >= this._effectiveMin && inputValue <= this._effectiveMax; if (!isValueValid) { return; } if (input.hasAttribute("data-sap-ui-start-value")) { - this.startValue = value; + this.startValue = inputValue; return; } - this.endValue = value; + this.endValue = inputValue; } /** @@ -786,20 +796,32 @@ class RangeSlider extends SliderBase implements IFormInputElement { } } - _updateInputValues() { + _updateInputValue() { const startValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]") as Input; const endValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]") as Input; - if (this.editableTooltip && startValueInput && endValueInput) { - const isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; - const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; + if (!startValueInput && !endValueInput) { + return; + } - startValueInput.value = startValueInput.value ? startValueInput.value : this._lastValidStartValue; - endValueInput.value = endValueInput.value ? endValueInput.value : this._lastValidEndValue; + const isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; + const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; - this._lastValidStartValue = (!!startValueInput.value && isStartValueValid) ? startValueInput.value : this._lastValidStartValue; - this._lastValidEndValue = (!!endValueInput.value && isEndValueValid) ? endValueInput.value : this._lastValidEndValue; + if (!isStartValueValid) { + startValueInput.valueState = "Negative"; + return; } + + if (!isEndValueValid) { + endValueInput.valueState = "Negative"; + return; + } + + this._lastValidStartValue = startValueInput.value; + this._lastValidEndValue = endValueInput.value; + + startValueInput.valueState = "None"; + endValueInput.valueState = "None"; } /** diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index eb69f90e3dcc..d70b098d139d 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -120,7 +120,9 @@ class Slider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { - this._updateInputValue(); + if (this.editableTooltip) { + this._updateInputValue(); + } if (!this.isCurrentStateOutdated()) { return; @@ -273,19 +275,36 @@ class Slider extends SliderBase implements IFormInputElement { } _onInputFocusOut(e: FocusEvent) { + const tooltipInput = e.target as Input; + this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; this._updateValueFromInput(e); + this._updateInputValue(); + + const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + + if (!isTooltipInputValueValid) { + tooltipInput.value = this._lastValidInputValue; + tooltipInput.valueState = "None"; + } } _updateInputValue() { const tooltipInput = this.shadowRoot!.querySelector("ui5-input") as Input; - if (this.editableTooltip && tooltipInput) { - const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + if (!tooltipInput) { + return; + } + + const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; - tooltipInput.value = tooltipInput.value ? tooltipInput.value : this._lastValidInputValue; - this._lastValidInputValue = (!!tooltipInput.value && isTooltipInputValueValid) ? tooltipInput.value : this._lastValidInputValue; + if (!isTooltipInputValueValid) { + tooltipInput.valueState = "Negative"; + return; } + + this._lastValidInputValue = tooltipInput.value; + tooltipInput.valueState = "None"; } /** Determines if the press is over the handle diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 5a01d4a28eab..295416dafa5e 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -197,6 +197,8 @@ abstract class SliderBase extends UI5Element { _handleActionKeyPress(e: Event) {} // eslint-disable-line + _updateInputValue() {} // eslint-disable-line + // used in base template, but implemented in subclasses abstract styles: { label: object, @@ -318,6 +320,7 @@ abstract class SliderBase extends UI5Element { if (isEnter(e)) { this._updateValueFromInput(e); + this._updateInputValue(); } } diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 39cec215e28b..efc79be45cab 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -192,6 +192,22 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSlider.getProperty("startValue"), 8, "The input value is reflected in the slider"); }); + it("Input tooltips value state should change to 'Negative' if value is invalid", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + + await rangeSlider.setProperty("startValue", 1); + + await rangeSliderHandle.click(); + await rangeSliderStartTooltipInput.click(); + await browser.keys(["2", "3"]); + + await browser.keys("Enter"); + + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("valueState"), "Negative", "The input value state is negative when the value is invalid"); + }); + it("Input tooltip should become hidden when input is looses focus", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); const anotherSlider = await browser.$("#basic-range-slider"); diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index 2526f942a78c..e08d81f0e249 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -203,15 +203,30 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + await slider.setProperty("value", 1); + + await sliderHandle.click(); + await sliderTooltipInput.click(); + await browser.keys(["2"]); + await browser.keys("Enter"); + + assert.strictEqual(await slider.getProperty("value"), 21, "The input value is reflected in the slider"); + }); + + it("Input tooltip should change the value state to error if it is invalid", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + await slider.setProperty("value", 16); await sliderTooltipInput.setProperty("value", ""); await sliderHandle.click(); await sliderTooltipInput.click(); - await browser.keys(["1", "2"]); + await browser.keys(["1", "2", "3"]); await browser.keys("Enter"); - assert.strictEqual(await slider.getProperty("value"), 12, "The input value is reflected in the slider"); + assert.strictEqual(await sliderTooltipInput.getProperty("valueState"), "Negative", "Value state is changed to negative when the value is invalid"); }); it("Slider Tooltip should stay visible when slider is focused and mouse moves away", async () => { From 459d15a6b2786d4bb1b8ede35069e80364c93eb5 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 23 Aug 2024 13:28:41 +0300 Subject: [PATCH 22/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip add more tests --- packages/main/test/specs/RangeSlider.spec.js | 74 ++++++++++++++++++++ packages/main/test/specs/Slider.spec.js | 67 ++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index efc79be45cab..41b51ad3bbb1 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -223,6 +223,80 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSlider.getProperty("_tooltipVisibility"), "hidden", "Slider tooltip visibility property should be 'visible'"); }); + it("F2 should switch the focus between the handle and the tooltip input", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + + + await rangeSliderHandle.click(); + await browser.keys("F2"); + + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("focused"), true, "Slider tooltip is focused on F2"); + + await browser.keys("F2"); + + const isHandleFocused = await browser.executeAsync(done => { + const focusedElement = document.getElementById("range-slider-tickmarks-labels").shadowRoot.activeElement; + const isHandleFocused = focusedElement.classList.contains("ui5-slider-handle"); + done(isHandleFocused); + }); + + assert.strictEqual(await isHandleFocused, true, "Slider tooltip visibility property should be 'visible'"); + }); + + it("Arrow up/down should not increase/decrease the value of the input", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderStartHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const rangeSliderEndHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const rangeSliderEndTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + + await rangeSlider.setProperty("startValue", 1); + await rangeSliderStartHandle.click(); + await rangeSliderStartTooltipInput.click(); + + await browser.keys("ArrowUp"); + assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The start value is not changed on arrow up"); + + await browser.keys("ArrowDown"); + assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The start value is not changed on arrow down"); + + await rangeSlider.setProperty("endValue", 10); + await rangeSliderEndHandle.click(); + await rangeSliderEndTooltipInput.click(); + + await browser.keys("ArrowUp"); + assert.strictEqual(await rangeSlider.getProperty("endValue"), 10, "The end value is not changed on arrow up"); + + await browser.keys("ArrowDown"); + assert.strictEqual(await rangeSlider.getProperty("endValue"), 10, "The end value is not changed on arrow down"); + }); + + it("Tab on slider handle should not move the focus to the tooltip input", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderEndTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + + assert.strictEqual(await rangeSliderStartTooltipInput.getAttribute("tabindex"), "-1", "The tooltip input is not tabbable"); + assert.strictEqual(await rangeSliderEndTooltipInput.getAttribute("tabindex"), "-1", "The tooltip input is not tabbable"); + }); + + it("Focus out with invalid value should reset it", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const nextSlider = await browser.$("#test-slider"); + + await rangeSlider.setProperty("startValue", 2); + await rangeSliderStartHandle.click(); + await rangeSliderStartTooltipInput.click(); + await browser.keys(["2", "3"]); + + await nextSlider.click(); + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "2", "Value is reset to the last valid one"); + }); + it("Range Slider tooltips should become visible when range slider is focused", async () => { await browser.url(`test/pages/RangeSlider.html`); diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index e08d81f0e249..d4cfa9ec3b67 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -229,6 +229,73 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await sliderTooltipInput.getProperty("valueState"), "Negative", "Value state is changed to negative when the value is invalid"); }); + it("F2 should switch the focus between the handle and the tooltip input", async () => { + await browser.url(`test/pages/Slider.html`); + + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + + await sliderHandle.click(); + await browser.keys("F2"); + + assert.strictEqual(await sliderTooltipInput.getProperty("focused"), true, "The focus is on the input"); + + await browser.keys("F2"); + + const isHandleFocused = await browser.executeAsync(done => { + const focusedElement = document.getElementById("slider-tickmarks-labels").shadowRoot.activeElement; + const isHandleFocused = focusedElement.classList.contains("ui5-slider-handle"); + done(isHandleFocused); + }); + + assert.strictEqual(isHandleFocused, true, "The focus is on the handle"); + }); + + it("Arrow up/down should not increase/decrease the value of the input", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + + await slider.setProperty("value", 1); + await sliderHandle.click(); + await sliderTooltipInput.click(); + + await browser.keys("ArrowUp"); + + assert.strictEqual(await slider.getProperty("value"), 1, "The value is not changed on arrow up"); + + await browser.keys("ArrowDown"); + + assert.strictEqual(await slider.getProperty("value"), 1, "The value is not changed on arrow down"); + }); + + it("Tab on slider handle should not move the focus to the tooltip input", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const nextSlider = await browser.$("#slider-tickmarks-tooltips-labels"); + + await sliderHandle.click(); + await browser.keys("Tab"); + + assert.strictEqual(await nextSlider.isFocused(), true, "The next component is focused and not the tooltip input"); + }); + + it("Focus out with invalid value should reset it", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const nextSlider = await browser.$("#slider-tickmarks-tooltips-labels"); + + await slider.setProperty("value", 10); + await sliderHandle.click(); + await sliderTooltipInput.click(); + await browser.keys(["1", "2", "3"]); + + await nextSlider.click(); + assert.strictEqual(await sliderTooltipInput.getProperty("value"), 10, "Value is reset to the last valid one"); + }); + it("Slider Tooltip should stay visible when slider is focused and mouse moves away", async () => { const slider = await browser.$("#basic-slider-with-tooltip"); const sliderTooltip = await slider.shadow$(".ui5-slider-tooltip"); From f7e291545cd358f7fbc7e6a2273b3de74811cbe2 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 23 Aug 2024 13:53:07 +0300 Subject: [PATCH 23/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip typo fix --- packages/main/test/specs/Slider.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index d4cfa9ec3b67..e282efef2913 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -293,7 +293,7 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { await browser.keys(["1", "2", "3"]); await nextSlider.click(); - assert.strictEqual(await sliderTooltipInput.getProperty("value"), 10, "Value is reset to the last valid one"); + assert.strictEqual(await sliderTooltipInput.getProperty("value"), "10", "Value is reset to the last valid one"); }); it("Slider Tooltip should stay visible when slider is focused and mouse moves away", async () => { From ecce66e43ad294b07213f1e04c4019ff2eeacb43 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sun, 25 Aug 2024 18:01:50 +0300 Subject: [PATCH 24/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip swapping input values - wip --- packages/main/src/RangeSlider.ts | 73 ++++++++++++++++++++++++++++---- packages/main/src/SliderBase.ts | 5 ++- 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index c2ed9b6fb1c4..15dca5a94b27 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -5,6 +5,7 @@ import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js"; import type { IFormInputElement } from "@ui5/webcomponents-base/dist/features/InputElementsFormSupport.js"; import { isEscape, + isEnter, isHome, isEnd, } from "@ui5/webcomponents-base/dist/Keys.js"; @@ -24,6 +25,7 @@ import { // Styles import rangeSliderStyles from "./generated/themes/RangeSlider.css.js"; +import Slider from "./Slider.js"; type AriaHandlesText = { startHandleText?: string, @@ -115,6 +117,12 @@ class RangeSlider extends SliderBase implements IFormInputElement { @property({ type: Number }) endValue = 100; + @property({ type: Number }) + _inputStartValue = 0; + + @property({ type: Number }) + _inputEndValue = 100; + @property({ type: Boolean }) rangePressed = false; @@ -133,6 +141,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { _reversedValues = false; _lastValidStartValue: string; _lastValidEndValue: string; + _inputValueUpdated = false; static i18nBundle: I18nBundle; @@ -208,10 +217,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { - if (this.editableTooltip) { - this._updateInputValue(); - } - if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -307,7 +312,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { _onInputFocusOut(e: FocusEvent) { const tooltipInput = e.target as Input; - this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + if (!this._inputValueUpdated) { + this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; + } + this._updateValueFromInput(e); this._updateInputValue(); @@ -317,6 +325,12 @@ class RangeSlider extends SliderBase implements IFormInputElement { tooltipInput.value = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this._lastValidStartValue : this._lastValidEndValue; tooltipInput.valueState = "None"; } + + if (tooltipInput.hasAttribute("data-sap-ui-start-value")) { + this._setAffectedValue("startValue"); + } + + this._setAffectedValue("endValue"); } /** @@ -325,9 +339,15 @@ class RangeSlider extends SliderBase implements IFormInputElement { * user interaction. * @private */ - _onkeyup() { - super._onkeyup(); - this._setAffectedValue(undefined); + _onkeyup(e: KeyboardEvent) { + super._onkeyup(e); + + if (!isEnter(e)) { + this._setAffectedValue(undefined); + } else { + this._inputValueUpdated = false; + } + if (this.startValue !== this._startValueAtBeginningOfAction || this.endValue !== this._endValueAtBeginningOfAction) { this.fireEvent("change"); @@ -571,6 +591,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { } _updateValueFromInput(e: Event) { + if (this._inputValueUpdated) { + return; + } + const input = e.target as HTMLInputElement; const inputValue = parseFloat(input.value); const isValueValid = inputValue >= this._effectiveMin && inputValue <= this._effectiveMax; @@ -585,6 +609,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { } this.endValue = inputValue; + + if (this.startValue > this.endValue) { + this._inputValueUpdated = true; + } } /** @@ -796,6 +824,23 @@ class RangeSlider extends SliderBase implements IFormInputElement { } } + _onInputKeydown(e: KeyboardEvent): void { + const input = e.target as Input; + + super._onInputKeydown(e); + const affectedValue = input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"; + const otherInput: Input = input.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; + + if (isEnter(e)) { + this._setAffectedValue(affectedValue); + + if (this.startValue > this.endValue) { + this._inputValueUpdated = true; + otherInput.focus(); + } + } + } + _updateInputValue() { const startValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]") as Input; const endValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]") as Input; @@ -804,6 +849,9 @@ class RangeSlider extends SliderBase implements IFormInputElement { return; } + this._inputStartValue = parseFloat(startValueInput.value); + this._inputEndValue = parseFloat(endValueInput.value); + const isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; @@ -822,6 +870,9 @@ class RangeSlider extends SliderBase implements IFormInputElement { startValueInput.valueState = "None"; endValueInput.valueState = "None"; + + this.storePropertyState("_inputStartValue"); + this.storePropertyState("_inputEndValue"); } /** @@ -855,7 +906,11 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._setValuesAreReversed(); this._updateHandlesAndRange(this[affectedValue]); - this.focusInnerElement(); + + if (!this._inputValueUpdated) { + this.focusInnerElement(); + } + this.syncUIAndState(); } diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 295416dafa5e..7179de298e40 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -311,6 +311,7 @@ abstract class SliderBase extends UI5Element { this._handleActionKeyPress(e); } } + _onInputKeydown(e: KeyboardEvent) { const target = e.target as HTMLElement; @@ -319,12 +320,12 @@ abstract class SliderBase extends UI5Element { } if (isEnter(e)) { - this._updateValueFromInput(e); this._updateInputValue(); + this._updateValueFromInput(e); } } - _onkeyup() { + _onkeyup(e: KeyboardEvent) { if (this.disabled) { return; } From ba84adcfd731d4bd2a7ae39ec59e58d74d961a25 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sun, 25 Aug 2024 18:34:21 +0300 Subject: [PATCH 25/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix input swapping bugs --- packages/main/src/RangeSlider.ts | 46 +++++++++++++++----------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 15dca5a94b27..81e85fff1973 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -117,12 +117,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { @property({ type: Number }) endValue = 100; - @property({ type: Number }) - _inputStartValue = 0; - - @property({ type: Number }) - _inputEndValue = 100; - @property({ type: Boolean }) rangePressed = false; @@ -141,7 +135,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { _reversedValues = false; _lastValidStartValue: string; _lastValidEndValue: string; - _inputValueUpdated = false; + _areInputValuesSwapped = false; static i18nBundle: I18nBundle; @@ -311,8 +305,20 @@ class RangeSlider extends SliderBase implements IFormInputElement { _onInputFocusOut(e: FocusEvent) { const tooltipInput = e.target as Input; + const otherInput: Input = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; + + if (this.startValue > this.endValue) { + this._areInputValuesSwapped = true; + otherInput.focus(); + } - if (!this._inputValueUpdated) { + if (tooltipInput.hasAttribute("data-sap-ui-start-value")) { + this._setAffectedValue("startValue"); + } else { + this._setAffectedValue("endValue"); + } + + if (!this._areInputValuesSwapped) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; } @@ -326,11 +332,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { tooltipInput.valueState = "None"; } - if (tooltipInput.hasAttribute("data-sap-ui-start-value")) { - this._setAffectedValue("startValue"); - } - - this._setAffectedValue("endValue"); + this.update(this._valueAffected, parseFloat(this._lastValidStartValue), parseFloat(this._lastValidEndValue)); } /** @@ -345,7 +347,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { if (!isEnter(e)) { this._setAffectedValue(undefined); } else { - this._inputValueUpdated = false; + this._areInputValuesSwapped = false; } @@ -591,7 +593,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { } _updateValueFromInput(e: Event) { - if (this._inputValueUpdated) { + if (this._areInputValuesSwapped) { return; } @@ -611,7 +613,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { this.endValue = inputValue; if (this.startValue > this.endValue) { - this._inputValueUpdated = true; + this._areInputValuesSwapped = true; } } @@ -831,11 +833,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { const affectedValue = input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"; const otherInput: Input = input.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; - if (isEnter(e)) { - this._setAffectedValue(affectedValue); - + if (isEnter(e)) { if (this.startValue > this.endValue) { - this._inputValueUpdated = true; + this._areInputValuesSwapped = true; + this._setAffectedValue(affectedValue); otherInput.focus(); } } @@ -849,9 +850,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { return; } - this._inputStartValue = parseFloat(startValueInput.value); - this._inputEndValue = parseFloat(endValueInput.value); - const isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; @@ -907,7 +905,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._setValuesAreReversed(); this._updateHandlesAndRange(this[affectedValue]); - if (!this._inputValueUpdated) { + if (!this._areInputValuesSwapped) { this.focusInnerElement(); } From 4da66e6fd27a624ba03b0640fc5464df66e8696c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sun, 25 Aug 2024 18:58:04 +0300 Subject: [PATCH 26/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix code review comments --- packages/main/src/RangeSlider.hbs | 4 ++-- packages/main/src/RangeSlider.ts | 14 ++++++++++---- packages/main/src/Slider.hbs | 6 +++--- packages/main/src/Slider.ts | 15 +++++++++------ packages/main/src/SliderBase.ts | 12 ------------ 5 files changed, 24 insertions(+), 27 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index ab72cab4ee5b..4e33e0b4ce4e 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -53,7 +53,7 @@ = this.min && parseFloat(startValueInput.value) <= this.max; - const isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; + this._isStartValueValid = parseFloat(startValueInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; + this._isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; - if (!isStartValueValid) { + if (!this._isStartValueValid) { startValueInput.valueState = "Negative"; return; } - if (!isEndValueValid) { + if (!this._isEndValueValid) { endValueInput.valueState = "Negative"; return; } diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 6a68623565f9..d40e66774878 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -27,10 +27,10 @@ aria-valuemin="{{min}}" aria-valuemax="{{max}}" aria-valuenow="{{value}}" - aria-labelledby="{{_ariaLabelledByHandleRefs}}" + aria-labelledby="ui5-slider-accName ui5-sliderDesc" aria-disabled="{{_ariaDisabled}}" aria-keyshortcuts="F2" - aria-describedby="{{_ariaDescribedByHandleRefs}}" + aria-describedby="ui5-slider-accName ui5-slider-sliderInputDesc" data-sap-focus-ref part="handle" @@ -43,7 +43,7 @@ = this._effectiveMin && value <= this._effectiveMax; + this._isInputValueValid = value >= this._effectiveMin && value <= this._effectiveMax; - if (!isValueValid) { + if (!this._isInputValueValid) { return; } @@ -281,9 +284,9 @@ class Slider extends SliderBase implements IFormInputElement { this._updateValueFromInput(e); this._updateInputValue(); - const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; - if (!isTooltipInputValueValid) { + if (!this._isInputValueValid ) { tooltipInput.value = this._lastValidInputValue; tooltipInput.valueState = "None"; } @@ -296,9 +299,9 @@ class Slider extends SliderBase implements IFormInputElement { return; } - const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; - if (!isTooltipInputValueValid) { + if (!this._isInputValueValid) { tooltipInput.valueState = "Negative"; return; } diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 7179de298e40..cff8fc2143fa 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -758,18 +758,6 @@ abstract class SliderBase extends UI5Element { return this.disabled ? "-1" : "0"; } - get _ariaLabelledByHandleRefs() { - return ["ui5-slider-accName", "ui5-sliderDesc"].join(" ").trim(); - } - - get _ariaDescribedByHandleRefs() { - return ["ui5-slider-accName", `ui5-slider-sliderInputDesc`].join(" ").trim(); - } - - get _ariaLabelledByInputRefs() { - return ["ui5-slider-accName", "ui5-slider-InputLabel"].join(" ").trim(); - } - get _ariaDescribedByInputText() { return ""; } From 78b74eae555907dbc51dec4ba86f2ac72480b92c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 26 Aug 2024 13:04:24 +0300 Subject: [PATCH 27/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix bugs --- packages/main/src/RangeSlider.ts | 28 ++++++++++++++++++++-------- packages/main/src/Slider.ts | 6 +++--- packages/main/src/SliderBase.ts | 2 +- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index c092bd420a22..7e55c584ea09 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -25,7 +25,6 @@ import { // Styles import rangeSliderStyles from "./generated/themes/RangeSlider.css.js"; -import Slider from "./Slider.js"; type AriaHandlesText = { startHandleText?: string, @@ -217,6 +216,15 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { + const isStartValueValid = this.startValue >= this.min && this.startValue <= this.max; + const isEndValueValid = this.endValue >= this.min && this.endValue <= this.max; + + this._lastValidStartValue = isStartValueValid ? this.startValue.toString() : this._lastValidStartValue; + this._lastValidEndValue = isEndValueValid ? this.endValue.toString() : this._lastValidEndValue; + + this.startValue = isStartValueValid ? this.startValue : parseFloat(this._lastValidStartValue); + this.endValue = isEndValueValid ? this.endValue : parseFloat(this._lastValidEndValue); + if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -312,7 +320,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { _onInputFocusOut(e: FocusEvent) { const tooltipInput = e.target as Input; const otherInput: Input = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; - + const relatedTarget = e.relatedTarget as HTMLElement; + if (this.startValue > this.endValue) { this._areInputValuesSwapped = true; otherInput.focus(); @@ -324,12 +333,13 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._setAffectedValue("endValue"); } - if (!this._areInputValuesSwapped) { + if (!this._areInputValuesSwapped || !this.shadowRoot!.contains(relatedTarget)) { this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; } this._updateValueFromInput(e); this._updateInputValue(); + this.update(this._valueAffected, parseFloat(this._lastValidStartValue), parseFloat(this._lastValidEndValue)); const isTooltipInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; @@ -337,8 +347,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { tooltipInput.value = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this._lastValidStartValue : this._lastValidEndValue; tooltipInput.valueState = "None"; } - - this.update(this._valueAffected, parseFloat(this._lastValidStartValue), parseFloat(this._lastValidEndValue)); } /** @@ -347,6 +355,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { * user interaction. * @private */ + _onkeyup(e: KeyboardEvent) { super._onkeyup(e); @@ -356,7 +365,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._areInputValuesSwapped = false; } - if (this.startValue !== this._startValueAtBeginningOfAction || this.endValue !== this._endValueAtBeginningOfAction) { this.fireEvent("change"); } @@ -717,6 +725,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { * @protected */ focusInnerElement() { + if (this.editableTooltip && this._tooltipVisibility === SliderBase.TOOLTIP_VISIBILITY.HIDDEN) { + return; + } + const isReversed = this._areValuesReversed(); const affectedValue = this._valueAffected; @@ -839,7 +851,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { const affectedValue = input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"; const otherInput: Input = input.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; - if (isEnter(e)) { + if (isEnter(e)) { if (this.startValue > this.endValue) { this._areInputValuesSwapped = true; this._setAffectedValue(affectedValue); @@ -914,7 +926,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { if (!this._areInputValuesSwapped) { this.focusInnerElement(); } - + this.syncUIAndState(); } diff --git a/packages/main/src/Slider.ts b/packages/main/src/Slider.ts index 46eb6b3b5556..4839c38b70b8 100644 --- a/packages/main/src/Slider.ts +++ b/packages/main/src/Slider.ts @@ -268,7 +268,7 @@ class Slider extends SliderBase implements IFormInputElement { _updateValueFromInput(e: Event) { const input = e.target as HTMLInputElement; const value = parseFloat(input.value); - this._isInputValueValid = value >= this._effectiveMin && value <= this._effectiveMax; + this._isInputValueValid = value >= this._effectiveMin && value <= this._effectiveMax; if (!this._isInputValueValid) { return; @@ -286,7 +286,7 @@ class Slider extends SliderBase implements IFormInputElement { this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; - if (!this._isInputValueValid ) { + if (!this._isInputValueValid) { tooltipInput.value = this._lastValidInputValue; tooltipInput.valueState = "None"; } @@ -299,7 +299,7 @@ class Slider extends SliderBase implements IFormInputElement { return; } - this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; + this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; if (!this._isInputValueValid) { tooltipInput.valueState = "Negative"; diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index cff8fc2143fa..9df388bcf199 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -325,7 +325,7 @@ abstract class SliderBase extends UI5Element { } } - _onkeyup(e: KeyboardEvent) { + _onkeyup(e: KeyboardEvent) { // eslint-disable-line if (this.disabled) { return; } From ca456597ca81c8de0b9be1c25e160c577033338f Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 26 Aug 2024 18:20:20 +0300 Subject: [PATCH 28/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix input value normalization bug --- packages/main/src/RangeSlider.ts | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 7e55c584ea09..11f2ef472e4d 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -216,14 +216,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { - const isStartValueValid = this.startValue >= this.min && this.startValue <= this.max; - const isEndValueValid = this.endValue >= this.min && this.endValue <= this.max; - - this._lastValidStartValue = isStartValueValid ? this.startValue.toString() : this._lastValidStartValue; - this._lastValidEndValue = isEndValueValid ? this.endValue.toString() : this._lastValidEndValue; - - this.startValue = isStartValueValid ? this.startValue : parseFloat(this._lastValidStartValue); - this.endValue = isEndValueValid ? this.endValue : parseFloat(this._lastValidEndValue); + this._saveInputValues(); if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -355,7 +348,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { * user interaction. * @private */ - _onkeyup(e: KeyboardEvent) { super._onkeyup(e); @@ -891,6 +883,25 @@ class RangeSlider extends SliderBase implements IFormInputElement { this.storePropertyState("_inputEndValue"); } + _saveInputValues() { + const startValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]") as Input; + const endValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]") as Input; + + if (this.editableTooltip && startValueInput && endValueInput) { + const inputStartValue = parseFloat(startValueInput.value); + const inputEndValue = parseFloat(endValueInput.value); + + const isStartValueValid = inputStartValue >= this.min && inputStartValue <= this.max; + const isEndValueValid = inputEndValue >= this.min && inputEndValue <= this.max; + + this._lastValidStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; + this._lastValidEndValue = isEndValueValid ? inputStartValue.toString() : this._lastValidEndValue; + + startValueInput.value = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; + endValueInput.value = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; + } + } + /** * Swaps the start and end values of the handles if one came accros the other: * - If the start value is greater than the endValue swap them and their handles From bb5df8757471bf47d66161c2db482700fdd8262b Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 26 Aug 2024 19:03:13 +0300 Subject: [PATCH 29/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip add input value swapping tests --- packages/main/test/specs/RangeSlider.spec.js | 45 +++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 41b51ad3bbb1..f8e8e4fc3f9b 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -129,8 +129,6 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { const rangeSliderStartTooltipValue = await rangeSliderStartTooltip.shadow$(".ui5-slider-tooltip-value"); const rangeSliderEndTooltip = await rangeSlider.shadow$(".ui5-slider-tooltip--end"); const rangeSliderEndTooltipValue = await rangeSliderEndTooltip.shadow$(".ui5-slider-tooltip-value"); - const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); - const endHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); await rangeSlider.moveTo(); @@ -297,6 +295,49 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "2", "Value is reset to the last valid one"); }); + it("Input values should be swapped if the start value is bigger than the end value", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const rangeSliderEndHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + + const rangeSliderEndTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + + await rangeSlider.setProperty("startValue", 0); + await rangeSlider.setProperty("endValue", 1); + + await rangeSliderStartHandle.click(); + await rangeSliderStartTooltipInput.click(); + await browser.keys("2"); + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("endValue"), 20, "The start value is now end value"); + assert.strictEqual(await rangeSliderEndTooltipInput.getProperty("value"), "20", "The start input value is now end value"); + }); + + + it("Input values should be swapped if the end value is lower than the start value", async () => { + await browser.url(`test/pages/RangeSlider.html`); + + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderEndHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + + const rangeSliderEndTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + + await rangeSlider.setProperty("startValue", 2); + await rangeSlider.setProperty("endValue", 3); + + await rangeSliderEndHandle.click(); + await rangeSliderEndTooltipInput.click(); + await browser.keys("Delete"); + await browser.keys("1"); + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The end value is now start value"); + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "1", "The end input value is now start value"); + }); + it("Range Slider tooltips should become visible when range slider is focused", async () => { await browser.url(`test/pages/RangeSlider.html`); From 049b92204e9ea1173c9d9a1af15d58c6612f76d8 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 26 Aug 2024 19:12:04 +0300 Subject: [PATCH 30/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip lint --- packages/main/src/RangeSlider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 11f2ef472e4d..60ea6cfe6ed4 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -893,10 +893,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { const isStartValueValid = inputStartValue >= this.min && inputStartValue <= this.max; const isEndValueValid = inputEndValue >= this.min && inputEndValue <= this.max; - + this._lastValidStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; this._lastValidEndValue = isEndValueValid ? inputStartValue.toString() : this._lastValidEndValue; - + startValueInput.value = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; endValueInput.value = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; } From 37f145f6c8098ceb7e33f229b6c9fe5962cae524 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 10 Sep 2024 12:00:25 +0300 Subject: [PATCH 31/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix code review comments (wip) --- packages/main/src/RangeSlider.hbs | 6 ++++-- packages/main/src/RangeSlider.ts | 29 ++++++++++++++----------- packages/main/src/Slider.hbs | 3 ++- packages/main/src/Slider.ts | 35 +++++++++++-------------------- packages/main/src/SliderBase.hbs | 2 +- packages/main/src/SliderBase.ts | 27 ++++++++++++++++++++---- 6 files changed, 59 insertions(+), 43 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 4e33e0b4ce4e..013abc587151 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -52,7 +52,8 @@ {{#if editableTooltip}} this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -349,7 +357,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { * @private */ _onkeyup(e: KeyboardEvent) { - super._onkeyup(e); + super._onKeyupBase(); if (!isEnter(e)) { this._setAffectedValue(undefined); @@ -864,23 +872,20 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; if (!this._isStartValueValid) { - startValueInput.valueState = "Negative"; + this._tooltipInputStartValueState = "Negative"; return; } if (!this._isEndValueValid) { - endValueInput.valueState = "Negative"; + this._tooltipInputEndValueState = "Negative"; return; } this._lastValidStartValue = startValueInput.value; this._lastValidEndValue = endValueInput.value; - startValueInput.valueState = "None"; - endValueInput.valueState = "None"; - - this.storePropertyState("_inputStartValue"); - this.storePropertyState("_inputEndValue"); + this._tooltipInputStartValueState = "None"; + this._tooltipInputEndValueState = "None"; } _saveInputValues() { @@ -897,8 +902,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._lastValidStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; this._lastValidEndValue = isEndValueValid ? inputStartValue.toString() : this._lastValidEndValue; - startValueInput.value = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; - endValueInput.value = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; + this._tooltipInputStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; + this._tooltipInputEndValue = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; } } diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index d40e66774878..bd3f955700c5 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -41,7 +41,8 @@
{{#if editableTooltip}} = this._effectiveMin && value <= this._effectiveMax; - - if (!this._isInputValueValid) { - return; - } - - this.value = value; - } - _onInputFocusOut(e: FocusEvent) { - const tooltipInput = e.target as Input; + const tooltipInput = this.shadowRoot!.querySelector("ui5-input") as Input; this._tooltipVisibility = SliderBase.TOOLTIP_VISIBILITY.HIDDEN; this._updateValueFromInput(e); - this._updateInputValue(); - - this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; if (!this._isInputValueValid) { tooltipInput.value = this._lastValidInputValue; - tooltipInput.valueState = "None"; + this._isInputValueValid = true; + this._tooltipInputValueState = "None"; } } @@ -302,12 +287,16 @@ class Slider extends SliderBase implements IFormInputElement { this._isInputValueValid = parseFloat(tooltipInput.value) >= this.min && parseFloat(tooltipInput.value) <= this.max; if (!this._isInputValueValid) { - tooltipInput.valueState = "Negative"; + this._tooltipInputValue = this._lastValidInputValue; + this._isInputValueValid = true; + this._tooltipInputValueState = "Negative"; + return; } - this._lastValidInputValue = tooltipInput.value; - tooltipInput.valueState = "None"; + this._tooltipInputValue = this.value.toString(); + this._lastValidInputValue = this._tooltipInputValue; + this._tooltipInputValueState = "None"; } /** Determines if the press is over the handle diff --git a/packages/main/src/SliderBase.hbs b/packages/main/src/SliderBase.hbs index 8f3b5b4e1405..d12a39bda045 100644 --- a/packages/main/src/SliderBase.hbs +++ b/packages/main/src/SliderBase.hbs @@ -5,7 +5,7 @@ @mouseover="{{_onmouseover}}" @mouseout="{{_onmouseout}}" @keydown="{{_onkeydown}}" - @keyup="{{_onkeyup}}" + @keyup="{{_onKeyupBase}}" part="root-container" > {{> handlesAriaText}} diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 9df388bcf199..c269544c6067 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -134,6 +134,12 @@ abstract class SliderBase extends UI5Element { @property() accessibleName?: string; + /** + * @private + */ + @property({ type: Number }) + value = 0; + /** * @private */ @@ -146,6 +152,9 @@ abstract class SliderBase extends UI5Element { @property({ type: Boolean }) _hiddenTickmarks = false; + @property({ type: Boolean }) + _isInputValueValid = false; + _resizeHandler: ResizeObserverCallback; _moveHandler: (e: TouchEvent | MouseEvent) => void; _upHandler: (e: TouchEvent | MouseEvent) => void; @@ -193,11 +202,9 @@ abstract class SliderBase extends UI5Element { _onmousedown(e: TouchEvent | MouseEvent) {} // eslint-disable-line - _updateValueFromInput(e: KeyboardEvent | FocusEvent) {} // eslint-disable-line - _handleActionKeyPress(e: Event) {} // eslint-disable-line - _updateInputValue() {} // eslint-disable-line + _updateInputValue() {} // used in base template, but implemented in subclasses abstract styles: { @@ -325,7 +332,19 @@ abstract class SliderBase extends UI5Element { } } - _onkeyup(e: KeyboardEvent) { // eslint-disable-line + _updateValueFromInput(e: Event) { + const input = e.target as HTMLInputElement; + const value = parseFloat(input.value); + this._isInputValueValid = value >= this._effectiveMin && value <= this._effectiveMax; + + if (!this._isInputValueValid) { + return; + } + + this.value = value; + } + + _onKeyupBase() { if (this.disabled) { return; } From 715da42cc492b2045b676e56849cc719a3950a95 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 10 Sep 2024 12:42:17 +0300 Subject: [PATCH 32/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip revert ui5-range-slider state changes --- packages/main/src/RangeSlider.hbs | 6 ++---- packages/main/src/RangeSlider.ts | 20 +++++++------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 013abc587151..4e33e0b4ce4e 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -52,8 +52,7 @@ {{#if editableTooltip}} this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -872,20 +866,20 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; if (!this._isStartValueValid) { - this._tooltipInputStartValueState = "Negative"; + startValueInput.valueState = "Negative"; return; } if (!this._isEndValueValid) { - this._tooltipInputEndValueState = "Negative"; + endValueInput.valueState = "Negative"; return; } this._lastValidStartValue = startValueInput.value; this._lastValidEndValue = endValueInput.value; - this._tooltipInputStartValueState = "None"; - this._tooltipInputEndValueState = "None"; + startValueInput.valueState = "None"; + endValueInput.valueState = "None"; } _saveInputValues() { @@ -902,8 +896,8 @@ class RangeSlider extends SliderBase implements IFormInputElement { this._lastValidStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; this._lastValidEndValue = isEndValueValid ? inputStartValue.toString() : this._lastValidEndValue; - this._tooltipInputStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; - this._tooltipInputEndValue = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; + startValueInput.value = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; + endValueInput.value = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; } } From 5735d0ced3007e8b53bc657e5e34c8aaae6e33d7 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 13 Sep 2024 09:25:08 +0300 Subject: [PATCH 33/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix the cntrl+arrow key navigation decimal value bug --- packages/main/src/RangeSlider.ts | 35 ++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index cc4fc743c3f5..a1141a3386c3 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -893,14 +893,41 @@ class RangeSlider extends SliderBase implements IFormInputElement { const isStartValueValid = inputStartValue >= this.min && inputStartValue <= this.max; const isEndValueValid = inputEndValue >= this.min && inputEndValue <= this.max; - this._lastValidStartValue = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; - this._lastValidEndValue = isEndValueValid ? inputStartValue.toString() : this._lastValidEndValue; + if (this._isUserInteraction) { + startValueInput.value = isStartValueValid ? this._getFormattedValue(this.startValue.toString()) : this._getFormattedValue(this._lastValidStartValue); + endValueInput.value = isEndValueValid ? this._getFormattedValue(this.endValue.toString()) : this._getFormattedValue(this._lastValidEndValue); - startValueInput.value = isStartValueValid ? inputStartValue.toString() : this._lastValidStartValue; - endValueInput.value = isEndValueValid ? inputEndValue.toString() : this._lastValidEndValue; + this.startValue = parseFloat(this._getFormattedValue(this.startValue.toString())); + this.endValue = parseFloat(this._getFormattedValue(this.endValue.toString())); + + if (this.startValue > this.endValue) { + this._areInputValuesSwapped = true; + this._setValuesAreReversed(); + } + + this.syncUIAndState(); + this._updateHandlesAndRange(0); + this.update(this._valueAffected, this.startValue, this.endValue); + + return; + } + + this._lastValidStartValue = isStartValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidStartValue); + this._lastValidEndValue = isEndValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidEndValue); + + startValueInput.value = isStartValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidStartValue); + endValueInput.value = isEndValueValid ? this._getFormattedValue(inputEndValue.toString()) : this._getFormattedValue(this._lastValidEndValue); } } + _getFormattedValue(value: string) { + const valueNumber = parseFloat(value); + const ctor = this.constructor as typeof RangeSlider; + const stepPrecision = ctor._getDecimalPrecisionOfNumber(this._effectiveStep); + + return valueNumber.toFixed(stepPrecision).toString(); + } + /** * Swaps the start and end values of the handles if one came accros the other: * - If the start value is greater than the endValue swap them and their handles From 3b34a23f1e57b9ec3897136338a8421d73a419ce Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 13 Sep 2024 09:56:11 +0300 Subject: [PATCH 34/51] feat(ui5-slider, ui5-range-slider): fix aria-describedby attribute for handles fix aria-describedby attribute for handles --- packages/main/src/RangeSlider.hbs | 2 ++ packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 4e33e0b4ce4e..5889e5812485 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -43,6 +43,7 @@ aria-valuenow="{{startValue}}" aria-labelledby="{{_ariaLabelledByStartHandleRefs}}" aria-disabled="{{_ariaDisabled}}" + aria-describedby="{{_ariaDescribedByHandleText}}" >
@@ -77,6 +78,7 @@ aria-valuemax="{{max}}" aria-valuenow="{{endValue}}" aria-labelledby="{{_ariaLabelledByEndHandleRefs}}" + aria-describedby="{{_ariaDescribedByHandleText}}" aria-disabled="{{_ariaDisabled}}" > diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index bd3f955700c5..0e4f27692276 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -30,7 +30,7 @@ aria-labelledby="ui5-slider-accName ui5-sliderDesc" aria-disabled="{{_ariaDisabled}}" aria-keyshortcuts="F2" - aria-describedby="ui5-slider-accName ui5-slider-sliderInputDesc" + aria-describedby="{{_ariaDescribedByHandleText}}" data-sap-focus-ref part="handle" diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index c269544c6067..e2d50131aef6 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -777,6 +777,10 @@ abstract class SliderBase extends UI5Element { return this.disabled ? "-1" : "0"; } + get _ariaDescribedByHandleText { + return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : "ui5-slider-accName"; + } + get _ariaDescribedByInputText() { return ""; } From 11e343e673376a6ebac8f63c0ebceb266b251aad Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Fri, 13 Sep 2024 11:20:57 +0300 Subject: [PATCH 35/51] feat(ui5-slider, ui5-range-slider): fix property getter typo fix property getter typo --- packages/main/src/SliderBase.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index e2d50131aef6..bb16b3a002b2 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -777,7 +777,7 @@ abstract class SliderBase extends UI5Element { return this.disabled ? "-1" : "0"; } - get _ariaDescribedByHandleText { + get _ariaDescribedByHandleText() { return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : "ui5-slider-accName"; } From 96afa78ceb4884704f0dd7ac63b9aba42c18304f Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sat, 14 Sep 2024 20:09:01 +0300 Subject: [PATCH 36/51] feat(ui5-slider, ui5-range-slider): add event firing add event firing --- packages/main/src/RangeSlider.hbs | 4 ++++ packages/main/src/Slider.hbs | 2 ++ packages/main/src/SliderBase.ts | 11 +++++++++++ packages/main/test/pages/RangeSlider.html | 9 +++++++++ packages/main/test/pages/Slider.html | 11 ++++++++++- packages/main/test/specs/RangeSlider.spec.js | 20 ++++++++++++++++++++ packages/main/test/specs/Slider.spec.js | 19 +++++++++++++++++++ 7 files changed, 75 insertions(+), 1 deletion(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 5889e5812485..e63475dc2c1b 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -57,6 +57,8 @@ accessible-name-ref="ui5-slider-accName ui5-slider-InputLabel" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" + @ui5-change="{{_onInputChange}}" + @ui5-input="{{_onInputInput}}" data-sap-ui-start-value tabindex="-1" > @@ -93,6 +95,8 @@ accessible-name-ref="ui5-slider-accName ui5-slider-InputLabel" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" + @ui5-change="{{_onInputChange}}" + @ui5-input="{{_onInputInput}}" data-sap-ui-end-value tabindex="-1" > diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 0e4f27692276..c48565c31b1b 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -47,6 +47,8 @@ accessible-name-ref="ui5-slider-accName ui5-slider-InputLabel" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" + @ui5-change="{{_onInputChange}}" + @ui5-input="{{_onInputInput}}" data-sap-ui-end-value tabindex="-1" > diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index bb16b3a002b2..d41a505b8367 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -168,6 +168,7 @@ abstract class SliderBase extends UI5Element { _oldMax?: number; _labelWidth = 0; _labelValues?: Array; + _valueOnInteractionStart?: number; async formElementAnchor() { return this.getFocusDomRefAsync(); @@ -332,6 +333,16 @@ abstract class SliderBase extends UI5Element { } } + _onInputChange() { + if (this._valueOnInteractionStart !== this.value) { + this.fireEvent("change"); + } + } + + _onInputInput() { + this.fireEvent("input"); + } + _updateValueFromInput(e: Event) { const input = e.target as HTMLInputElement; const value = parseFloat(input.value); diff --git a/packages/main/test/pages/RangeSlider.html b/packages/main/test/pages/RangeSlider.html index 24a3da0ae5bf..0e26d0e4772e 100644 --- a/packages/main/test/pages/RangeSlider.html +++ b/packages/main/test/pages/RangeSlider.html @@ -71,6 +71,7 @@

Event Testing Result Slider

diff --git a/packages/main/test/pages/Slider.html b/packages/main/test/pages/Slider.html index d3f51a9240ea..caa84a7e9042 100644 --- a/packages/main/test/pages/Slider.html +++ b/packages/main/test/pages/Slider.html @@ -44,7 +44,7 @@

Slider with many steps and small width

Inactive slider with no steps and tooltip

-

Slider with steps, tooltips, tickmarks and labels

+

Slider with steps, input tooltips, tickmarks and labels

Slider with float number step (1.25), tooltips, tickmarks and labels between 3 steps (3.75 value)

@@ -64,6 +64,7 @@

Event Testing Result Slider

diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index f8e8e4fc3f9b..8a2759e6c6dc 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -190,6 +190,23 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSlider.getProperty("startValue"), 8, "The input value is reflected in the slider"); }); + it("Input tooltip value change should fire change event", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); + const rangeSliderHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); + const eventResultSlider = await browser.$("#test-result-slider"); + + await rangeSlider.setProperty("startValue", 1); + + await rangeSliderHandle.click(); + await rangeSliderStartTooltipInput.click(); + await eventResultSlider.setProperty("endValue", 2); + await browser.keys("2"); + await browser.keys("Enter"); + + assert.strictEqual(await eventResultSlider.getProperty("endValue") , 4, "'change' and 'input' events are fired"); + }); + it("Input tooltips value state should change to 'Negative' if value is invalid", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); @@ -471,6 +488,9 @@ describe("Testing events", () => { const rangeSlider = await browser.$("#test-slider"); const eventResultRangeSlider = await browser.$("#test-result-slider"); + await eventResultRangeSlider.setProperty("startValue", 1); + await eventResultRangeSlider.setProperty("endValue", 2); + await rangeSlider.click(); assert.strictEqual(await eventResultRangeSlider.getProperty("endValue") , 4, "Both input event and change event are fired after user interaction"); diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index e282efef2913..319bf0c7f9c9 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -213,6 +213,24 @@ describe("Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await slider.getProperty("value"), 21, "The input value is reflected in the slider"); }); + it("Input tooltip value change should fire change event", async () => { + const slider = await browser.$("#slider-tickmarks-labels"); + const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); + const sliderHandle = await slider.shadow$(".ui5-slider-handle"); + const eventResultSlider = await browser.$("#test-result-slider"); + + await slider.setProperty("value", 1); + await eventResultSlider.setProperty("value", 1); + + await sliderHandle.click(); + await sliderTooltipInput.click(); + await browser.keys(["2"]); + await browser.keys("Enter"); + + assert.strictEqual(await slider.getProperty("value"), 21, "The input value is reflected in the slider"); + assert.strictEqual(await eventResultSlider.getProperty("value") , 3, "'input' and 'change' events are fired on input 'change' and 'input' events"); + }); + it("Input tooltip should change the value state to error if it is invalid", async () => { const slider = await browser.$("#slider-tickmarks-labels"); const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); @@ -369,6 +387,7 @@ describe("Testing events", () => { const slider = await browser.$("#test-slider"); const eventResultSlider = await browser.$("#test-result-slider"); + await eventResultSlider.setProperty("value", 1); await slider.click(); assert.strictEqual(await eventResultSlider.getProperty("value") , 3, "Both input event and change event are fired after user interaction"); From 125bf065745d196bfbb16d1b9a7ec372a4bcd41a Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sun, 15 Sep 2024 20:43:10 +0300 Subject: [PATCH 37/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix value swapping bug --- packages/main/src/RangeSlider.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index a1141a3386c3..5e5a13b91c46 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -216,10 +216,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { * */ onBeforeRendering() { - if (this.editableTooltip) { - this._saveInputValues(); - } - if (this.startValue > this.endValue) { const affectedValue = this._valueAffected === "startValue" ? "endValue" : "startValue"; @@ -228,6 +224,10 @@ class RangeSlider extends SliderBase implements IFormInputElement { this.update(affectedValue, this.startValue, this.endValue); } + if (this.editableTooltip) { + this._saveInputValues(); + } + if (!this.isCurrentStateOutdated()) { return; } @@ -900,15 +900,9 @@ class RangeSlider extends SliderBase implements IFormInputElement { this.startValue = parseFloat(this._getFormattedValue(this.startValue.toString())); this.endValue = parseFloat(this._getFormattedValue(this.endValue.toString())); - if (this.startValue > this.endValue) { - this._areInputValuesSwapped = true; - this._setValuesAreReversed(); - } - this.syncUIAndState(); this._updateHandlesAndRange(0); this.update(this._valueAffected, this.startValue, this.endValue); - return; } From 65c7588a7fbf63ddca638e5922369836e22c7c0b Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Sun, 15 Sep 2024 20:56:06 +0300 Subject: [PATCH 38/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip remove duplicated translateble texts --- packages/main/src/RangeSlider.ts | 8 ++++---- packages/main/src/i18n/messagebundle.properties | 6 ------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 5e5a13b91c46..a04d158b5dee 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -19,8 +19,8 @@ import { RANGE_SLIDER_ARIA_DESCRIPTION, RANGE_SLIDER_START_HANDLE_DESCRIPTION, RANGE_SLIDER_END_HANDLE_DESCRIPTION, - RANGE_SLIDER_TOOLTIP_INPUT_LABEL, - RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION, + SLIDER_TOOLTIP_INPUT_LABEL, + SLIDER_TOOLTIP_INPUT_DESCRIPTION, } from "./generated/i18n/i18n-defaults.js"; // Styles @@ -1019,11 +1019,11 @@ class RangeSlider extends SliderBase implements IFormInputElement { } get _ariaLabelledByInputText() { - return RangeSlider.i18nBundle.getText(RANGE_SLIDER_TOOLTIP_INPUT_LABEL); + return RangeSlider.i18nBundle.getText(SLIDER_TOOLTIP_INPUT_LABEL); } get _ariaDescribedByInputText() { - return RangeSlider.i18nBundle.getText(RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION); + return RangeSlider.i18nBundle.getText(SLIDER_TOOLTIP_INPUT_DESCRIPTION); } get styles() { diff --git a/packages/main/src/i18n/messagebundle.properties b/packages/main/src/i18n/messagebundle.properties index 23667275d210..3cf58a24c308 100644 --- a/packages/main/src/i18n/messagebundle.properties +++ b/packages/main/src/i18n/messagebundle.properties @@ -504,12 +504,6 @@ SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value #XACT: ARIA label for slider tooltip input SLIDER_TOOLTIP_INPUT_LABEL = Current value -#XACT: ARIA label for range slider tooltip input -RANGE_SLIDER_TOOLTIP_INPUT_LABEL = Current value - -#XACT: ARIA description for range slider tooltip input -RANGE_SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value - #XTOL: tooltip for decrease button of the StepInput STEPINPUT_DEC_ICON_TITLE=Decrease From a39435cc0647a5f59fb4bf5794c5b479f63dbdf5 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 16 Sep 2024 08:40:18 +0300 Subject: [PATCH 39/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix aria-labelledby and aria-describedby, restore the icon mode property --- packages/main/src/RangeSlider.hbs | 16 ++++++++-------- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 2 +- packages/main/test/specs/RangeSlider.spec.js | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index e63475dc2c1b..f09635793d68 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -1,8 +1,8 @@ {{>include "./SliderBase.hbs"}} {{#*inline "handlesAriaText"}} - {{_ariaHandlesText.startHandleText}} - {{_ariaHandlesText.endHandleText}} + {{_ariaHandlesText.startHandleText}} + {{_ariaHandlesText.endHandleText}} {{/inline}} {{#*inline "progressBar"}} @@ -41,11 +41,11 @@ aria-valuemin="{{min}}" aria-valuemax="{{max}}" aria-valuenow="{{startValue}}" - aria-labelledby="{{_ariaLabelledByStartHandleRefs}}" + aria-labelledby="ui5-slider-startHandleDesc" aria-disabled="{{_ariaDisabled}}" - aria-describedby="{{_ariaDescribedByHandleText}}" + .aria-describedby="{{_ariaDescribedByHandleText}}" > - +
{{#if showTooltip}} @@ -79,11 +79,11 @@ aria-valuemin="{{min}}" aria-valuemax="{{max}}" aria-valuenow="{{endValue}}" - aria-labelledby="{{_ariaLabelledByEndHandleRefs}}" - aria-describedby="{{_ariaDescribedByHandleText}}" + aria-labelledby="ui5-slider-endHandleDesc" + .aria-describedby="{{_ariaDescribedByHandleText}}" aria-disabled="{{_ariaDisabled}}" > - +
{{#if showTooltip}} diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index c48565c31b1b..7f8f03c01098 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -35,7 +35,7 @@ data-sap-focus-ref part="handle" > - +
{{#if showTooltip}}
diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index d41a505b8367..4f3c2c1685d1 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -789,7 +789,7 @@ abstract class SliderBase extends UI5Element { } get _ariaDescribedByHandleText() { - return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : "ui5-slider-accName"; + return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : ""; } get _ariaDescribedByInputText() { diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 8a2759e6c6dc..20f87dd2337e 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -583,7 +583,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await startHandle.getAttribute("aria-labelledby"), - "ui5-slider-accName ui5-slider-startHandleDesc", "aria-labelledby is set correctly"); + "ui5-slider-startHandleDesc", "aria-labelledby is set correctly"); assert.strictEqual(await startHandle.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await startHandle.getAttribute("aria-valuemax"), @@ -598,7 +598,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await endHandle.getAttribute("aria-labelledby"), - "ui5-slider-accName ui5-slider-endHandleDesc", "aria-labelledby is set correctly"); + "ui5-slider-endHandleDesc", "aria-labelledby is set correctly"); assert.strictEqual(await endHandle.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await endHandle.getAttribute("aria-valuemax"), @@ -619,8 +619,8 @@ describe("Accessibility", async () => { const rangeSlider = await browser.$("#range-slider-tickmarks"); const rangeSliderId = await rangeSlider.getProperty("_id"); const startHandle = await rangeSlider.shadow$(".ui5-slider-handle--start"); - const rangeSliderStartHandleSpan = await rangeSlider.shadow$(`#${rangeSliderId}-startHandleDesc`); - const rangeSliderEndHandleSpan = await rangeSlider.shadow$(`#${rangeSliderId}-endHandleDesc`); + const rangeSliderStartHandleSpan = await rangeSlider.shadow$(`#ui5-slider-startHandleDesc`); + const rangeSliderEndHandleSpan = await rangeSlider.shadow$(`#ui5-slider-endHandleDesc`); await rangeSlider.setProperty("endValue", 9); await startHandle.dragAndDrop({ x: 100, y: 1 }); From e6793b8143e02cdb8078ef47afc49ff17e2da4b9 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 16 Sep 2024 15:38:12 +0300 Subject: [PATCH 40/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip apply code review suggestions --- packages/main/src/RangeSlider.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index a04d158b5dee..e1d0431da820 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -314,12 +314,12 @@ class RangeSlider extends SliderBase implements IFormInputElement { _onInputFocusOut(e: FocusEvent) { const tooltipInput = e.target as Input; - const otherInput: Input = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; + const oppositeTooltipInput: Input = tooltipInput.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; const relatedTarget = e.relatedTarget as HTMLElement; if (this.startValue > this.endValue) { this._areInputValuesSwapped = true; - otherInput.focus(); + oppositeTooltipInput.focus(); } if (tooltipInput.hasAttribute("data-sap-ui-start-value")) { @@ -1006,14 +1006,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { return this.shadowRoot!.querySelector(".ui5-slider-progress")!; } - get _ariaLabelledByStartHandleRefs() { - return ["ui5-slider-accName", "ui5-slider-startHandleDesc"].join(" ").trim(); - } - - get _ariaLabelledByEndHandleRefs() { - return ["ui5-slider-accName", "ui5-slider-endHandleDesc"].join(" ").trim(); - } - get _ariaLabelledByProgressBarRefs() { return ["ui5-slider-accName", "ui5-slider-sliderDesc"].join(" ").trim(); } From c471639aacdd50d29aacedb9b620bdd47214e7ac Mon Sep 17 00:00:00 2001 From: Evdokia Yordanova Date: Wed, 18 Sep 2024 00:57:00 +0300 Subject: [PATCH 41/51] fix(ui5-multi-combobox): fix aria-describedby token count --- packages/main/src/MultiComboBox.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/main/src/MultiComboBox.ts b/packages/main/src/MultiComboBox.ts index 1f9feba88741..c9f1e9a839e0 100644 --- a/packages/main/src/MultiComboBox.ts +++ b/packages/main/src/MultiComboBox.ts @@ -497,7 +497,6 @@ class MultiComboBox extends UI5Element implements IFormInputElement { _isOpenedByKeyboard?: boolean; _itemToFocus?: IMultiComboBoxItem; _itemsBeforeOpen: Array; - _tokenCount: number | undefined; selectedItems: Array; static i18nBundle: I18nBundle; @@ -707,7 +706,6 @@ class MultiComboBox extends UI5Element implements IFormInputElement { } _tokenDelete(e: CustomEvent) { - this._tokenCount = this._tokenizer.tokens.length - e.detail.tokens.length; this._previouslySelectedItems = this._getSelectedItems(); const token: Token[] = e.detail.tokens; const deletingItems = this._getItems().filter(item => token.some(t => t.getAttribute("data-ui5-id") === item._id)); @@ -1894,8 +1892,7 @@ class MultiComboBox extends UI5Element implements IFormInputElement { return; } - this._tokenCount = this._tokenCount !== undefined ? this._tokenCount : this._tokenizer.tokens.length; - return getTokensCountText(this._tokenCount); + return getTokensCountText(this.selectedValues.length); } get _tokensCountTextId() { From 33071a6ed395dbf1a80f085e38f3e4f570a0ebe3 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 18 Sep 2024 09:43:11 +0300 Subject: [PATCH 42/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip code reviews acc fixes --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/RangeSlider.ts | 4 ---- packages/main/src/SliderBase.ts | 2 +- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index f09635793d68..9a5f2279101e 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -22,7 +22,7 @@ aria-valuemax="{{max}}" aria-valuenow="{{_ariaValueNow}}" aria-valuetext="From {{startValue}} to {{endValue}}" - aria-labelledby="{{_ariaLabelledByProgressBarRefs}}" + aria-labelledby="ui5-slider-accName ui5-slider-sliderDesc" aria-disabled="{{_ariaDisabled}}" >
diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index e1d0431da820..a31707cb9704 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -1006,10 +1006,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { return this.shadowRoot!.querySelector(".ui5-slider-progress")!; } - get _ariaLabelledByProgressBarRefs() { - return ["ui5-slider-accName", "ui5-slider-sliderDesc"].join(" ").trim(); - } - get _ariaLabelledByInputText() { return RangeSlider.i18nBundle.getText(SLIDER_TOOLTIP_INPUT_LABEL); } diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 4f3c2c1685d1..42db05d0d650 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -789,7 +789,7 @@ abstract class SliderBase extends UI5Element { } get _ariaDescribedByHandleText() { - return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : ""; + return this.editableTooltip ? "ui5-slider-accName ui5-slider-InputDesc" : undefined; } get _ariaDescribedByInputText() { From 6f2875e181c42e9b7e4cda60e06394c95d47f3b7 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 25 Sep 2024 09:26:03 +0300 Subject: [PATCH 43/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip apply code review feedback --- packages/main/src/RangeSlider.hbs | 4 +-- packages/main/src/RangeSlider.ts | 28 ++++++++++-------- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.hbs | 12 ++++++-- packages/main/test/pages/RangeSlider.html | 2 +- packages/main/test/specs/RangeSlider.spec.js | 30 ++++++++++++++++++-- 6 files changed, 57 insertions(+), 21 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index 9a5f2279101e..dd78d7831ec8 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -22,7 +22,7 @@ aria-valuemax="{{max}}" aria-valuenow="{{_ariaValueNow}}" aria-valuetext="From {{startValue}} to {{endValue}}" - aria-labelledby="ui5-slider-accName ui5-slider-sliderDesc" + aria-labelledby="ui5-slider-sliderDesc" aria-disabled="{{_ariaDisabled}}" >
@@ -54,7 +54,7 @@ this.endValue) { this._areInputValuesSwapped = true; oppositeTooltipInput.focus(); + return; } if (tooltipInput.hasAttribute("data-sap-ui-start-value")) { @@ -355,8 +357,6 @@ class RangeSlider extends SliderBase implements IFormInputElement { if (!isEnter(e)) { this._setAffectedValue(undefined); - } else { - this._areInputValuesSwapped = false; } if (this.startValue !== this._startValueAtBeginningOfAction || this.endValue !== this._endValueAtBeginningOfAction) { @@ -842,16 +842,17 @@ class RangeSlider extends SliderBase implements IFormInputElement { const input = e.target as Input; super._onInputKeydown(e); - const affectedValue = input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"; const otherInput: Input = input.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; - if (isEnter(e)) { - if (this.startValue > this.endValue) { - this._areInputValuesSwapped = true; - this._setAffectedValue(affectedValue); - otherInput.focus(); - } + if (isEnter(e) && this.startValue > this.endValue) { + this._areInputValuesSwapped = true; + this._setAffectedValue(input.hasAttribute("data-sap-ui-start-value") ? "endValue" : "startValue"); + + otherInput.focus(); + return; } + + this._setAffectedValue(input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"); } _updateInputValue() { @@ -907,10 +908,12 @@ class RangeSlider extends SliderBase implements IFormInputElement { } this._lastValidStartValue = isStartValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidStartValue); - this._lastValidEndValue = isEndValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidEndValue); + this._lastValidEndValue = isEndValueValid ? this._getFormattedValue(inputEndValue.toString()) : this._getFormattedValue(this._lastValidEndValue); - startValueInput.value = isStartValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidStartValue); - endValueInput.value = isEndValueValid ? this._getFormattedValue(inputEndValue.toString()) : this._getFormattedValue(this._lastValidEndValue); + if (startValueInput.valueState !== "Negative" && endValueInput.valueState !== "Negative") { + startValueInput.value = isStartValueValid ? this._getFormattedValue(inputStartValue.toString()) : this._getFormattedValue(this._lastValidStartValue); + endValueInput.value = isEndValueValid ? this._getFormattedValue(inputEndValue.toString()) : this._getFormattedValue(this._lastValidEndValue); + } } } @@ -959,6 +962,7 @@ class RangeSlider extends SliderBase implements IFormInputElement { } this.syncUIAndState(); + this._areInputValuesSwapped = false; } /** diff --git a/packages/main/src/Slider.hbs b/packages/main/src/Slider.hbs index 7f8f03c01098..3c6425e7586c 100644 --- a/packages/main/src/Slider.hbs +++ b/packages/main/src/Slider.hbs @@ -44,7 +44,7 @@ value="{{_tooltipInputValue}}" value-state="{{_tooltipInputValueState}}" type="Number" - accessible-name-ref="ui5-slider-accName ui5-slider-InputLabel" + accessible-name-ref="ui5-slider-InputLabel" @focusout="{{_onInputFocusOut}}" @keydown="{{_onInputKeydown}}" @ui5-change="{{_onInputChange}}" diff --git a/packages/main/src/SliderBase.hbs b/packages/main/src/SliderBase.hbs index d12a39bda045..da75fbd468a0 100644 --- a/packages/main/src/SliderBase.hbs +++ b/packages/main/src/SliderBase.hbs @@ -37,10 +37,16 @@ {{> handles}}
- {{accessibleName}} + {{#if accessibleName}} + {{accessibleName}} + {{/if}} + {{_ariaLabelledByText}} - {{_ariaDescribedByInputText}} - {{_ariaLabelledByInputText}} + + {{#if editableTooltip}} + {{_ariaDescribedByInputText}} + {{_ariaLabelledByInputText}} + {{/if}}
diff --git a/packages/main/test/pages/RangeSlider.html b/packages/main/test/pages/RangeSlider.html index 0e26d0e4772e..94282d41d2b9 100644 --- a/packages/main/test/pages/RangeSlider.html +++ b/packages/main/test/pages/RangeSlider.html @@ -40,7 +40,7 @@

Range Slider with tickmarks

Disabled Range Slider

-

Range Slider with steps, tooltips, tickmarks and labels

+

Range Slider with steps, input tooltips, tickmarks and labels

diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 20f87dd2337e..08dfb331ae34 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -330,6 +330,14 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSlider.getProperty("endValue"), 20, "The start value is now end value"); assert.strictEqual(await rangeSliderEndTooltipInput.getProperty("value"), "20", "The start input value is now end value"); + + await rangeSliderEndHandle.click(); + await rangeSliderEndTooltipInput.click(); + await rangeSliderEndTooltipInput.setProperty("value", "3"); + + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("endValue"), 3, "Slider value is changed on a followup input after initial swap interaction"); }); @@ -354,6 +362,24 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { assert.strictEqual(await rangeSlider.getProperty("startValue"), 1, "The end value is now start value"); assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "1", "The end input value is now start value"); }); + + it("Invalid tooltip value should not be changed on 'Enter'", async () => { + const rangeSlider = await browser.$("#range-slider-tickmarks-labels"); + const rangeSliderTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--end ui5-input"); + const rangeSliderHandle = await rangeSlider.shadow$(".ui5-slider-handle--end"); + + await rangeSlider.setProperty("endValue", 12); + + await rangeSliderHandle.click(); + await rangeSliderTooltipInput.click(); + await rangeSliderTooltipInput.setProperty("value", "60"); + + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("endValue"), 12, "The slider's value is not changed when invalid"); + assert.strictEqual(await rangeSliderTooltipInput.getProperty("valueState"), "Negative", "The input value is not changed when invalid"); + assert.strictEqual(await rangeSliderTooltipInput.getProperty("value"), "60", "The input value is not changed when invalid"); + }); it("Range Slider tooltips should become visible when range slider is focused", async () => { await browser.url(`test/pages/RangeSlider.html`); @@ -567,7 +593,7 @@ describe("Accessibility", async () => { const rangeSliderId = await rangeSlider.getProperty("_id"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-labelledby"), - "ui5-slider-accName ui5-slider-sliderDesc", "aria-labelledby is set correctly"); + "ui5-slider-sliderDesc", "aria-labelledby is set correctly"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-valuemin"), `${await rangeSlider.getProperty("min")}`, "aria-valuemin is set correctly"); assert.strictEqual(await rangeSliderProgressBar.getAttribute("aria-valuemax"), @@ -612,7 +638,7 @@ describe("Accessibility", async () => { const rangeSliderStartTooltipInput = await rangeSlider.shadow$(".ui5-slider-tooltip--start ui5-input"); assert.strictEqual(await rangeSliderStartTooltipInput.getAttribute("accessible-name-ref"), - "ui5-slider-accName ui5-slider-InputLabel"); + "ui5-slider-InputLabel"); }); it("Aria-labelledby text is mapped correctly when values are swapped", async () => { From 3342e70650feee1051668fc334b181a057836c3e Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 25 Sep 2024 13:52:11 +0300 Subject: [PATCH 44/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix slider test --- packages/main/test/specs/Slider.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/test/specs/Slider.spec.js b/packages/main/test/specs/Slider.spec.js index 319bf0c7f9c9..74c0fb804249 100644 --- a/packages/main/test/specs/Slider.spec.js +++ b/packages/main/test/specs/Slider.spec.js @@ -424,7 +424,7 @@ describe("Accessibility", async () => { const sliderTooltipInput = await slider.shadow$(".ui5-slider-tooltip ui5-input"); assert.strictEqual(await sliderTooltipInput.getAttribute("accessible-name-ref"), - "ui5-slider-accName ui5-slider-InputLabel"); + "ui5-slider-InputLabel"); }); it("Click anywhere in the Slider should focus the Slider's handle", async () => { From 713bd575a388ed748de7d2a3ad0e69c1766c063c Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 26 Sep 2024 08:42:31 +0300 Subject: [PATCH 45/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip remove the slider's accessible name from the tooltip input and othe minor improvements acc improvements --- packages/main/src/RangeSlider.hbs | 2 +- packages/main/src/Slider.hbs | 2 +- packages/main/src/SliderBase.ts | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/main/src/RangeSlider.hbs b/packages/main/src/RangeSlider.hbs index dd78d7831ec8..6ac3e9b8e866 100644 --- a/packages/main/src/RangeSlider.hbs +++ b/packages/main/src/RangeSlider.hbs @@ -92,7 +92,7 @@ Date: Thu, 26 Sep 2024 09:34:49 +0300 Subject: [PATCH 46/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip minor css improvements --- packages/main/src/themes/SliderBase.css | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/main/src/themes/SliderBase.css b/packages/main/src/themes/SliderBase.css index 08404195024b..1e642f9ca3a2 100644 --- a/packages/main/src/themes/SliderBase.css +++ b/packages/main/src/themes/SliderBase.css @@ -152,7 +152,7 @@ top: var(--_ui5_slider_handle_top); } -.ui5-slider-handle-container .ui5-slider-tooltip { +:host(:not([hidden])) .ui5-slider-handle-container .ui5-slider-tooltip { display: flex; justify-content: center; align-items: center; @@ -173,16 +173,19 @@ box-sizing: var(--_ui5_slider_tooltip_border_box); } +:host(:not([hidden])):host([editable-tooltip]) .ui5-slider-handle-container .ui5-slider-tooltip { + border: none; + background: none; + box-shadow: none; +} + :host([editable-tooltip]) .ui5-slider-tooltip { padding: 0; box-shadow: none; } .ui5-slider-tooltip [ui5-input] { - height: 100%; width: 100%; - margin: 0; - padding: 0; text-align: center; } From b1f8d3adb72d32215e74e91d0b2e1ecbad56d75b Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Tue, 1 Oct 2024 09:41:03 +0300 Subject: [PATCH 47/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix code review bug --- packages/main/src/RangeSlider.ts | 30 ++++++++++++++++---- packages/main/test/specs/RangeSlider.spec.js | 15 ++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/main/src/RangeSlider.ts b/packages/main/src/RangeSlider.ts index 5f1aefcb02f0..ec027f15faa2 100644 --- a/packages/main/src/RangeSlider.ts +++ b/packages/main/src/RangeSlider.ts @@ -839,20 +839,38 @@ class RangeSlider extends SliderBase implements IFormInputElement { } _onInputKeydown(e: KeyboardEvent): void { - const input = e.target as Input; + const targetedInput = e.target as Input; + const startValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]") as Input; + const endValueInput = this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]") as Input; + + const startValue = parseFloat(startValueInput.value); + const endValue = parseFloat(endValueInput.value); + const affectedValue = targetedInput.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"; super._onInputKeydown(e); - const otherInput: Input = input.hasAttribute("data-sap-ui-start-value") ? this.shadowRoot!.querySelector("ui5-input[data-sap-ui-end-value]")! : this.shadowRoot!.querySelector("ui5-input[data-sap-ui-start-value]")!; - if (isEnter(e) && this.startValue > this.endValue) { + if (isEnter(e) && startValue > endValue) { + const swappedInput = affectedValue === "startValue" ? endValueInput : startValueInput; + const isValueValid = parseFloat(targetedInput.value) >= this.min && parseFloat(startValueInput.value) <= this.max; + + if (!isValueValid) { + targetedInput.valueState = "Negative"; + return; + } + + this._isEndValueValid = parseFloat(endValueInput.value) >= this.min && parseFloat(endValueInput.value) <= this.max; + this._areInputValuesSwapped = true; - this._setAffectedValue(input.hasAttribute("data-sap-ui-start-value") ? "endValue" : "startValue"); + this._setAffectedValue(affectedValue === "startValue" ? "endValue" : "startValue"); + + startValueInput.value = this._getFormattedValue(this.endValue.toString()); + endValueInput.value = this._getFormattedValue(this.startValue.toString()); + swappedInput.focus(); - otherInput.focus(); return; } - this._setAffectedValue(input.hasAttribute("data-sap-ui-start-value") ? "startValue" : "endValue"); + this._setAffectedValue(affectedValue); } _updateInputValue() { diff --git a/packages/main/test/specs/RangeSlider.spec.js b/packages/main/test/specs/RangeSlider.spec.js index 08dfb331ae34..751b45314fe7 100644 --- a/packages/main/test/specs/RangeSlider.spec.js +++ b/packages/main/test/specs/RangeSlider.spec.js @@ -338,6 +338,21 @@ describe("Range Slider elements - tooltip, step, tickmarks, labels", () => { await browser.keys("Enter"); assert.strictEqual(await rangeSlider.getProperty("endValue"), 3, "Slider value is changed on a followup input after initial swap interaction"); + + await browser.keys("ArrowDown"); + await browser.keys("Enter"); + + await browser.keys("ArrowDown"); + await browser.keys("Enter"); + + await browser.keys("ArrowDown"); + await browser.keys("Enter"); + + assert.strictEqual(await rangeSlider.getProperty("endValue"), 1, "Slider value is changed on a followup keyboard actions after initial swap interaction"); + assert.strictEqual(await rangeSlider.getProperty("startValue"), 0, "Slider value is changed on a followup keyboard actions after initial swap interaction"); + + assert.strictEqual(await rangeSliderEndTooltipInput.getProperty("value"), "1", "Slider end value is changed on a followup keyboard actions after initial swap interaction"); + assert.strictEqual(await rangeSliderStartTooltipInput.getProperty("value"), "0", "Slider start value is changed on a followup keyboard actions after initial swap interaction"); }); From 2f1bd2a84534370b0d739c23afbc1ebb43623f52 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 9 Oct 2024 12:58:28 +0300 Subject: [PATCH 48/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip add documentation to the new property --- packages/main/src/SliderBase.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index dc4dcf0c3180..f85750edec42 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -110,7 +110,8 @@ abstract class SliderBase extends UI5Element { showTooltip = false; /** - * Enables handle tooltip displaying the current value. + * +Indicates whether input fields should be used as tooltips for the handles. Note: Setting this option to true will only work if showTooltip is set to true. * @default false * @public */ From 7720fa85a2c8106e50618cc1982926a19a099d72 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Wed, 9 Oct 2024 13:09:06 +0300 Subject: [PATCH 49/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip update documentation --- packages/main/src/SliderBase.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index f85750edec42..6700feede092 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -111,7 +111,9 @@ abstract class SliderBase extends UI5Element { /** * -Indicates whether input fields should be used as tooltips for the handles. Note: Setting this option to true will only work if showTooltip is set to true. + * Indicates whether input fields should be used as tooltips for the handles. Note: Setting this option to true will only work if showTooltip is set to true. + * + * **Note:** In order for the component to comply with the accessibility standard, it is recommended to set the editableTooltip property to true. * @default false * @public */ From a2448c77031ed496e5f6b28cb434085431dea252 Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Thu, 10 Oct 2024 10:27:48 +0300 Subject: [PATCH 50/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip fix translatable string --- packages/main/src/i18n/messagebundle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/main/src/i18n/messagebundle.properties b/packages/main/src/i18n/messagebundle.properties index 3cf58a24c308..eadb967ade56 100644 --- a/packages/main/src/i18n/messagebundle.properties +++ b/packages/main/src/i18n/messagebundle.properties @@ -502,7 +502,7 @@ YEAR_PICKER_DESCRIPTION = Year Picker SLIDER_TOOLTIP_INPUT_DESCRIPTION = Press F2 to enter a value #XACT: ARIA label for slider tooltip input -SLIDER_TOOLTIP_INPUT_LABEL = Current value +SLIDER_TOOLTIP_INPUT_LABEL = Current Value #XTOL: tooltip for decrease button of the StepInput STEPINPUT_DEC_ICON_TITLE=Decrease From 0e288a3f4f241c8685df450c2578fe28eafe60af Mon Sep 17 00:00:00 2001 From: Nikolay Deshev Date: Mon, 14 Oct 2024 09:35:21 +0300 Subject: [PATCH 51/51] feat(ui5-slider, ui5-range-slider): add input as a tooltip update documentation --- packages/main/src/SliderBase.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/main/src/SliderBase.ts b/packages/main/src/SliderBase.ts index 6700feede092..2dce681bc11b 100644 --- a/packages/main/src/SliderBase.ts +++ b/packages/main/src/SliderBase.ts @@ -111,8 +111,9 @@ abstract class SliderBase extends UI5Element { /** * - * Indicates whether input fields should be used as tooltips for the handles. Note: Setting this option to true will only work if showTooltip is set to true. + * Indicates whether input fields should be used as tooltips for the handles. * + * **Note:** Setting this option to true will only work if showTooltip is set to true. * **Note:** In order for the component to comply with the accessibility standard, it is recommended to set the editableTooltip property to true. * @default false * @public