feat: Merged state and user can change merged state. #224
Conversation
yanguoyu
requested review from
Daryl-L,
Keith-CY,
zhengjianhui and
PainterPuppets
| merge(object: S, source?: S): S { | ||
| if (source === undefined || source === null) return object | ||
| if (Array.isArray(object)) return object.concat(source) as S | ||
| return this.defaultMergeWith(source, object) |
There was a problem hiding this comment.
The order of destination and source is reversed from that of definition
There was a problem hiding this comment.
We should use the new value in object to cover the old value in source, so we need set object as the second parameter.
There was a problem hiding this comment.
If we can switch the parameters of private defaultMergeWith so they won't be mixed in all cases.
There was a problem hiding this comment.
Let's do it later because defaultMergeWith is private
|
Please have a review @zhengjianhui @Daryl-L @PainterPuppets |
|
Please have a review @zhengjianhui @Daryl-L @PainterPuppets |
|
Please resolve the conflicts |
|
I'm adding test cases for |
yanguoyu
force-pushed
the
feat-merge-cells
branch
from
3cd0810
to
2be2274
Compare
Codecov Report
@@ Coverage Diff @@
## develop #224 +/- ##
===========================================
+ Coverage 68.56% 70.12% +1.56%
===========================================
Files 84 85 +1
Lines 2306 2490 +184
Branches 515 555 +40
===========================================
+ Hits 1581 1746 +165
- Misses 658 675 +17
- Partials 67 69 +2
Continue to review full report in Codecov by Sentry.
|
merge-strategy, and createDefaultMergeStrategyandUseLatestStrategy.mergeStatehas calculated whenhandleCell, and developers can changemergeStatebyset.