Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(legend): legend of pie/funnel/radar can pick from visualMap encoded color #11737

Merged
merged 4 commits into from
Dec 1, 2019

Conversation

pissang
Copy link
Contributor

@pissang pissang commented Nov 28, 2019

More technique discussion can be seen in #10766 (review)

radar2-full-shot-actual

legend-visualMapColor-full-shot-actual

@pissang pissang changed the title fix(legend): legend of pie/funnel can pick from visualMap encoded color … fix(legend): legend of pie/funnel can pick from visualMap encoded color Nov 28, 2019
@pissang pissang changed the title fix(legend): legend of pie/funnel can pick from visualMap encoded color fix(legend): legend of pie/funnel/radar can pick from visualMap encoded color Nov 28, 2019
100pah
100pah previously approved these changes Nov 30, 2019
return;
}

var color = data.getItemVisual(idx, 'color');
var borderColor = data.getItemVisual(idx, 'borderColor');
var idx = provider.indexOfName(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provider.indexOfName seems to have covered the functionality of provider.containName.
Why we need provider.containName and do the O(n) search twice?

"01",
"高档",
61278,
"61278.00"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to obfuscate the data in case some unexpected legal issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be

}
}
]
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to add (or already have?) a test case like #2372 described (I know it is correct both before and after this change).
Like:

option = {
    legend: {
    },
    series: {
        type: 'pie',
        itemStyle: {
            color: function (params) {
                var colorList = ['#ff3322', '#232211', '#aabbcc'];
                return colorList[params.dataIndex];
            }
        },
        data: [
            {name: 'a', value: 1222},
            {name: 'b', value: 2333},
            {name: 'c', value: 3444},
        ]
    }
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants