rusackas commented on PR #41637:
URL: https://github.com/apache/superset/pull/41637#issuecomment-4883700396
@yousoph the collision-scoping approach is sound... I traced it against
`resolveDragEnd` and scoping reorder collisions to same-context sortables is
exactly what keeps the dropzone from swallowing them. The new scoping tests are
solid too. Two things in the way though:
The CI failure is real and self-inflicted: `DndColumnMetricSelect.test.tsx`
("can drag and reorder items", ~line 228) still asserts the old adjacent-swap
order, so it fails against the new `arrayMove` semantics. The expectation
should become `['metric_a','column_b','column_a']`.
Bigger one: the standard `adhoc_filters` control is `DndFilterSelect` (via
the shared dnd controls), and its `onShiftOptions` still does an adjacent swap
with only `setValues`... no `onChange`, so filter reorder still won't persist
to form_data on the path nearly every chart uses. The PR only fixes the legacy
`AdhocFilterControl`. Mirroring the other handlers there (`arrayMove` +
`setValues` + `onChange`) looks like ~5 lines and squarely in scope of the PR's
claim.
Related-but-not-blocking: `DndColumnSelect` and `ContourControl` still use
swap semantics for non-adjacent drags (they do persist, so fine as a
follow-up), and legacy `MetricsControl` reorder was broken before and after...
worth a follow-up issue.
Happy to re-review once those two are in. Thanks!
--
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]
