noodleslove opened a new pull request, #21773: URL: https://github.com/apache/echarts/pull/21773
<!-- Please fill in the following information to help us review your PR more efficiently. --> ## Brief Information This pull request is in the type of: - [x] bug fixing - [ ] new feature - [ ] others ### What does this PR do? Update the status of the toolbox brush icons as soon as the brushed areas are changed, and fix that the toolbox features are never updated or disposed. ### Fixed issues - #21771: Toolbox brush icon "Clear Selections" stays highlighted after being clicked ## Details ### Before: What was the problem? The highlight of the brush icon "Clear Selections" did not follow the brushed areas. With a brush component and the toolbox brush feature: | Step | Brushed areas | "Clear Selections" | | --- | --- | --- | | Brush an area | 1 | Not highlighted | | Any full update, e.g., click "Switch to Line Chart" | 1 | Highlighted | | Click "Clear Selections" | 0 | Still highlighted | It only went back to normal after the next full update. Two causes: 1. The status of the brush icons is only calculated in `render` and `updateView` of the brush feature. The action `brush` is registered with `update: 'updateVisual'`, which `ToolboxView` did not handle. So the icons were not updated when the areas were changed. When "Clear Selections" is clicked, `axisAreaSelect` is dispatched first, which triggers a full update while the areas still exist, and then `brush` with `command: 'clear'`, which only updates visual. That is why the icon stayed highlighted. 2. `ToolboxView._features` is a `HashMap`, but `updateView` and `dispose` iterated it with `zrUtil.each`, which only visits the internal `data` property. So both methods have done nothing since 6de824dc0. The features were never updated by `updateView`, and never disposed. <!-- ADD SCREENSHOT HERE IF APPLICABLE. --> ### After: How does it behave after the fixing? The icon "Clear Selections" is highlighted as soon as an area is brushed, and goes back to normal as soon as the areas are cleared. The other icons are not affected. The features are disposed when the chart is disposed. Changes in `src/component/toolbox/ToolboxView.ts`: - Iterate `_features` with its own `each` in `updateView` and `dispose`. - Add `updateVisual`, which delegates to `updateView`, so that the features can update the icon status after the actions that only update visual. - Apply the icon status by `api.enterEmphasis` / `api.leaveEmphasis` instead of the functions in `util/states`, so that the states are applied in the next frame even if the status is changed out of a full update. <!-- ADD SCREENSHOT HERE IF APPLICABLE. --> ## Document Info One of the following should be checked. - [x] This PR doesn't relate to document changes - [ ] The document should be updated later - [ ] The document changes have been made in apache/echarts-doc#xxx ## Misc ### Security Checking - [ ] This PR uses security-sensitive Web APIs. <!-- PLEASE CHECK IT AGAINST: <https://github.com/apache/echarts/wiki/Security-Checklist-for-Code-Contributors> --> ### ZRender Changes - [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx). ### Related test cases or examples to use the new APIs - `test/toolbox-brush-iconStatus.html` - `test/ut/spec/component/toolbox/brushIconStatus.test.ts`. All of the four cases fail without the fix. ### Merging options - [x] Please squash the commits into a single one when merging. ### Other information - Targets `release` as requested in #21771. - The first change (iterating `_features`) is the same as #21684, which targets `master` and is not merged yet. It is required here, since `release` does not have it and `updateVisual` relies on `updateView`. Please feel free to decide how to reconcile the two. - I have another pull request in preparation (`select.iconStyle` for toolbox icons, for #21672, targeting `master`) that changes the same lines of `ToolboxView.ts`. Whichever is merged later will need a small conflict resolution; I will handle it. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
