You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2020/04/24 02:53:47 UTC

[GitHub] [incubator-echarts] Ovilia opened a new pull request #12484: (WIP) feat: bar race

Ovilia opened a new pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484


   <!-- Please fill in the following information to help us review your PR more efficiently. -->
   
   ## Brief Information
   
   This pull request is in the type of:
   
   - [ ] bug fixing
   - [x] new feature
   - [ ] others
   
   
   
   ### What does this PR do?
   
   <!-- USE ONCE SENTENCE TO DESCRIBE WHAT THIS PR DOES. -->
   
   
   
   ### Fixed issues
   
   <!--
   - #xxxx: ...
   -->
   
   
   ## Details
   
   ### Before: What was the problem?
   
   <!-- DESCRIBE THE BUG OR REQUIREMENT HERE. -->
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   
   
   ### After: How is it fixed in this PR?
   
   <!-- THE RESULT AFTER FIXING AND A SIMPLE EXPLANATION ABOUT HOW IT IS FIXED. -->
   
   <!-- ADD SCREENSHOT HERE IF APPLICABLE. -->
   
   
   
   ## Usage
   
   ### Are there any API changes?
   
   - [ ] The API has been changed.
   
   <!-- LIST THE API CHANGES HERE -->
   
   
   
   ### Related test cases or examples to use the new APIs
   
   NA.
   
   
   
   ## Others
   
   ### Merging options
   
   - [ ] Please squash the commits into a single one when merge.
   
   ### Other information
   


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] plainheart edited a comment on pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
plainheart edited a comment on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-654088597


   @Ovilia Hi, thank you for your excellent work for bar race. Here I'd like to give some feedback for this new feature, please feel free to check and evaluate it.
   Normally, if the value of bar doesn't change, the value of its label should not change and if changed, it should count from its previous newest value, but in fact, the value of bar label will always count from 0 when updating, just like the screenshot below:
   
   ![unexpected](https://user-images.githubusercontent.com/26999792/86570584-bb86c400-bfa2-11ea-89ac-390ffd97b4f6.gif)
   
   What I think it's expected is like this:
   
   ![expected](https://user-images.githubusercontent.com/26999792/86570612-c5a8c280-bfa2-11ea-8373-c198629a9814.gif)
   
   That is,
   1). The label of bar has no need to recount from 0 if its value has no change.
   2). If its value changed, it should recount from its previous value, like the screenshot above.
   
   What do you think of these?
   
   EDITED: I saw it looks like working well in the screenshot you provided in PR description.


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-647396940


   Congratulations! Your PR has been merged. Thanks for your contribution! 👍


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] Ovilia edited a comment on pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
Ovilia edited a comment on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-654147867


   Yes, it should change from previous values. I think there might be a bug with this. Not sure about it. I will check it later.
   This feature is not completed yet, I'm still working on show the top 10 bars with more data. 
   Thanks for your feedback! I will at you once I've completed the others.


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] echarts-bot[bot] commented on pull request #12484: (WIP) feat: bar race

Posted by GitBox <gi...@apache.org>.
echarts-bot[bot] commented on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-618773164


   Thanks for your contribution!
   The community will review it ASAP. In the meanwhile, please checkout [the coding standard](https://echarts.apache.org/en/coding-standard.html) and Wiki about [How to make a pull request](https://github.com/apache/incubator-echarts/wiki/How-to-make-a-pull-request).


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] Ovilia commented on a change in pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
Ovilia commented on a change in pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#discussion_r443370353



##########
File path: src/coord/cartesian/AxisModel.ts
##########
@@ -35,6 +36,10 @@ interface CartesianAxisOption extends AxisBaseOption {
     position?: CartesianAxisPosition;
     // Offset is for multiple axis on the same position.
     offset?: number;
+    sort?: boolean;
+    realtimeSort?: boolean;
+    sortSeriesIndex?: number;

Review comment:
       Leave this as it is for now.

##########
File path: src/scale/Ordinal.ts
##########
@@ -99,6 +103,23 @@ class OrdinalScale extends Scale {
         return;
     }
 
+    setCategorySortInfo(info: OrdinalSortInfo[]): void {
+        this._categorySortInfo = info;
+    }
+

Review comment:
       It's not implemented yet. It will be fixed later.




----------------------------------------------------------------
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

[GitHub] [incubator-echarts] pissang commented on a change in pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
pissang commented on a change in pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#discussion_r442009191



##########
File path: src/chart/helper/labelHelper.ts
##########
@@ -20,22 +20,27 @@
 
 import {retrieveRawValue} from '../../data/helper/dataProvider';
 import List from '../../data/List';
+import {ParsedValue} from '../../util/types';
 
 /**
  * @return label string. Not null/undefined
  */
-export function getDefaultLabel(data: List, dataIndex: number): string {
+export function getDefaultLabel(
+    data: List,
+    dataIndex: number,
+    interpolatedValues?: ParsedValue | ParsedValue[]
+): string {
     const labelDims = data.mapDimensionsAll('defaultedLabel');
     const len = labelDims.length;
 
     // Simple optimization (in lots of cases, label dims length is 1)
     if (len === 1) {
-        return retrieveRawValue(data, dataIndex, labelDims[0]);
+        return retrieveRawValue(data, dataIndex, labelDims[0], interpolatedValues);
     }

Review comment:
       Perhaps it's more reasonable to be:
    
   ```js
   return interpolatedValues == null ? retrieveRawValue(data, dataIndex, labelDims[0]) : interpolatedValues
   ```

##########
File path: src/scale/Ordinal.ts
##########
@@ -99,6 +103,23 @@ class OrdinalScale extends Scale {
         return;
     }
 
+    setCategorySortInfo(info: OrdinalSortInfo[]): void {
+        this._categorySortInfo = info;
+    }
+

Review comment:
       I can't find the general logic of sorting the categories when `realtimeSort` is not set. Did I miss it?

##########
File path: src/coord/cartesian/Grid.ts
##########
@@ -447,6 +462,36 @@ class Grid implements CoordinateSystemMaster {
                 );
             });
         }
