fix: Drag into sidebar always docks into an empty zone #499
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gwhitney
force-pushed
the
tab_drag
branch
2 times, most recently
from
0b8cd74 to
38230ec
Compare
Prior to this PR, dragging a tab on top of a docked one would leave it floating over the docked tab. Moreover, if the empty zone in the sidebar was less than half the size of the tab being dragged, it would be impossible to dock the dragged tab at all. Now any drag into a sidebar with at least one empty zone results in a docking to the targeted zone if it is empty, or the first empty zone if not. Resolves numberscope#431. Note that this PR does not at the moment implement any resizing. There is resizing described in issue numberscope#431. However, now that this is working smoothly, it is making perfect visual sense to me that the dragged tab fits itself into the remaining empty space, leaving the already-docked tab alone, and in fact, this behavior seems to me preferable to what's described in numberscope#431. But if there remains a clear desire for the resizing as described in the issue, that can be implemented. Just let me know.
|
Aaron found a bug: pull out bottom tab, drag top tab down, try to drag that tab back up, it won't go, and then try to drag the old bottom tab back into the gap at the top, it just lands over the sidebar, it doesn't dock. |
|
Aaron is fine with only the dragged tab resizing, so unless Kate disagrees we will just go with the current behavior (once the bug is fixed). |
|
Another bug: Drag tab into left, resize it in left, drag it out so nothing is in left. When you put something in left, it now uses previous divider position. |
Also fixes bug that sometimes prevented docking in an apparently empty spot (the `empty` class was not being properly maintained).
|
OK, I believe that commit fixes all of the issues we observed at today's meeting. Ready for further review. |
|
Ok, I can't get it to do anything that seems like a bug. I'm going to merge. |
gwhitney
added a commit
that referenced
this pull request
Jan 20, 2025
* fix: Make sure warnings are shown every time Formula changes * fix: Drag into sidebar always docks into an empty zone Prior to this PR, dragging a tab on top of a docked one would leave it floating over the docked tab. Moreover, if the empty zone in the sidebar was less than half the size of the tab being dragged, it would be impossible to dock the dragged tab at all. Now any drag into a sidebar with at least one empty zone results in a docking to the targeted zone if it is empty, or the first empty zone if not. Resolves #431. Note that this PR does not at the moment implement any resizing. There is resizing described in issue #431. However, now that this is working smoothly, it is making perfect visual sense to me that the dragged tab fits itself into the remaining empty space, leaving the already-docked tab alone, and in fact, this behavior seems to me preferable to what's described in #431. But if there remains a clear desire for the resizing as described in the issue, that can be implemented. Just let me know. * chore: revert change to Formula.ts left over from Git manipulations * fix: tab drags and docking/undocking preserve size whenever possible Also fixes bug that sometimes prevented docking in an apparently empty spot (the `empty` class was not being properly maintained).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By submitting this PR, I am indicating to the Numberscope maintainers that I have read and understood the contributing guidelines and that this PR follows those guidelines to the best of my knowledge. I have also read the pull request checklist and followed the instructions therein.
Prior to this PR, dragging a tab on top of a docked one would leave
it floating over the docked tab. Moreover, if the empty zone in the
sidebar was less than half the size of the tab being dragged, it would
be impossible to dock the dragged tab at all.
Now any drag into a sidebar with at least one empty zone results in
a docking to the targeted zone if it is empty, or the first empty zone
if not.
Resolves #431.
Note that this PR does not at the moment implement any resizing.
There is resizing described in issue #431. However, now that this is
working smoothly, it is making perfect visual sense to me that the
dragged tab fits itself into the remaining empty space, leaving the
already-docked tab alone, and in fact, this behavior seems to me
preferable to what's described in #431. But if there remains a clear
desire for the resizing as described in the issue, that can be
implemented. Just let me know.
OK, this has been rebased and is ready for its own review. There are two spurious commits with changes to Formula.ts, a637a7f and 46d1ac9. These are an artifact of how I originally submitted this on top of #498. But the latter one precisely undoes the other, and so they will "squash commit" to nothing: the only commit that matters at this point is a53d7ea, and as you can see at the top, GitHub realizes that only Scope.vue has changed. Sorry about the confusion, I should have just done this as an independent PR like I did with the last two, but I didn't set Git up quite right this time. But now all should be well for this to be reviewed normally and once it is ready, squash-merged like normal.