Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

feat: cross-browser validation of values in a numeric input field #1082

Merged
merged 6 commits into from
Jun 6, 2018

Conversation

tamazlykar
Copy link
Collaborator

Fixes #940
The implemented directive used to prevent user to the entry of the text values into the numeric input field and to limit the possibility of entering numbers beyond the boundaries settled by "csMaxValue" and "csMinValue".
Problem was that it wasn't validated by some browsers like IE.

Also, there is a new requirement for number inputs that have some valid ranges.

  • input can't be empty, min value should be placed instead (exception: new untouched form)
  • input doesn't allow you enter lesser value than a minimum or greater than a maximum

package.json Outdated
@@ -50,6 +50,7 @@
"rxjs": "^5.5.2",
"showdown": "^1.8.4",
"sprintf-js": "^1.1.1",
"typescript-debounce-decorator": "^0.0.17",
Copy link
Contributor

Choose a reason for hiding this comment

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

use lodash instead

Copy link
Contributor

@zolotyx zolotyx left a comment

Choose a reason for hiding this comment

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

Just fix small typing issue and put to verify

return this.updateElementValue(newValue);
}

const value: number = +newValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

newValue is a number, but you convert it to number anyway, could you please add a correct typing for the method arguments

@tamazlykar tamazlykar force-pushed the 940-input-number-validation branch from fe82561 to 948ff4f Compare May 31, 2018 09:32
@tamazlykar tamazlykar merged commit dad0f00 into bwsw:master Jun 6, 2018
@tamazlykar tamazlykar deleted the 940-input-number-validation branch June 6, 2018 02:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants