From 787320b8ac5774ca0aee270c9da1db0decd058ae Mon Sep 17 00:00:00 2001 From: Eddie Ho Date: Wed, 30 Sep 2026 12:13:19 -0700 Subject: [PATCH] fix(toolbox): (1) Fix that the icon status of brush is not updated when the brushed areas are changed, e.g., the icon "clear" stays highlighted after clicked. (2) Fix that the features are not updated and disposed, since the HashMap of features is iterated incorrectly. close #21771 --- src/component/toolbox/ToolboxView.ts | 27 ++- test/toolbox-brush-iconStatus.html | 110 ++++++++++++ .../component/toolbox/brushIconStatus.test.ts | 163 ++++++++++++++++++ 3 files changed, 295 insertions(+), 5 deletions(-) create mode 100644 test/toolbox-brush-iconStatus.html create mode 100644 test/ut/spec/component/toolbox/brushIconStatus.test.ts diff --git a/src/component/toolbox/ToolboxView.ts b/src/component/toolbox/ToolboxView.ts index 8b7c82421a..08d363fe92 100644 --- a/src/component/toolbox/ToolboxView.ts +++ b/src/component/toolbox/ToolboxView.ts @@ -19,7 +19,6 @@ import * as textContain from 'zrender/src/contain/text'; import * as graphic from '../../util/graphic'; -import { enterEmphasis, leaveEmphasis } from '../../util/states'; import Model from '../../model/Model'; import DataDiffer from '../../data/DataDiffer'; import * as listComponentHelper from '../helper/listComponent'; @@ -169,7 +168,7 @@ class ToolboxView extends ComponentView { option.iconStatus = option.iconStatus || {}; option.iconStatus[iconName] = status; if (iconPaths[iconName]) { - (status === 'emphasis' ? enterEmphasis : leaveEmphasis)(iconPaths[iconName]); + applyIconStatus(api, iconPaths[iconName], status); } }; @@ -297,7 +296,7 @@ class ToolboxView extends ComponentView { } textContent.hide(); }); - (featureModel.get(['iconStatus', iconName]) === 'emphasis' ? enterEmphasis : leaveEmphasis)(path); + applyIconStatus(api, path, featureModel.get(['iconStatus', iconName])); group.add(path); (path as graphic.Path).on('click', bind( @@ -377,7 +376,8 @@ class ToolboxView extends ComponentView { api: ExtensionAPI, payload: unknown ) { - each(this._features, function (feature) { + // `_features` is a HashMap, which can not be iterated by the util `each`. + this._features && this._features.each(function (feature) { feature && feature instanceof ToolboxFeature && feature.updateView @@ -385,8 +385,19 @@ class ToolboxView extends ComponentView { }); } + updateVisual( + toolboxModel: ToolboxModel, + ecModel: GlobalModel, + api: ExtensionAPI, + payload: Payload + ) { + // The icon status may depend on the result of the actions that only update visual. + // For example, the action `brush` changes the brushed areas, which the icon `clear` depends on. + this.updateView(toolboxModel, ecModel, api, payload); + } + dispose(ecModel: GlobalModel, api: ExtensionAPI) { - each(this._features, function (feature) { + this._features && this._features.each(function (feature) { feature && feature instanceof ToolboxFeature && feature.dispose @@ -396,6 +407,12 @@ class ToolboxView extends ComponentView { } +function applyIconStatus(api: ExtensionAPI, iconPath: IconPath, status: DisplayState | NullUndefined): void { + // Use the methods of `api` to make sure that the states are applied in the next frame, + // even if the status is changed out of a full update. + status === 'emphasis' ? api.enterEmphasis(iconPath) : api.leaveEmphasis(iconPath); +} + function isUserFeatureName(featureName: string): boolean { return featureName.indexOf('my') === 0; } diff --git a/test/toolbox-brush-iconStatus.html b/test/toolbox-brush-iconStatus.html new file mode 100644 index 0000000000..ece4bf6a9b --- /dev/null +++ b/test/toolbox-brush-iconStatus.html @@ -0,0 +1,110 @@ + + + + + + + + + + + + + + + + + + + + + +
+ + + + + + + + + + + diff --git a/test/ut/spec/component/toolbox/brushIconStatus.test.ts b/test/ut/spec/component/toolbox/brushIconStatus.test.ts new file mode 100644 index 0000000000..c43993659d --- /dev/null +++ b/test/ut/spec/component/toolbox/brushIconStatus.test.ts @@ -0,0 +1,163 @@ + +/* +* Licensed to the Apache Software Foundation (ASF) under one +* or more contributor license agreements. See the NOTICE file +* distributed with this work for additional information +* regarding copyright ownership. The ASF licenses this file +* to you under the Apache License, Version 2.0 (the +* "License"); you may not use this file except in compliance +* with the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, +* software distributed under the License is distributed on an +* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +* KIND, either express or implied. See the License for the +* specific language governing permissions and limitations +* under the License. +*/ + +import { createChart, getECModel } from '../../../core/utHelper'; +import { EChartsType } from '../../../../../src/echarts'; +import { ToolboxComponentOption } from '../../../../../src/export/option'; +import ToolboxView from '../../../../../src/component/toolbox/ToolboxView'; +import { ToolboxFeature } from '../../../../../src/component/toolbox/featureManager'; +import BrushModel from '../../../../../src/component/brush/BrushModel'; +import { HOVER_STATE_EMPHASIS, HOVER_STATE_NORMAL } from '../../../../../src/util/states'; +import { ECElement } from '../../../../../src/util/types'; +import Path from 'zrender/src/graphic/Path'; +import { ElementEvent } from 'zrender/src/Element'; + + +describe('toolbox_brushIconStatus', function () { + + let chart: EChartsType; + beforeEach(function () { + chart = createChart(); + }); + + afterEach(function () { + chart.dispose(); + }); + + function setOption(toolbox: ToolboxComponentOption): void { + chart.setOption({ + animation: false, + toolbox: toolbox, + brush: {xAxisIndex: 0}, + xAxis: {type: 'category', data: ['a', 'b', 'c']}, + yAxis: {}, + series: [{type: 'bar', data: [1, 2, 3]}] + }); + } + + // The icon paths are re-created whenever toolbox is re-rendered. Always fetch them when needed. + function getFeature(featureName: string): ToolboxFeature { + const toolboxModel = getECModel(chart).getComponent('toolbox'); + // @ts-ignore + const toolboxView = chart._componentsMap[toolboxModel.__viewId] as ToolboxView; + return toolboxView._features.get(featureName) as ToolboxFeature; + } + + function getIcon(featureName: string, iconName: string): Path & ECElement { + return getFeature(featureName).model.iconPaths[iconName] as Path & ECElement; + } + + function trigger(icon: Path, eventName: 'click' | 'mouseover' | 'mouseout'): void { + icon.trigger(eventName, {} as ElementEvent); + } + + function click(featureName: string, iconName: string): void { + trigger(getIcon(featureName, iconName), 'click'); + } + + function getBrushedAreaCount(): number { + return (getECModel(chart).getComponent('brush') as BrushModel).areas.length; + } + + function brushArea(): void { + // The action `brush` does not trigger a full update. + chart.dispatchAction({ + type: 'brush', + areas: [{ + xAxisIndex: 0, + brushType: 'lineX', + coordRange: [0, 1] + }] + }); + } + + function expectHighlighted(featureName: string, iconName: string, highlighted: boolean): void { + // The states changed out of a full update are applied in the next frame. + chart.getZr().animation.update(); + + const icon = getIcon(featureName, iconName); + expect(icon.hoverState || HOVER_STATE_NORMAL).toEqual( + highlighted ? HOVER_STATE_EMPHASIS : HOVER_STATE_NORMAL + ); + expect(icon.currentStates).toEqual(highlighted ? ['emphasis'] : []); + } + + it('should_highlight_clear_as_soon_as_an_area_is_brushed', function () { + setOption({feature: {brush: {}}}); + expectHighlighted('brush', 'clear', false); + + brushArea(); + expect(getBrushedAreaCount()).toEqual(1); + expectHighlighted('brush', 'clear', true); + + chart.setOption({}); + expectHighlighted('brush', 'clear', true); + }); + + it('should_reset_clear_as_soon_as_the_areas_are_cleared', function () { + setOption({feature: {brush: {}}}); + brushArea(); + expectHighlighted('brush', 'clear', true); + + trigger(getIcon('brush', 'clear'), 'mouseover'); + click('brush', 'clear'); + // The icons are re-created by the click, since the action `axisAreaSelect` triggers a full update. + trigger(getIcon('brush', 'clear'), 'mouseout'); + + expect(getBrushedAreaCount()).toEqual(0); + expectHighlighted('brush', 'clear', false); + + chart.setOption({}); + expectHighlighted('brush', 'clear', false); + }); + + it('should_keep_the_status_of_the_other_icons_when_clear_is_clicked', function () { + setOption({feature: {brush: {}}}); + + click('brush', 'rect'); + click('brush', 'keep'); + brushArea(); + click('brush', 'clear'); + + expectHighlighted('brush', 'rect', true); + expectHighlighted('brush', 'keep', true); + expectHighlighted('brush', 'polygon', false); + expectHighlighted('brush', 'clear', false); + }); + + it('should_dispose_the_features_when_the_chart_is_disposed', function () { + setOption({feature: {dataZoom: {}, brush: {}}}); + + const dataZoomFeature = getFeature('dataZoom'); + const originalDispose = dataZoomFeature.dispose; + let disposeCount = 0; + dataZoomFeature.dispose = function (ecModel, api) { + disposeCount++; + originalDispose.call(this, ecModel, api); + }; + + chart.dispose(); + expect(disposeCount).toEqual(1); + + // For `afterEach`. + chart = createChart(); + }); + +});