lucapazz opened a new pull request, #38094: URL: https://github.com/apache/superset/pull/38094
### SUMMARY When interacting with **Time-series table** charts, clicking on a column header should sort the table based on that specific column. Currently, however, sorting always uses the values from the first column, regardless of which header is clicked. This makes it impossible to sort the table by any of the time series metric columns. #### Root Cause The problem originates in the `sortNumberWithMixedTypes` function. Specifically, it attempts to access the cell value using: `rowA.values[columnId].props['data-value']` However: - the `data-value` property does not exist; - the `rowA` object does not contain the values of its cells at all. As a result, the function ends up comparing undefined data, causing sorting to fallback to the first column. In addition, the function currently handles sort direction internally, flipping the sign of the comparison result if the order is descending. This behavior is not correct because the sort direction seems to be handled externally by the `react-table` component. #### What this PR fixes - Ensures that each `row` object exposes the cell values needed for sorting. - Corrects the way cell values are extracted during sorting, ensuring the comparison uses the actual metric values of the clicked column. - Removes direction-handling logic from `sortNumberWithMixedTypes`, so the function only performs a pure value comparison. - Restores the correct display of **null** values, which in previous versions were shown as "null", but are currently being converted to 0. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS 1. Create or open a **Time-series Table** chart. 2. Click on the header of a time series column to sort it - Expected behavior: it should sort rows numerically (ascending/descending) based on that column's values. 3. If present, you should see _null_ values when no data is available (in the sparkline they causes the line to be discontinuous rather than continuous). ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [x] Has associated issue: fixes #37892 - [ ] 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: [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]
