feat(ui5-number-input): introduce NumberInput component#13799
feat(ui5-number-input): introduce NumberInput component#13799GDamyanov wants to merge 14 commits into
Conversation
👋 Heads-up: dev close is in effectThanks for the contribution! This repository is currently in dev close ahead of release This PR appears to introduce public-API changes (detected by diffing the Custom Elements Manifest against the latest published version on npm):
Could you please hold off on merging into If this change must ship in the current release, please request a review from one or two members of @UI5/ui5-team-webc so the team can sign off explicitly.
Posted automatically by the Dev Close Notice workflow. |
|
🚀 Deployed on https://pr-13799--ui5-webcomponents-preview.netlify.app |
GDamyanov
temporarily deployed
to
netlify-preview
GitHub Actions
Inactive
GDamyanov
temporarily deployed
to
netlify-preview
GitHub Actions
Inactive
GDamyanov
temporarily deployed
to
netlify-preview
GitHub Actions
Inactive
hinzzx
left a comment
hinzzx
left a comment
There was a problem hiding this comment.
- We should also register the NumberInput in ReactPlayground;
| _onButtonFocusOut() { | ||
| setTimeout(() => { | ||
| if (!this._inputFocused && !this.shadowRoot!.activeElement) { | ||
| this.inputOuter.removeAttribute("focused"); |
There was a problem hiding this comment.
This must be property/template-driven. Using remove/setAttribute is a forbidden pattern.
Just use this.focused = false, and add the focused={this.focused} in the tempalte.
| } from "@ui5/webcomponents-base/dist/Keys.js"; | ||
| import i18n from "@ui5/webcomponents-base/dist/decorators/i18n.js"; | ||
| import type I18nBundle from "@ui5/webcomponents-base/dist/i18nBundle.js"; | ||
| import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js"; |
There was a problem hiding this comment.
import type ValueState, then wherever used, replace with template string literals.
ValueState.None → `${ValueState.None}`
| this._validate(); | ||
| this._setButtonState(); | ||
| this.focused = true; | ||
| this.inputOuter.setAttribute("focused", ""); |
There was a problem hiding this comment.
Here it should also be property/template-driven. We should not use remove/setattribute.
| return { | ||
| patternMismatch: this.value !== 0 && !this._isValueWithCorrectPrecision, | ||
| rangeOverflow: this.max !== undefined && this.value >= this.max, | ||
| rangeUnderflow: this.min !== undefined && this.value <= this.min, |
There was a problem hiding this comment.
form-validity boundaries are inverted: rangeOverflow: value >= max, rangeUnderflow: value <= min,
this means a value exactly at min or max (e.g. max=10, value=10) is flagged form-invalid; clamping to max leaves it permanently invalid.
Think we should remove the =
| onFocusOut={this._onfocusout} | ||
| onWheel={this._onMouseWheel} | ||
| <NumberInput | ||
| _showStepButtons={true} |
There was a problem hiding this comment.
The shadow DOM surface changed.
Step-input's internals went from [ui5-input] (.ui5-step-input-root/-input) to a nested [ui5-number-input].
Any consumer CSS, querySelector, or ::part targeting the old internals breaks.
I think this is a BREAKING CHANGE, which we should document somewhere.
| bubbles: true, | ||
| cancelable: true, | ||
| }) | ||
| @event("_request-submit", { |
There was a problem hiding this comment.
internal event is used as the public coordination channel between two public components
| _setDefaultInputValueIfNeeded() { | ||
| if (this.input.value === "") { | ||
| const defaultValue = this._formatNumber(this.min || 0); | ||
| this.input.value = defaultValue; |
| return !Number.isNaN(parsedValue) && !/, {2,}/.test(typedValue); | ||
| } | ||
|
|
||
| _decSpin(e: MouseEvent) { |
There was a problem hiding this comment.
I think the whole spin functionality lack tests
| } | ||
|
|
||
| onExitDOM() { | ||
| this._cleanupLanguageChangeHandler(); |
There was a problem hiding this comment.
I think we should reset the spin there.
this._resetSpin()
| --_ui5_number_input_error_focused_background: var(--sapField_Focus_Background); | ||
| --_ui5_number_input_button_state_hover_background_color: var(--sapField_Background); | ||
| --_ui5_number_input_error_hover_background: var(--sapField_Background); | ||
| --_ui5_number_input_border_style: 1px solid var(--sapField_BorderColor);; |
| --_ui5_number_input_error_warning_information_border_style: none; | ||
| --_ui5_number_input_after_border_style: var(--_ui5_input_error_warning_border_style); | ||
| --_ui5_number_input_error_background_color: var(--_ui5_input_input_background_color); | ||
| --_ui5_number_input_error_background_color_hover: var(--_ui5_number_input_input_error_background_color); |
| @@ -0,0 +1,32 @@ | |||
| :host { | |||
There was a problem hiding this comment.
I think we should add: --_ui5_number_input_min_width: 7.25rem;
2 participants
Summary
Introduces a new
ui5-number-inputcomponent as the dedicated numeric input primitive, and refactorsui5-step-inputto be built on top of it.New Component:
ui5-number-inputA numeric input field
ui5-number-inputhas been extracted as a standalone component. It handles all numeric input logic internally:_showStepButtons)Refactored: ui5-step-input
ui5-step-inputis now a thin wrapper aroundui5-number-input, delegating all numeric logic to itA new private property is added in the
ui5-number-input:_showStepButtonsis a @Private, noAttribute boolean property onui5-number-input(defaults to false) that controls whether the increment/decrement buttons are rendered. When false,ui5-number-inputrenders as a plain numeric input with no step buttons — suitable for standalone use.ui5-step-inputsets_showStepButtons={true}in its template, which activates the buttons and the associated CSS modifier class (ui5-number-input-root--with-buttons). This means the step button UI lives entirely insideui5-number-input;ui5-step-inputdoes not render any buttons of its own.