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): fix highlight state after inverseselect #11480 #11547

Merged
merged 1 commit into from Nov 20, 2019
Merged

fix(legend): fix highlight state after inverseselect #11480 #11547

merged 1 commit into from Nov 20, 2019

Conversation

SnailSword
Copy link
Contributor

@SnailSword SnailSword commented Nov 1, 2019

Item should be downplayed before unselecting.

@pissang pissang merged commit 1df68e7 into apache:master Nov 20, 2019
@@ -238,7 +238,7 @@ export default echarts.extendComponentView({
);

// FIXME: consider different series has items with the same name.
itemGroup.on('click', curry(dispatchSelectAction, name, api))
itemGroup.on('click', curry(dispatchSelectAction, name, null, api, excludeSeriesId))
Copy link
Member

@100pah 100pah Nov 20, 2019

Choose a reason for hiding this comment

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

The name here is "item name", but in dispatchHighlightAction and dispatchDownplayAction the first parameter is seriesName.

Copy link
Contributor

@pissang pissang Nov 21, 2019

Choose a reason for hiding this comment

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

The name here can be seriesName or itemName. It depends on series.

api.dispatchAction({
type: 'legendToggleSelect',
name: name
});
// highlight after select
dispatchHighlightAction(name, dataName, api, excludeSeriesId);
Copy link
Member

@100pah 100pah Nov 20, 2019

Choose a reason for hiding this comment

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

This highlight action probably is not needed.
Otherwise if we call

chart.dispatchAction({
    type: 'legendToggleSelect',
    name: 'b'
});

manually, there are still unexpected highlight state.

Copy link
Contributor

@pissang pissang Nov 21, 2019

Choose a reason for hiding this comment

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

I also had concerns about this.
But after run the test case. It seems to be a resonable behaviour.

100pah added a commit that referenced this pull request Nov 21, 2019
Currently the highlight state is stored in el (maybe it is inappropriate
but that is another big topic). The el in oldData may be already removed
prevoiusly but still has highlight state, which is unexpected. We simply
do not reuse those el (detected by `!symbolEl.parent`).
Check the test case in `main0` of `test/hoverStyle2.html`.
100pah added a commit that referenced this pull request Nov 21, 2019
@100pah 100pah mentioned this pull request Nov 21, 2019
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.

None yet

4 participants