Skip to content

Commit

Permalink
Improve dirty handling for check-boxes (#1762)
Browse files Browse the repository at this point in the history
`dirty(undefined, false)` should be `false`. It was `true`.

Intuitively, clicking twice on a check-box should return it to
pristine, which is false atm (unless the checkbox is initialized with
`false`).

Same behavior as empty and undefined for a string.

Also decreases code complexity. If both value and initial are falsey,
return �equal true�.

Comes with tests, both immutable and plain structures are covered.

I�d call it a safe merge and would be very pleased to see it shipped.
:hatching_chick:
  • Loading branch information
bbenezech authored and erikras committed Sep 15, 2016
1 parent 787de21 commit a4c1d9f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
12 changes: 12 additions & 0 deletions src/structure/immutable/__tests__/deepEqual.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,5 +275,17 @@ describe('structure.immutable.deepEqual', () => {
}
}), true)
})

it('should treat false and undefined as equal', () => {
testBothWays(fromJS({
a: {
b: false
}
}), fromJS({
a: {
b: undefined
}
}), true)
})
})

4 changes: 2 additions & 2 deletions src/structure/immutable/deepEqual.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { isEqualWith } from 'lodash'

const customizer = (obj, other) => {
if (obj == other) return true
if (obj == null && other === '') return true
if (obj === '' && other == null) return true
if ((obj == null || obj === '' || obj === false) &&
(other == null || other === '' || other === false)) return true

if (Iterable.isIterable(obj) && Iterable.isIterable(other)) {
return obj.count() === other.count() && obj.every((value, key) => {
Expand Down
12 changes: 12 additions & 0 deletions src/structure/plain/__tests__/deepEqual.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,5 +188,17 @@ describe('structure.plain.deepEqual', () => {
testBothWays(a, b, false)
testBothWays(b, c, true)
})

it('should treat false and undefined as equal', () => {
testBothWays({
a: {
b: false
}
}, {
a: {
b: undefined
}
}, true)
})
})

5 changes: 3 additions & 2 deletions src/structure/plain/deepEqual.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { isEqualWith } from 'lodash'

const customizer = (obj, other) => {
if (obj == other) return true
if (obj == null && other === '') return true
if (obj === '' && other == null) return true
if ((obj == null || obj === '' || obj === false) &&
(other == null || other === '' || other === false)) return true

if (obj && other && obj._error !== other._error) return false
}

Expand Down

0 comments on commit a4c1d9f

Please sign in to comment.