Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve vaadin-number-field #292

Merged

Conversation

samiheikki
Copy link
Contributor

@samiheikki samiheikki commented Dec 22, 2018

Connects to #296

This change is Reviewable

src/vaadin-number-field.html Outdated Show resolved Hide resolved
src/vaadin-number-field.html Outdated Show resolved Hide resolved
Copy link
Member

@tomivirkki tomivirkki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the stepper controls for increasing/decreasing the value, we probably don't want the field to unnecessarily get focused. Focusing the input opens the virtual keyboard on mobile which might be undesired. @jouni wdyt?

@jouni
Copy link
Member

jouni commented Jan 10, 2019

Yes, I agree with Tomi.

@samiheikki samiheikki force-pushed the finalize-feature/vaadin-number-field branch from d638e9b to c7d6c17 Compare January 13, 2019 16:45
@samiheikki
Copy link
Contributor Author

Removed input focus when using the stepper controls.

@samiheikki
Copy link
Contributor Author

As I've been implementing this I have noticed that trying to validate the <input type="number"> value is surprisingly difficult.

input.value is not the same as the actual letters/numbers I'm typing to the input field (as shown in the demo) https://jsfiddle.net/samiheikki/q7dx8Lfw/12/ Try running it in Chrome & Safari

And thus I don't know what actually is typed in the number field when I'm relying on the browser's validated value of the number field.

So instead of using <input type="number"> & custom validation I'm seeing 2 alternative options.

<input type="text"> with our custom validation.

  • input.value works as we expect it to.
  • Easy to validate numbers and special characters like ,, ., -, e
  • Hurts a11y

<input type="number"> with no custom validation.

  • We can use input.value and that will always give us the correct numeric value
  • Some browsers allow letters (but they are not added to the input.value)
  • Works more like native <input type="number">

I'd be in favor of option 2. BTW I haven't implemented either of these yet.

@tomivirkki LMKWYT. I can discuss more about this.

@tomivirkki
Copy link
Member

  1. sounds like the better option

src/vaadin-number-field.html Show resolved Hide resolved
src/vaadin-number-field.html Outdated Show resolved Hide resolved
@tomivirkki
Copy link
Member

@samiheikki UX of the last number field on the demo page (with steps) is pretty unexpected when using keyboard (both desktop and mobile). Can something be done about it?

kapture 2019-01-14 at 15 02 13

@samiheikki
Copy link
Contributor Author

@tomivirkki That could be achieved by adding !this.__userInput to this if clause: https://github.com/vaadin/vaadin-text-field/blob/finalize-feature/vaadin-number-field/src/vaadin-number-field.html#L202

But then it won't validate the step when input value changes. So not sure which is more correct behaviour.

@tomivirkki tomivirkki merged commit dc56a3a into feature/vaadin-number-field Jan 15, 2019
tomivirkki pushed a commit that referenced this pull request Jan 15, 2019
* Improve vaadin-number-field

* Fix value control buttons focusing behavior for touch

* Add browser native validation behaviour
web-padawan pushed a commit that referenced this pull request Jan 16, 2019
* Implement number-field

* Include number-field demos in demos suite

* Align the name of the file with number field demo with other demos

* The reference to native input has changed. Fixed tests

* move number field to src, and create its lumo file

* Update screenshots

* Validate against min, max. Synchronize input value

* Disable buttons on readonly

* Fix min/max limits when set to zero

* Simply code by reducing conditional blocks

* Disable buttons when limits have been reached

* Align with master

* Adding step feature and focus input on click.

* Improve vaadin-number-field (#292)

* Improve vaadin-number-field

* Fix value control buttons focusing behavior for touch

* Add browser native validation behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants