daybrush / moveable

Moveable! Draggable! Resizable! Scalable! Rotatable! Warpable! Pinchable! Groupable! Snappable!
https://daybrush.com/moveable/
MIT License
10.16k stars 624 forks source link

`resizeGroup` not honoring `snapThreshold` (choppy resize of selected elements) #961

Closed davidalejandroaguilar closed 1 year ago

davidalejandroaguilar commented 1 year ago

Environments

Description

When resizing grouped elements, the width and height coming on the events do not honor snapThreshold.

On the video below (and on the codepen above), you can see that each individual item can be resized in increments of 50px.

However, when you select both items and resize, it gets choppy.

https://github.com/daybrush/moveable/assets/15317732/62ed6543-3113-42b2-9cda-6e36cde30975

daybrush commented 1 year ago

@davidalejandroaguilar

moveable's new version is released. Check it again.

davidalejandroaguilar commented 1 year ago

Thanks for the prompt follow up @daybrush! The choppiness is indeed fixed 🎉 . Though I still don't think resize is working properly.

Let me know if this should be a new issue.

In this video (same Codepen), you can see that the green rectangle (the one we're using to resize the selection) scales with the grid ✅ . However, the red rectangle does not ❌ , and you end up with the red rectangle with a smaller size than 50px (the grid size).

https://github.com/daybrush/moveable/assets/15317732/d18a8049-b1eb-4894-9310-dfd97c0fdf6c

Also in this other 2 videos (same Codepen), you can see how, if you select and resize a single element, you can resize it in all directions. However, when you select and resize multiple elements, this no longer becomes possible:

Resizing a single element in all directions

https://github.com/daybrush/moveable/assets/15317732/3b628166-3524-408d-b7bb-fcc5ce548d04

Resizing multiple elements in all directions

https://github.com/daybrush/moveable/assets/15317732/d8762122-eadc-4a6d-bfff-224c98042582

Let me know if you need more clarification and thank you again for such a wonderful library!

daybrush commented 1 year ago

@davidalejandroaguilar

Sorry. group has keepRatio applied and moves with snapThreshold on a group basis. So there are a lot of problems.

However, this example may be the best.

on("beforeResizeGroup", e => {
        const throttle = num => Math.round(num / 50) * 50;
        const ratio = e.boundingWidth / e.boundingHeight;

        if (ratio < 1) {
            const nextBoundingWidth = throttle(e.boundingWidth);
            const nextBoundingHeight = nextBoundingWidth / ratio;

            e.setSize([nextBoundingWidth, nextBoundingHeight]);
        } else {
            const nextBoundingHeight = throttle(e.boundingHeight);
            const nextBoundingWidth = nextBoundingHeight * ratio;

            e.setSize([nextBoundingWidth, nextBoundingHeight]);
        }

    }).
davidalejandroaguilar commented 1 year ago

Thank you very much for your time, closing this since the choppy resize (original issue) has been fixed 😃

Also, thanks for the suggested code using beforeResizeGroup for the resize problem. Seems like that would be a great solution for someone that wanted to use keepRatio: true. However, I actually want keepRatio: false.

Here's the new issue I opened to talk about this.