[GitHub] [incubator-echarts] plainheart commented on a change in pull request #12561: fix(pie): pie series render incorrectly after editing its data in DataView. close #12560.

[GitBox <gi...@apache.org>]

plainheart commented on a change in pull request #12561:
URL: https://github.com/apache/incubator-echarts/pull/12561#discussion_r420661647



##########
File path: src/component/toolbox/feature/DataView.js
##########
@@ -426,13 +432,17 @@ function tryMergeDataOption(newData, originalData) {
     return zrUtil.map(newData, function (newVal, idx) {
         var original = originalData && originalData[idx];
         if (zrUtil.isObject(original) && !zrUtil.isArray(original)) {
-            if (zrUtil.isObject(newVal) && !zrUtil.isArray(newVal)) {
-                newVal = newVal.value;
+            var _newVal;
+            if (zrUtil.isArray(newVal) || zrUtil.isObject(newVal)) {

Review comment:
       Thanks for your review and sorry for the wrong variable name defined before.
   
   The reason for making these changes is to ensure more compatibility at first.
   
   For the case: **Delete any data item in any line in GridView**
   
   In original logic context, it only handles the change of `value` but does not handle the change of `name`. Therefore, after any data item line is deleted, final merged result will ignore `name`.
   
   An example:
   **original data items:**
   ```js
   [
       { name:  'USA', value: 15276 },
       { name:  'UK', value: 4855 }
   ]
   ```
   
   **the first in original data items** 
   `{ name: 'USA', value: 15276 }`
   
   **the first data item after the old first data item was deleted(the second became the first)**
   `{  name:  "UK", value: 4855  }`
   
   After `tryMergeDataOption`, data items became
   `[ { name: 'USA', value: 4855 } ]`
   
   The result above is not obviously expected.
   Both `name` and `value` of each data item should be handled and merged well.
   
   By now, I think there should be some changes here.
   ```js
   function tryMergeDataOption(newData, originalData) {
       return zrUtil.map(newData, function (newVal, idx) {
           var original = originalData && originalData[idx];
           if (zrUtil.isObject(original) && !zrUtil.isArray(original)) {
               var newValIsObject = zrUtil.isObject(newVal) && !zrUtil.isArray(newVal);
               if (!newValIsObject) {
                   newVal = {
                       value: newVal
                   };
               }
               if (original.name != null && newVal.name == null) {
                   // original data has name but new data has no name
                   delete original.name;
               }
               // Original data has option
               return zrUtil.defaults(newVal, original);
           }
           else {
               return newVal;
           }
       });
   }
   ```
   
   Finally, I found another bug that it will occur after editing the series name in DataView.
   
   ![image](https://user-images.githubusercontent.com/26999792/81162277-7ef91500-8fbf-11ea-8784-f356ccd8e74b.png)
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org