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

Prevent resizing row child element completely over adjacent element #732

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

apedroferreira
Copy link
Member

@apedroferreira apedroferreira commented Aug 3, 2022

Previously it was possible to, while resizing an element horizontally, extend it completely over the element next to it - which leads to bugs and weird behavior.
This PR limits the resizing to the width of one column in the adjacent element.

Video ("before" in blue, "after" in red):

Screen.Recording.2022-08-03.at.18.30.36.mov

@render
Copy link

render bot commented Aug 3, 2022

@oliviertassinari oliviertassinari requested a deployment to fix-resizing-bug - toolpad-db PR #732 August 3, 2022 16:56 — with Render Abandoned
@apedroferreira apedroferreira changed the base branch from master to drag-and-drop-fixes August 3, 2022 16:56
@apedroferreira apedroferreira changed the title Prevent resizing row child element over adjacent element Prevent resizing row child element completely over adjacent element Aug 3, 2022
@Janpot
Copy link
Member

Janpot commented Aug 4, 2022

🤔 Ok, yes, I see how this is a constraint imposed by the fact that for us resizing means "redistribute the space proportionally between left and right sibling". That's a bit unfortunate.

I think one possible way to allow resizing beyond the left sibling here is to not just consider the left sibling, but all siblings to the left. sum up their original fr values (X), use it to calculate the new total fr value (Y) the same way we do it for one sibling. then calculate the fraction (Y/X) and multiply each individual fr value with that fraction.

This may result in a more unpredictable resizing behavior so it may be worse, it's just something to think about.

Another way is to consider a minimum width for a sibling and not just resize against the sibling to the left, but once it reaches minimum width, start resizing the sibling next to it and so on. This may be a bit more involved to implement as you'd need to sort of "undo" it once you start dragging back to the right.

@apedroferreira
Copy link
Member Author

🤔 Ok, yes, I see how this is a constraint imposed by the fact that for us resizing means "redistribute the space proportionally between left and right sibling". That's a bit unfortunate.

I think one possible way to allow resizing beyond the left sibling here is to not just consider the left sibling, but all siblings to the left. sum up their original fr values (X), use it to calculate the new total fr value (Y) the same way we do it for one sibling. then calculate the fraction (Y/X) and multiply each individual fr value with that fraction.

This may result in a more unpredictable resizing behavior so it may be worse, it's just something to think about.

Another way is to consider a minimum width for a sibling and not just resize against the sibling to the left, but once it reaches minimum width, start resizing the sibling next to it and so on. This may be a bit more involved to implement as you'd need to sort of "undo" it once you start dragging back to the right.

At first I didn't think about this change as a limitation in resizing, but yeah I guess it could end up being an unexpected limitation for users...
It should be possible to do something like you first proposed, it would probably just be a bit more complex than the current logic. And even though it would have advantages like more freedom to resize elements, the way the other elements resized would be more unpredictable, not sure if it would be worth it, depends on how important users would consider to be able to resize beyond adjacent elements. I can try it out later and see if I come up with any obstacles.
The second proposal you made I'm not sure about how intuitive it would be to use, and it also sounds more complicated to implement, the first one sounds like a better trade-off in my opinion.

@Janpot
Copy link
Member

Janpot commented Aug 4, 2022

the first one sounds like a better trade-off in my opinion.

I'm coming back from this. Imagine the scenario where you have three panels, and you know how you want to size each of them. You size the left one to the width you want, and then you size the right one to the width you want. But this last operation also resized the left one again. You basically can never pin one to the exact place you desire as long as you have others to resize. Theoretically, only the last panel you resized will end up in the exact spot you intend.

the way the other elements resized would be more unpredictable, not sure if it would be worth it

Here's a real world example of the behavior I'm talking about:

Screen.Recording.2022-08-04.at.16.50.49.mov

Personally, I find this more predictable behavior than the first scenario

@apedroferreira
Copy link
Member Author

apedroferreira commented Aug 4, 2022

the first one sounds like a better trade-off in my opinion.

I'm coming back from this. Imagine the scenario where you have three panels, and you know how you want to size each of them. You size the left one to the width you want, and then you size the right one to the width you want. But this last operation also resized the left one again. You basically can never pin one to the exact place you desire as long as you have others to resize. Theoretically, only the last panel you resized will end up in the exact spot you intend.

the way the other elements resized would be more unpredictable, not sure if it would be worth it

Here's a real world example of the behavior I'm talking about:

Screen.Recording.2022-08-04.at.16.50.49.mov
Personally, I find this more predictable behavior than the first scenario

That looks pretty cool! - just not sure how to do it with the current transparent colored preview rectangles - we would have to find another solution for that too.
Those are good ideas though so we should eventually improve the resizing to be able get rid of the restriction I'm adding in this PR. That could also go along with not having to drag the edges of the elements themselves but the border between them, like you suggested when you reviewed the horizontal resizing feature (which I haven't implemented because the preview rectangle always shows for a specific element).
Let's make a separate issue for it, I can do it in a bit and link to this PR for context.

@apedroferreira
Copy link
Member Author

apedroferreira commented Aug 4, 2022

I've created an issue - feel free to add more things there! #736

@apedroferreira apedroferreira merged commit 8fcc7e1 into drag-and-drop-fixes Aug 4, 2022
@apedroferreira apedroferreira deleted the fix-resizing-bug branch August 4, 2022 17:51
apedroferreira added a commit that referenced this pull request Aug 8, 2022
* Refactor drag and drop for more simplicity and readability (#730)

* Prevent resizing row child element completely over adjacent element (#732)

* Disallow creating columns in single-child rows (#733)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants