Skip to content

Simplify Input Validation #679

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

Merged
merged 9 commits into from
Mar 6, 2019
Merged

Conversation

nicknisi
Copy link
Member

@nicknisi nicknisi commented Mar 4, 2019

Type: feature

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Simplify text-input validation

  • remove validate property
  • add valid?: boolean | { valid?: boolean; message?: string; } property
  • call onValidate and pass message and valid from meta
  • onValidate is now a callback to receive native validation information from the input. Custom validation can now be achieved simply by setting the valid property.
  • helperText will show a message if the message value exists. Otherwise, it'll show the set helperText.

nicknisi added 2 commits March 4, 2019 10:36
- remove validate property
- add `valid?: boolean` property
- call `onValidate` and pass message and valid from meta
- onValidate is now a place where the `valid` property can be
set/changed and `invalidate` called
valid can now be `boolean | { valid?: boolean; message?: string; }`

helperText will show a message if the message value exists. Otherwise,
it'll show the set helperText.
@nicknisi nicknisi marked this pull request as ready for review March 4, 2019 20:30
@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #679 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #679      +/-   ##
==========================================
- Coverage   99.03%   99.03%   -0.01%     
==========================================
  Files          44       44              
  Lines        3319     3317       -2     
  Branches      913      912       -1     
==========================================
- Hits         3287     3285       -2     
  Misses         32       32
Impacted Files Coverage Δ
src/enhanced-text-input/index.ts 100% <ø> (ø) ⬆️
src/combobox/index.ts 100% <100%> (ø) ⬆️
src/text-input/index.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb0849a...f180cf5. Read the comment docs.

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #679 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #679      +/-   ##
========================================
- Coverage   99.03%    99%   -0.03%     
========================================
  Files          44     44              
  Lines        3319   3328       +9     
  Branches      913    918       +5     
========================================
+ Hits         3287   3295       +8     
- Misses         32     33       +1
Impacted Files Coverage Δ
src/enhanced-text-input/index.ts 100% <ø> (ø) ⬆️
src/combobox/index.ts 100% <100%> (ø) ⬆️
src/text-input/index.ts 100% <100%> (ø) ⬆️
src/common/InputValidity.ts 81.81% <0%> (-9.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb0849a...938947c. Read the comment docs.

Copy link
Member

@tomdye tomdye left a comment

Choose a reason for hiding this comment

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

Looks ok to me, without having checked it all out. Does it work as expected @nicknisi ?

nicknisi added 2 commits March 4, 2019 15:21
- add customValidator optional property that can be used to trigger
custom vaidation checks AFTER native validation succeeds
- prevent onValidate from being called when the validation state hasn't
changed
@nicknisi nicknisi requested a review from tomdye March 5, 2019 22:05
@nicknisi nicknisi merged commit 008866a into dojo:master Mar 6, 2019
@nicknisi nicknisi deleted the onvalidate-refactor branch March 6, 2019 16:54
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.

2 participants