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

V5 new feature: remove array field by predicate function #3739

Merged
merged 9 commits into from Mar 2, 2018
Merged

V5 new feature: remove array field by predicate function #3739

merged 9 commits into from Mar 2, 2018

Conversation

danikkks
Copy link

@danikkks danikkks commented Jan 11, 2018

solution for this feature request - #3738

@danikkks
Copy link
Author

@danikkks
fix: getValue not return object when one of keys is "value"
9ea4a7c
@danikkks
add _initData prop to reducer
7896340
@danikkks
remove .npmrc and add it to gitignore
d5370bf
@danikkks
remove .npmrc and add it to gitignore
a0c083d
@danikkks
fix version
db300aa
@danikkks
Merge branch 'v5' of https://github.com/danikkks/redux-form into v5
a386334

this commits are already merged here - #3382
I dont know how to remove them from this pull request

@@ -0,0 +1,4 @@
export default (value) => {
Copy link

Choose a reason for hiding this comment

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

why don't you use the isFunction from loadash?

https://lodash.com/docs/4.17.4#isFunction

Copy link
Author

Choose a reason for hiding this comment

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

v5 of redux-form don't include lodash as dependency

src/reducer.js Outdated
@@ -79,10 +80,20 @@ const behaviors = {
[INITIALIZE](state, {data, fields, overwriteValues}) {
return createInitialState(data, fields, state, overwriteValues);
},
[REMOVE_ARRAY_VALUE](state, {path, index}) {
[REMOVE_ARRAY_VALUE](state, {path, indexOrPredicate}) {
Copy link

Choose a reason for hiding this comment

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

suggestion: it would be better if the param 'indexOrPredicate' would split into 2 different params, the first 1 is 'index' and the second is 'predicate', and the logic is basically the same.

when you check isFunction the indexOrPredicate, you'll check if (predicate && isFuncion(predicate))

the reason for the split is to keep Single Responsibility, each param should have only 1 purpose, and the word 'or' applies that this param have 2 different purposes

it's just a suggestion :)

@danikkks
Copy link
Author

danikkks commented Feb 12, 2018

@itommen commented on this pull request.

In src/reducer.js:

@@ -79,10 +80,20 @@ const behaviors = {
[INITIALIZE](state, {data, fields, overwriteValues}) {
return createInitialState(data, fields, state, overwriteValues);
},

  • [REMOVE_ARRAY_VALUE](state, {path, index}) {
  • [REMOVE_ARRAY_VALUE](state, {path, indexOrPredicate}) {

suggestion: it would be better if the param 'indexOrPredicate' would split into 2 different params, the first 1 is 'index' and the second is 'predicate', and the logic is basically the same.

when you check isFunction the indexOrPredicate, you'll check if (predicate && isFuncion(predicate))

the reason for the split is to keep Single Responsibility, each param should have only 1 purpose, and the word 'or' applies that this param have 2 different purposes

it's just a suggestion :)

DONE!

@erikras erikras merged commit 13eaf54 into redux-form:v5 Mar 2, 2018
@lock
Copy link

lock bot commented Jun 1, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 1, 2018
@erikras
Copy link
Member

erikras commented Mar 22, 2019

Published in v5.4.0.

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.

None yet

3 participants