Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Removes slider from number item on control page #3437

Merged
merged 3 commits into from
May 16, 2017

Conversation

aounhaider1
Copy link
Contributor

as discussed here #1700 (comment)
Signed-off-by: Aoun Bukhari bukhari@itemis.de

Signed-off-by: Aoun Bukhari <bukhari@itemis.de>
@htreu
Copy link
Contributor

htreu commented May 15, 2017

Looks good, works as described. @aounhaider1 do you mind adding min and max attributes to the input field? This way at least the stepper buttons will not allow input of values outside the given range.

Signed-off-by: Aoun Bukhari <bukhari@itemis.de>
@aounhaider1
Copy link
Contributor Author

Updated

@htreu
Copy link
Contributor

htreu commented May 15, 2017

Its awesome to have the validation build right in with the ng-min and ng-max attributes. Nice! But the steppers will allow the input of values beyond the valid range. With additional min & max attributes this can be avoided.
Another thing: When angular reports the value as invalid its still possible to "commit" the value which will result in a 400 - Bad Request response. Is it possible to deactivate the commit button on invalid data? See here:

screen shot 2017-05-15 at 13 56 29

The valid range is [-80;12]. The value 100 is clearly reported as being invalid, see attribute aria-invalid="true"

Signed-off-by: Aoun Bukhari <bukhari@itemis.de>
@aounhaider1
Copy link
Contributor Author

I've added min, max validation to number and rollershutter widget. But I think we should have decent editing option on control page as well (with proper error message and so on). However, we have a limited space for items so maybe we can introduce an expandable area and show the input field in that area. Wdyt?

@htreu
Copy link
Contributor

htreu commented May 15, 2017

Imho the current editing function is fine. I´m just talking about the submit button when ng-min or ng-max detected an invalid value:

image

can't we disable the button then?

@aounhaider1
Copy link
Contributor Author

It should be working with the last commit. :)

@htreu
Copy link
Contributor

htreu commented May 16, 2017

ready to get merged.

@sjsf sjsf merged commit 32625e9 into eclipse-archived:master May 16, 2017
@sjsf
Copy link
Contributor

sjsf commented May 16, 2017

Thanks!

@kaikreuzer kaikreuzer modified the milestone: 0.9.0 Jun 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants