feat: Created a minimap that is positionable in the primary workspace #1824
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
cesarades
force-pushed
the
implementPositionable
branch
from
343f13d
to
b00717a
Compare
| const hasVerticalScrollbars = this.primaryWorkspace.scrollbar && | ||
| this.primaryWorkspace.scrollbar.canScrollHorizontally(); | ||
| const hasHorizontalScrollbars = this.primaryWorkspace.scrollbar && | ||
| this.primaryWorkspace.scrollbar.canScrollVertically(); |
There was a problem hiding this comment.
Are these checking the correct scrollbars? I know this matches what the backpack is doing, but I am a bit confused haha.
There was a problem hiding this comment.
Yes, they are checking the scrollbars in the primary workspace. It needs that to offset the location of the minimap in case they are present.
There was a problem hiding this comment.
I'm just confused why hasVerticalScrollbars is based on canScrollHorizontally. I'm pretty sure the vertical scrollbars affect the vertical scroll.
There was a problem hiding this comment.
I think Beka may have been referring to how hasVerticalScrollbars is checking canScrollHorizontally, and vice versa, vs checking the primary workspace scrollbars as opposed to the minimap scrollbars. That was (is) confusing to me at least - if intentional, a comment explaining the reasoning would be helpful!
There was a problem hiding this comment.
Ok, so this is in fact an error. I've corrected it so that vertical goes with vertical and vise versa. Also, I added another use case that was no previously checked. If the scrollbar exists and scrolling is enabled, but the scrollbar is not visible, it should adjust for that.
There was a problem hiding this comment.
Please also file a bug to update the backpack plugin similarly!
| const hasVerticalScrollbars = this.primaryWorkspace.scrollbar && | ||
| this.primaryWorkspace.scrollbar.canScrollHorizontally(); | ||
| const hasHorizontalScrollbars = this.primaryWorkspace.scrollbar && | ||
| this.primaryWorkspace.scrollbar.canScrollVertically(); |
There was a problem hiding this comment.
I think Beka may have been referring to how hasVerticalScrollbars is checking canScrollHorizontally, and vice versa, vs checking the primary workspace scrollbars as opposed to the minimap scrollbars. That was (is) confusing to me at least - if intentional, a comment explaining the reasoning would be helpful!
Description
I created a minimap that is positionable in the primary workspace. I did so by adding a sibling node to the primary workspace and then using absolute CSS positioning to place it correctly.
Tests
I updated the testing so that it includes a positioned and an unpositioned minimap use case.