concord-consortium / codap

CODAP (Common Online Data Analysis Platform)
MIT License
94 stars 38 forks source link

fix: disable vertical auto-scroll of table on attribute drag [PT-187949371] #1383

Closed kswenson closed 1 month ago

kswenson commented 1 month ago

[PT-187949371]

From the PT story:

The bug occurs when dragging an attribute near the bottom of a table triggers auto-scroll, which allows the column headers to be scrolled out of view and leaves a gap at the bottom of the table. The table must support auto-scroll horizontally when dragging an attribute, but should not auto-scroll vertically.

Auto-scrolling during drag is the province of DndKit, which supports several auto-scroll configuration options described in DndKit #140. In particular, the canScroll callback function is used to indicate particular elements that should not be auto-scrolled. None of the existing configuration options make it possible to support auto-scroll in one dimension only rather than enabling/disabling scrolling for all dimensions.

Therefore, we patch @dnd-kit/core to pass direction information to the canScroll callback function so that clients can enable/disable scrolling in each dimension separately. In addition, we implement a registry of such callback functions so that individual tiles can register their individual canScroll callbacks. The table uses this to allow horizontal auto-scroll but prevent vertical auto-scroll of the case table component, and to disable auto-scroll of the table body (grid cells) in both directions since there's nothing droppable to scroll to in the grid.

The @dndkit/core patch changes from this:

        if (canScroll?.(scrollContainer) === false) {  <= canScroll called with just an element
          continue;
        }

        const index = scrollableAncestors.indexOf(scrollContainer);
        const scrollContainerRect = scrollableAncestorRects[index];

        if (!scrollContainerRect) {
          continue;
        }

        const {direction, speed} = getScrollDirectionAndSpeed(
          scrollContainer,
          scrollContainerRect,
          rect,
          acceleration,
          threshold
        );

in which the canScroll function is only passed the element, to:

        const index = scrollableAncestors.indexOf(scrollContainer);
        const scrollContainerRect = scrollableAncestorRects[index];

        if (!scrollContainerRect) {
          continue;
        }

        const {direction, speed} = getScrollDirectionAndSpeed(
          scrollContainer,
          scrollContainerRect,
          rect,
          acceleration,
          threshold
        );

        if (canScroll?.(scrollContainer, direction) === false) {  <= canScroll() is passed a direction as well
          continue;
        }
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.75%. Comparing base (f5d06d0) to head (7c71737). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1383 +/- ## ========================================== + Coverage 85.63% 85.75% +0.12% ========================================== Files 522 523 +1 Lines 25582 25598 +16 Branches 6534 6976 +442 ========================================== + Hits 21906 21951 +45 + Misses 3522 3373 -149 - Partials 154 274 +120 ``` | [Flag](https://app.codecov.io/gh/concord-consortium/codap/pull/1383/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | Coverage Δ | | |---|---|---| | [cypress](https://app.codecov.io/gh/concord-consortium/codap/pull/1383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `72.59% <100.00%> (+0.14%)` | :arrow_up: | | [jest](https://app.codecov.io/gh/concord-consortium/codap/pull/1383/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `53.56% <83.87%> (+<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=concord-consortium#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cypress[bot] commented 1 month ago



Test summary

200 0 29 0Flakiness 2


Run details

Project codap-v3
Status Passed
Commit b80596be7b
Started Jul 31, 2024 3:13 PM
Ended Jul 31, 2024 3:22 PM
Duration 09:10 💡
OS Linux Ubuntu -
Browser Chrome 126

View run in Cypress Cloud ➡️


Flakiness

toolbar.spec.ts Flakiness
1 codap toolbar > will open a graph
formula/formula-component.spec.ts Flakiness
1 Formula Engine > Component Formula Tests > Formula in a new dataset

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud