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. I've noticed
that when this PR proposed in 2019 the original return value is `boolean`, but
later I changed it to `BE_ORDINAL`. So that become 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. The strategy keep
not be changed for long time, 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. So any change to the basic data process code has to be
carefully.
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]
