amaannawab923 opened a new pull request, #33540: URL: https://github.com/apache/superset/pull/33540
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ###🔧 Summary This PR improves the user experience in the Table Chart control panel by addressing a confusing interaction between the "Sort Query By" and "Sort Descending" controls. ###🐛 The Issue If the user doesn't select a "Sort Query By" metric but toggles the "Sort Descending" checkbox, it appears to have no effect on the results. The server pagination controls sit between the "Sort Query By" and "Sort Descending" controls, making them feel unrelated. This leads users to believe that "Sort Descending" should work independently—which it doesn’t. ###🧠 Root Cause When building the table query: if (sortByMetric) { orderby = [[sortByMetric, !orderDesc]]; } else if (metrics?.length > 0) { orderby = [[metrics[0], false]]; } If sortByMetric is provided, it uses the selected metric and the user's "Sort Descending" choice. If not, it defaults to the first metric with false (which means descending)—regardless of the "Sort Descending" checkbox state. This causes confusion: the user expects their input to be respected, but it isn’t when no "Sort By" metric is selected. ### ✅ The Fix Move the "Sort Descending" checkbox directly below the "Sort Query By" control to visually group them and clarify their relationship. Only show the "Sort Descending" checkbox when a "Sort Query By" metric is selected (i.e., when timeseries_limit_metric has a value). This makes the UI more intuitive and prevents misleading interactions. 🎯 ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
