Skip to content

fix bug #11049#11212

Closed
yufeng04 wants to merge 1 commit into
apache:masterfrom
yufeng04:bug11049
Closed

fix bug #11049#11212
yufeng04 wants to merge 1 commit into
apache:masterfrom
yufeng04:bug11049

Conversation

@yufeng04
Copy link
Copy Markdown
Contributor

The toolbox has been updated when using the dataZoom in dynamic added toolbox, but the dataZoom hasn't been updated.
图片

@pissang pissang requested a review from 100pah September 11, 2019 09:05

if (toolboxOpt && toolboxOpt.feature) {
var dataZoomOpt = toolboxOpt.feature.dataZoom;
var dataZoomOpt = toolboxOpt.feature.dataZoom || {};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Add a default {} will make the condition at L281 no longer make sense. So only add a {} is not enough.

  2. Add a defualt {} will cause that dataZoom models are always added whatever toolbox.featrue.dataZoom declared. I am worried that might bring some other bug or bad cases.
    And event we fix this error throw in this way, there are other issue: if setOption changes the axes, the toolbox dataZoom will not follow and will also be buggy.

I think this issue is a little complicated:
Currently, making a hidden dataZoom model is performed in the preprocess stage,
which can not take the existing options into account. But they need that, because
they need the axes info.
If we want to resolve this kind of issue thoroughly, we might need another stage,
perhaps in setOption(the second round of setOption, only triggered inside),
to insert/update this kind of hidden model, where the existing model (like axes)
can be taken into account.

Anyway, before we resolve it totally, we can probably consider the feature toolbox dataZoom
only support being used in the first setOption.

@github-actions
Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR. We are sorry for this but 2 years is a long time and the code base has been changed a lot. Thanks for your contribution anyway.

@github-actions github-actions Bot added the stale Inactive for a long time. Will be closed in 7 days. label Sep 24, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 1, 2021

This PR has been automatically closed because it has not had recent activity. Sorry for that and we are looking forward to your next contribution.

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

Labels

stale Inactive for a long time. Will be closed in 7 days.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants