Skip to content

Grid rows not wrapping when element is dragged up first column #617

Open
@clara-tsang

Description

@clara-tsang

I'm currently getting the UI bug as shown below:
ezgif com-video-to-gif

The rows are being pushed outside the grid instead of wrapping as it should.

Perhaps I haven't implemented the grid correctly - I've made the

  • elements display: inline-block and set the axis to 'xy'.

    Here's a demo made from the starter codesandbox.

  • Activity

    clara-tsang

    clara-tsang commented on Oct 30, 2019

    @clara-tsang
    Author

    Prematurely closed this... any ideas?

    I've found that this.containerBoundingRect.width seems to be returning the window width instead of the div containing the sortable elements; however, everything seems to be working in the demo.

    alentame

    alentame commented on Oct 30, 2019

    @alentame

    hi! where can i find the code for the demo on the website ? thanks!

    clara-tsang

    clara-tsang commented on Oct 30, 2019

    @clara-tsang
    Author

    @alentame Ah yes, I couldn't find that either which made it harder to debug :P

    Figured it out, though - turns out the code finds the width of the container by looking for the first 'scrollable' parent of the sortable elements. Because my container wasn't scrollable, it bubbled up to grab the width of the window. I added overflow: auto to my container and the issue was solved.

    I couldn't find anything in the docs about this issue, and I think it's worth adding into the README in the FAQ section.

    sarahzinger

    sarahzinger commented on Dec 6, 2019

    @sarahzinger

    Hello!

    We're experiencing a similar thing on glitch.com right now and I can't quite figure out why:

    I'm not entirely sure what's going on, but it seems related to the width of the container.

    I don't have a codepen but I did find that I could recreate the bug when using chrome dev tools on the storybook example. If I change the width of the container from 520px to 500px you see that items move to the right, when they should wrap:

    I tried the overflow:auto solution but it didn't help in this instance. Any help would be greatly appreciated thanks so much!

    clara-tsang

    clara-tsang commented on Dec 6, 2019

    @clara-tsang
    Author

    @sarahzinger Which element are you adding overflow: auto to? It should be the first parent element of the sortable elements. If that didn't help, I've found that setting relative widths -width: 25% seems to work more reliably... The closer that the elements fill up the container, the better.

    It does seem to be a bug, though, since the logic checks whether adding a column of elements on the right would surpass the right edge of the container. Hope this can help you a bit!

    sarahzinger

    sarahzinger commented on Dec 6, 2019

    @sarahzinger

    @clara-tsang thanks for the response! I was adding overflow: auto using chrome dev tools to the grid element and it was broken, but I just added it to our code directly and it looks like that might have worked! Thanks so much for your help!!

    Here's what I saw going on if it's helpful for future folks:
    It looks like this line was not consistently getting set to true for the right-hanging item in the grid when it should be

    if (
    edgeOffset.left + translate.x >
    this.containerBoundingRect.width - offset.width
    ) {

    it turned out that this.containerBoundingRect.width was getting set to the window width. We set bounding client to be the scrollContainer

    const containerBoundingRect = this.scrollContainer.getBoundingClientRect();

    And the scroll container takes our grid's container and then calls getScrollingParent on it
    this.scrollContainer = useWindowAsScrollContainer
    ? this.document.scrollingElement || this.document.documentElement
    : getScrollingParent(this.container) || this.container;

    which will move farther and farther up the dom until it hits the window.

    I'm not sure if or why this.containerBoundingRect needs to be set to a scrollable container rather than to the container directly. But if it does I think it'd make sense to either auto add a scroll to the container within the library or potentially just document that grids need to have overflow: auto set.

    Thanks again for the help!

    sarahzinger

    sarahzinger commented on Jan 2, 2020

    @sarahzinger

    Oh another update here is that adding overflow: auto ended up causing other UI bugs for us so I had to remove it, but I noticed we had overflow-x: hidden on our body tag. I removed that, which meant that getScrollableParent went all the way to document, which is not considered an element, which means it will default to the original container which is what we wanted!

    ValentinH

    ValentinH commented on Apr 3, 2020

    @ValentinH

    Thank for the tip @clara-tsang and for the investigation @sarahzinger.

    I've found this bug after updating from a very old version (0.8.1) and after trying each version, I've found out that this was introduced in 1.8.0.and especially by this PR: #507

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      No labels
      No labels

      Projects

      No projects

      Milestone

      No milestone

      Relationships

      None yet

        Development

        No branches or pull requests

          Participants

          @alentame@ValentinH@sarahzinger@clara-tsang

          Issue actions

            Grid rows not wrapping when element is dragged up first column · Issue #617 · clauderic/react-sortable-hoc