Implement scroll feature on drag over the edges

[Zubair286]

1601

This PR implements feature of scrolling over the edges during a drag process. The toolbar and breadcrumb height is taken into account when dragging over the top and bottom edges.

[Zubair286]

I see, it wasn't working for me at all, so I assumed we had to start from scratch. Sorry for misunderstanding.

[raineorshine]

Sorry, that definitely wasn't clear! The feature label was from the original issue; I just changed it to bug. I re-opened the issue in May with this comment when it stopped working properly:

The basic functionality is in place, but there was a regression.

The implementation is done in the global touchmove event here:

https://github.com/cybersemics/em/blob/cca37c8f2a2e39aadb465d981b327fd6047ae430/src/util/initEvents.ts#L205-L228

Note that it currently only works on mobile. We will need to do a desktop implementation at some point, but that's a separate issue.

I'm open to other implementation strategies, but we should discuss the tradeoffs before jumping in. I imagine the hook-based approach you used has different advantages and disadvantages.

[Zubair286]

Alright, I get it. So this issue is for mobile only, right?

[raineorshine]

Correct. You may be able to reproduce with Chrome Developer Tools device toolbar, but I haven't tried.

[Zubair286]

It seems that as long as we are only scrolling the window, it works but setting the scrollContainer to either toolbar or sidebar is not working.

  const scroll = () => {
    const scrollLeft = document.documentElement.scrollLeft
    const scrollLeftNew = Math.max(0, scrollLeft + rate.x)
    const scrollTop = document.documentElement.scrollTop
    const scrollTopNew = Math.max(0, scrollTop + rate.y)

    // if we have hit the end, stop autoscrolling
    // i.e. if the next increment would not change scrollTop
    if (scrollLeftNew === scrollLeft && scrollTopNew === scrollTop) {
      autoscrolling = false
      return
    }

    window.scrollTo(scrollLeftNew, scrollTopNew)
    window.requestAnimationFrame(() => {
      if (autoscrolling) {
        scroll()
      }
    })
  }

[Zubair286]

Screencast from 08-29-2024 06:35:38 PM.webm

[raineorshine]

Okay, thanks for the update. Let me know what you come up with.

[Zubair286]

The problem I see is that, it is always finding the toolbar in all cases, hence the scrollTop value never changes because toolbar is not being scrolled. We can check for the visibility of the sidebar but for toolbar it wouldn't work since it is always there. is the toolbar in this case refer to the one in CustomizeToolbar or main one?

[raineorshine]

Drag-and-drop functionality exists in CustomizeToolbar only.

[raineorshine]

We need a clean, flexible solution. The current implementation's direct reference to the sidebar and toolbar is questionable.

One idea is to add a data-autoscroll attribute to the sidebar and the toolbar when it's in customize mode. Then we could query the closest data-autoscroll above e.target in the onTouchMove event. Log(n) should be fast enough to avoid performance issues, and then the code in initEvents would be completely agnostic to the drag-and-drop source.

[Zubair286]

Yes, don't see the reason to traverse the entire tree for an element on every touch event.

[raineorshine]

You can grab the element on dragstart.