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

fix(Field): parseName defaults #683

Merged
merged 2 commits into from
May 21, 2019
Merged

Conversation

jdkahn
Copy link
Contributor

@jdkahn jdkahn commented May 20, 2019

  • field constructor saves passed values
  • field init looks at constructor values for defaults

- field constructor saves passed values
- field init looks at constructor values for defaults
@jdkahn jdkahn requested review from bindoon and youluna May 20, 2019 02:50
@@ -29,6 +30,7 @@ class Field {
this.fieldsMeta = {};
this.cachedBind = {};
this.instance = {};
this.initValues = options.values || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Persist constructor values

} else if (originalProps[defaultValueName]) {
defaultValue = originalProps[defaultValueName];
} else {
defaultValue = get(this.initValues, name);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If no other values to set as default, look at the initValues set in constructor.

@codecov
Copy link

codecov bot commented May 20, 2019

Codecov Report

Merging #683 into master will increase coverage by 80.15%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #683       +/-   ##
===========================================
+ Coverage   11.01%   91.17%   +80.15%     
===========================================
  Files         244      244               
  Lines       11144    13897     +2753     
  Branches     2776     4341     +1565     
===========================================
+ Hits         1228    12670    +11442     
+ Misses       9782     1208     -8574     
+ Partials      134       19      -115
Impacted Files Coverage Δ
src/field/index.js 91.04% <100%> (+90.67%) ⬆️
src/upload/runtime/iframe-uploader.jsx 1.09% <0%> (-0.28%) ⬇️
src/util/guid.js 100% <0%> (ø) ⬆️
src/util/support.js 100% <0%> (ø) ⬆️
src/balloon/util.js 100% <0%> (ø) ⬆️
src/badge/sup.jsx 97.5% <0%> (+2.85%) ⬆️
src/animate/expand.jsx 98.46% <0%> (+4.23%) ⬆️
src/affix/util.js 87.5% <0%> (+5.68%) ⬆️
src/overlay/utils/find-node.js 91.66% <0%> (+9.84%) ⬆️
src/overlay/utils/position.js 91.61% <0%> (+10.55%) ⬆️
... and 211 more

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 e5e7a71...83bf851. Read the comment docs.

@@ -1,4 +1,5 @@
import ReactDOM from 'react-dom';
import { get } from 'lodash';
Copy link
Member

@youluna youluna May 20, 2019

Choose a reason for hiding this comment

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

We don't have a plan to make lodash be our dependencies, it's better to write a get method in util on our own

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@youluna Resolved

defaultValue = initValue;
} else if (originalProps[defaultValueName]) {
defaultValue = originalProps[defaultValueName];
} else {
Copy link
Member

@bindoon bindoon May 24, 2019

Choose a reason for hiding this comment

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

should here be else if (this.options.parseName) ?
because setIn will talk a lot of compute works, if the initValues is big, this will slow down the progress.

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.

3 participants