You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "ump45nose (via GitHub)" <gi...@apache.org> on 2026/10/03 18:49:17 UTC
[PR] fix(explore): XAxisSortControl follows the value prop [superset]
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): XAxisSortControl follows the value prop [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #44956:
URL: https://github.com/apache/superset/pull/44956
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): XAxisSortControl follows the value prop [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #44956:
URL: https://github.com/apache/superset/pull/44956#issuecomment-5972361129
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(explore): XAxisSortControl follows the value prop [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #44956:
URL: https://github.com/apache/superset/pull/44956#issuecomment-5972487742
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/44956?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.29%. Comparing base ([`f2ff458`](https://app.codecov.io/gh/apache/superset/commit/f2ff458acde90662e0832a479a9b8b6db950a7d0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`010c962`](https://app.codecov.io/gh/apache/superset/commit/010c962cbfd6a76b2998148c8458ec6603a75b8d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #44956 +/- ##
=======================================
Coverage 82.29% 82.29%
=======================================
Files 2997 2997
Lines 185240 185239 -1
Branches 42867 42868 +1
=======================================
+ Hits 152434 152441 +7
+ Misses 30044 30036 -8
Partials 2762 2762
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/44956/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/44956/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.91% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/44956?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org