Fix sliding when resizing with minimum limits reached#3433
Conversation
|
OK, found the reason for the tests being unreliable: there was an However, this brought out an actual additional bug when fit-text is turned off, so looking into that now. |
|
|
||
| // Rotate the block and try again. | ||
| await rotateSelectedBlock(); | ||
| await dragAndResize( resizableHandleTop, { x: 300, y: 0 } ); |
There was a problem hiding this comment.
Here you switch to dragging in x direction in stead of y direction after rotation. But in the above test, you drag in x both before and after rotation. I don't know which is the right one, but it seems to be this one, perhaps?
I guess that depends on how the item is rotated of course 😝.
There was a problem hiding this comment.
Hah! Good catch, thank you. The rotation is not enough for the initial direction not to work -- will leave a comment with degrees.
|
If you resize "quickly", then the item jumps when you reach minimum position, as it jumps to the last "legal" top position and minimum height from there in stead of minimum height based off the "static" bottom position. This is particularly obvious with a rotated block, but happens for non-rotated ones as well. I tried to see, if I could fix this, but I just can't wrap my head around it. Maybe for non-rotated blocks, but definitely couldn't figure it out for the rotated ones.
|
|
Thanks for testing! I think I remember seeing this "jumpiness" on quick resizing from before as well, it's not easy to test on Will try to look into this jumpiness as well in this PR, if it starts taking too much time then will create a separate issue instead. |
|
I am going to wait to code review until the "jumpiness" is fixed. |
|
Added a new issue for the jumpiness. It's unrelated to the original issue anyway and possibly lower priority, too. |
…roject/amp-wp into fix/3397-resizing-min-limit
|
Still seeing the issue: https://cloudup.com/cRZMrlUPdy8 |
Hmm, that seems to be a different issue -- the original issue is for the position continuing to change even though the minimum limits have been reached and it's not possible to reduce anymore (in case of amp-fit-text turned off). I'll debug this new finding as well to see if it seems to be an easy fix. For testing/QA purposes might be good to track that as a different issue though. Looks like that's for when "amp-fit-text" is turned on, right? |
Correct. A separate issue is fine for me. Thanks for having a look! |
swissspidy
left a comment
There was a problem hiding this comment.
The proposed changes here work well. Related issues with resizing have been brought up, but these can be addressed in separate PRs.
…mp-wp into fix/3397-resizing-min-limit
|
OK, looks like the tests have passed a few times in a row now. |
Summary
Fixes #3397
Fixes #3326
scrollHeightcalculations to limit resizing if the Text block is empty anyway.Checklist