100pah commented on a change in pull request #10221: Fix #10210 wrong color of
piecewise visualMap with scatter charts
URL:
https://github.com/apache/incubator-echarts/pull/10221#discussion_r386256790
##########
File path: src/data/helper/sourceHelper.js
##########
@@ -664,14 +664,12 @@ function doGuessOrdinal(
}
function detectValue(val) {
- var beStr = isString(val);
- // Consider usage convenience, '1', '2' will be treated as "number".
- // `isFinit('')` get `true`.
- if (val != null && isFinite(val) && val !== '') {
- return beStr ? BE_ORDINAL.Might : BE_ORDINAL.Not;
- }
- else if (beStr && val !== '-') {
- return BE_ORDINAL.Must;
+ // For string, assign its type to 'ordinal'
+ if (isString(val)) {
+ return true;
+ }
+ else if (val != null && isNaN(val)) {
+ return false;
Review comment:
1. The original return type of `detectValue` is the enumerable value
`BE_ORDINAL`. But here the return value is changed to boolean. That is not
correct.
2. The original strategy of "guess ordinal" that treat string like number as
number is a historical setting since echarts2. The strategy has pros and cons.
It makes the logic complicated but follow the thought of "guess" which might
not precise but lower the barriers for lots of senior users. No matter the the
strategy is good or not, any change to the strategy should be carefully,
because it will bring break changes, which will impact lots of users and
prevent them from upgrading echarts to new version to resolve some the issues.
3. This feature is mainly about `visualMap`. I guess the fix can be
restricted in `visualMap` but not in the basic module `sourceHelper.js`.
Changing a basic module will cause broad implications and need to think
thoroughly and fully test.
The possible way about this fix I will post below later today ...
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
