Repository navigation
fix(explore): prevent duplicate onChange calls in RadioButtonControl (#44921) - #44945
FrancescoCastaldi wants to merge 5 commits into
Conversation
Code Review Agent Run #1f6470Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
…invocation (apache#44921) Signed-off-by: Francesco Castaldi <info@francescocastaldi.it>
…eady selected option (apache#44921) Signed-off-by: Francesco Castaldi <info@francescocastaldi.it>
…n selections (apache#44921) Signed-off-by: Francesco Castaldi <info@francescocastaldi.it>
… onClick handler (apache#44921) RadioButtonControl previously wired onChange both on the Radio.Group wrapper and in an onClick handler on each Radio.Button. Selecting an option triggered both handlers with the same value, causing duplicate updates, redundant renders, and duplicate Redux dispatches across Explore controls. Clicking an already-selected option also triggered onChange redundantly. Keep onChange handling exclusively on Radio.Group and retain focus management in the button onClick handler. Fixes: apache#44921 Signed-off-by: Francesco Castaldi <info@francescocastaldi.it>
…n tests (apache#44921) Signed-off-by: Francesco Castaldi <info@francescocastaldi.it>
7611488 to
6463eaf
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44945 +/- ##
=======================================
Coverage 82.28% 82.28%
=======================================
Files 2996 2996
Lines 185109 185115 +6
Branches 42834 42839 +5
=======================================
+ Hits 152316 152322 +6
Misses 30031 30031
Partials 2762 2762
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #9fbd8cActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
SUMMARY
RadioButtonControlpreviously wiredonChangein two places:<Radio.Group>container (onChange={e => onChange(e.target.value)})onClickhandler on each<Radio.Button>(onClick={e => { e.currentTarget?.focus(); onChange(val); }})Because of this duplicate wiring:
onChangetwice with the same value, causing redundant dispatches and re-renders in Explore controls (control panel, ColumnConfig form items, MapView, ZoomConfig).onChangethrough the button'sonClickeven though the selection did not change.This PR keeps
onChangehandling exclusively on<Radio.Group>and retains focus management (e.currentTarget?.focus()) in the buttononClickhandler without invokingonChange.TESTING INSTRUCTIONS
npm test -- packages/superset-ui-chart-controls/test/shared-controls/components/RadioButtonControl.test.tsxonChangeis called exactly once when clicking an unselected optiononChangeis not called when clicking an already-selected optionADDITIONAL INFORMATION