ump45nose opened a new pull request, #44956:
URL: https://github.com/apache/superset/pull/44956
### SUMMARY
`XAxisSortControl` copied `props.value` into local state on first render
(`const [value, setValue] = useState(props.value)`) and always passed that copy
to `SelectControl` (`value={value}`). Its only `useEffect` ran on
`[props.shouldReset, props.value]` but acted only when `shouldReset` was true,
so a later change to `props.value` that does not remount the control never
reached the render.
Direct selections still appeared to work because antd's inner `Select` keeps
its own state, which masked the problem in the common path.
This drops the local copy and reads `props.value` directly, substituting
`undefined` only while `shouldReset` is set (that is the value the reset path
has to clear). `props.onChange(undefined)` in the effect is unchanged, so the
reset behaviour the report describes still holds.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: this is a state-propagation fix with no visual change, and
it was reported at the component level with the UI repro not verified by the
reporter either.
### TESTING INSTRUCTIONS
Unit tests (component level, following the repro in the issue):
1. `XAxisSortControl.test.tsx` (new):
- renders the value it was given
- follows the `value` prop when it changes
- clears the value when `shouldReset` turns true
**Verified**
- `XAxisSortControl.test.tsx` fails on `master` with exactly the reported
symptom and passes with the fix (confirmed by stashing the fix and re-running:
`follows the value prop when it changes` fails).
- `npx jest src/explore/components/controls --silent` — 85 suites, 896
tests, all passing.
- `npx jest src/explore/components/ControlPanels
src/explore/components/Property --silent` — 1 suite, 23 tests passing.
- `npx oxlint --config oxlint.json --quiet` on both changed files — clean.
**Not verified**
- The Explore view in a browser: needs a running Superset instance and a
chart config with the X-axis sort control, plus a way to push a new `form_data`
value without remounting the control, which is what triggers it. I did not run
this, so the UI-level repro from the issue remains unconfirmed by me.
Note: `npx tsc --noEmit` reports 608 pre-existing `TS6305` errors ("Output
file ... has not been built") in `plugins/preset-chart-deckgl` on a clean
checkout; none are in the files I touched.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #44918
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