+
+        // function sortCategory(
+        //     data: List,

Review comment:
       Remove the commented code if they are not used anymore

##########
File path: src/model/mixin/dataFormat.ts
##########
@@ -96,13 +99,14 @@ class DataFormatMixin {
         status?: DisplayState,
         dataType?: string,
         dimIndex?: number,
-        labelProp?: string
+        labelProp?: string,
+        interpolateValues?: ParsedValue | ParsedValue[]
     ): string {
         status = status || 'normal';
         const data = this.getData(dataType);
         const itemModel = data.getItemModel(dataIndex);
 
-        const params = this.getDataParams(dataIndex, dataType);
+        const params = this.getDataParams(dataIndex, dataType, null, interpolateValues);
         if (dimIndex != null && (params.value instanceof Array)) {

Review comment:
       I think it's better to modify the `params.value` after getDataParams instead of adding an extra parameter.
   
   Other properties may also have a chance to be modified in the future. 

##########
File path: src/chart/bar/BarView.ts
##########
@@ -316,7 +423,84 @@ class BarView extends ChartView {
         }
     }
 
-    remove(ecModel?: GlobalModel): void {
+    _dataSort(
+        data: List<BarSeriesModel, DefaultDataVisual>,
+        map: ((idx: number) => number)
+    ): OrdinalSortInfo[] {
+        type SortValueInfo = {
+            mappedValue: number,
+            ordinalNumber: OrdinalNumber,
+            beforeSortIndex: number
+        };
+        const info: SortValueInfo[] = [];
+        data.each(idx => {
+            info.push({
+                mappedValue: map(idx),
+                ordinalNumber: idx,
+                beforeSortIndex: null
+            });
+        });
+
+        info.sort((a, b) => {
+            return b.mappedValue - a.mappedValue;
+        });
+
+        // Update beforeSortIndex
+        for (let i = 0; i < info.length; ++i) {
+            info[info[i].ordinalNumber].beforeSortIndex = i;
+        }
+        return info.map(item => {
+            return {

Review comment:
       Use `zrUtil.map` instead of ES5 map

##########
File path: src/coord/cartesian/AxisModel.ts
##########
@@ -35,6 +36,10 @@ interface CartesianAxisOption extends AxisBaseOption {
     position?: CartesianAxisPosition;
     // Offset is for multiple axis on the same position.
     offset?: number;
+    sort?: boolean;
+    realtimeSort?: boolean;
+    sortSeriesIndex?: number;

Review comment:
       Still not very show if it's better to put `sort` `realtimeSort` configuration on the series. Perhaps we can let developers do the decision.




----------------------------------------------------------------
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

[GitHub] [incubator-echarts] Ovilia merged pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
Ovilia merged pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484


   


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] Ovilia commented on pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
Ovilia commented on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-654147867


   Yes, it should change from previous values. I think there might be a bug with this. Not sure about it. I will check it later.
   This feature is not completed yet, I'm still working on show the top 10 bars with more data. 


----------------------------------------------------------------
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

[GitHub] [incubator-echarts] plainheart commented on pull request #12484: feat: bar race

Posted by GitBox <gi...@apache.org>.
plainheart commented on pull request #12484:
URL: https://github.com/apache/incubator-echarts/pull/12484#issuecomment-654088597


   @Ovilia Hi, thank you for your excellent work for bar race. Here I'd like to give some feedback for this new feature, please feel free to check and evaluate it.
   Normally, if the value of bar doesn't change, the value of its label should not change and if changed, it should count from its previous newest value, but in fact, the value of bar label will always count from 0 when updating, just like the screenshot below:
   
   ![unexpected](https://user-images.githubusercontent.com/26999792/86570584-bb86c400-bfa2-11ea-89ac-390ffd97b4f6.gif)
   
   What I think it's expected is like this:
   
   ![expected](https://user-images.githubusercontent.com/26999792/86570612-c5a8c280-bfa2-11ea-8373-c198629a9814.gif)
   
   That is,
   1). The label of bar has no need to recount from 0 if its value has no change.
   2). If its value changed, it should recount from its previous value, like the screenshot above.
   
   What do you think of these?


----------------------------------------------------------------
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