Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange Grid Layout bug? #454

Closed
3nematix opened this issue Jun 23, 2023 · 13 comments · Fixed by #457
Closed

Strange Grid Layout bug? #454

3nematix opened this issue Jun 23, 2023 · 13 comments · Fixed by #457

Comments

@3nematix
Copy link

Hi, I'm just wondering why does the col-span-2 class only applies when the draggable div is only released?

The thing is that I want to keep the grid layout as you can see in the attached video.
For some case when the image is being dragged to the first index it should get the col-span-2 class, but it doesn't.

03d94febde5436f93118cc9af4e046bc.mp4

The code:

<script>
    import {flip} from "svelte/animate";
    import {dndzone} from "svelte-dnd-action";

    let items = [
        {id: 1, src: "https://images.unsplash.com/photo-1460317442991-0ec209397118?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D&auto=format&fit=crop&w=1740&q=80"},
        {id: 2, src: "https://images.unsplash.com/photo-1560448204-e02f11c3d0e2?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D&auto=format&fit=crop&w=1740&q=80"},
        {id: 3, src: "https://images.unsplash.com/photo-1565618948089-5029088c3c67?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D&auto=format&fit=crop&w=876&q=80"},
        {id: 4, src: "https://images.unsplash.com/photo-1460317442991-0ec209397118?ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D&auto=format&fit=crop&w=1740&q=80"}
    ];

    const flipDurationMs = 300;
    function handleDndConsider(e) {
        items = e.detail.items;
    }

    function handleDndFinalize(e) {
        items = e.detail.items;
    }
</script>

<div class="flex flex-col gap-6 h-full fade-in">
    <div>
        <label for="id_first_name" class="text-primary font-semibold">Property Images</label>
        <p class="text-secondary font-medium text-sm">To rearrange or edit your photos, just simply drag the image to a new position. A maximum of <strong>52 images can be uploaded</strong>.</p>

        <div
            class="grid grid-cols-2 items-center justify-between gap-3 mt-3"
            use:dndzone={{items, flipDurationMs}}
            on:consider={handleDndConsider}
            on:finalize={handleDndFinalize}
        >
            {#each items as item(item.id)}
                <div
                    animate:flip={{duration: flipDurationMs}}
                    transformDraggedElement:
                    draggable="true"
                    class:col-span-2={
                        items.indexOf(item) === 0 || items.indexOf(item) % 3 === 0
                    }
                    class="drop-shadow-lg rounded-xl bg-image h-32 border-3 border-blue"
                >
                    <div class="relative w-full h-full">
                        <img
                            alt=""
                            class="rounded-lg object-cover absolute inset-0 h-full w-full"
                            src={item.src}
                        >
                        {#if items.indexOf(items.find(i => i.id == item.id)) === 0}
                            <p class="absolute top-3 right-3 bg-primary text-purewhite font-bold text-xs px-5 py-2 rounded-lg">Thumbnail</p>
                        {/if}
                    </div>
                </div>
            {/each}
        </div>
    </div>
</div>
@isaacHagoel
Copy link
Owner

Hi,
Can you please provide a REPL that reproduces the issue? The code snippet you provided doesn't include the css (is this tailwind?) and as a result doesn't render a grid.

@3nematix
Copy link
Author

3nematix commented Jun 23, 2023

Hi, I'm really sorry, but I think that I'm not able to make an example with tailwind, but the classes are same explanatory. Basically the col-span-2 is grid-column: span 2 / span 2;

So in this case the draggable div should just take up the two available grid columns, but it doesn't... Only after dragging over the top div and then after one more random hover on any element the col-span-2 works fine, you can see it in the video.

@isaacHagoel
Copy link
Owner

isaacHagoel commented Jun 23, 2023 via email

@3nematix
Copy link
Author

@isaacHagoel
Copy link
Owner

Thanks. So I can see the the shadowElement (as in the placeholder in the grid) is getting the correct dimensions and is in the right place but for some reason they don't always get copied to the dragged element as expected (happens here).
I haven't seen this happening before and will try to make some tomorrow to dig deeper and figure out why.

@3nematix
Copy link
Author

Yes, thank you very much:)

@isaacHagoel
Copy link
Owner

isaacHagoel commented Jun 25, 2023

I found the root cause. The browser is initially returning the wrong width when getBoundingClientRect is called. It seems like it takes time for the value to get to its correct value (around a 100ms). This could be seen by adding these two lines under line 75 in styler.js.

console.warn("morphing", copyFromEl, copyFromEl.getBoundingClientRect().width);
window.setTimeout(() => console.error(copyFromEl.getBoundingClientRect().width) , 100);

I checked without tailwind, with a normal grid layout (quick and dirty, just to check) and it behaves the same

	.my-grid {
             display: grid;
	     height: 70vh;
             grid-template-rows: 1fr 1fr 1fr; 
            grid-template-columns: 1fr 1fr;
            grid-gap: 10px; 
        }

       .my-grid-item {
	  height: 50px;
       } 
     .image-container {
           height: 100%;
	   width: 100%;
	}
	img {
		height: 100%;
		width: 100%;
		max-width: 100%;
		max-height: 100%;
	}

.my-grid-item:nth-child(1), 
.my-grid-item:nth-child(4) {
  grid-column: 1 / 3;
}

Interestingly, if I modify the code to copy the width from the computed styles instead of using the bounding rect by adding this

 || s === "width"

into copyStylesFromTo it fixes the issue and gets the correct value.
I will give it some more thought but that at least gives me a way to workaround what seems to be a browser bug.
there are other calculations in the lib that rely on getting a correct reading from getBoundingClientRect and now I wonder whether anything else is affected.

@3nematix
Copy link
Author

Great, thanks for your time!

I'll be waiting for any updates on this one :)

@3nematix
Copy link
Author

Hi, just wanted to follow-up with you.

Any fixes coming soon or new updates?

@3nematix
Copy link
Author

?

@isaacHagoel
Copy link
Owner

isaacHagoel commented Jun 30, 2023 via email

@isaacHagoel
Copy link
Owner

I made this fix. Tested it superficially and it seems fine, would be great if you could pull it and make sure that fixes your issue and maybe test a bit to help improve my confidence here :)
#457

@isaacHagoel
Copy link
Owner

@3nematix FYI, the workaround was causing other issues so I moved it to behind a flag ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants