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

Allow zooming on very thin selection boxes #1482

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Allow zooming on very thin selection boxes #1482

merged 1 commit into from
Aug 29, 2023

Conversation

axelboc
Copy link
Contributor

@axelboc axelboc commented Aug 25, 2023

Fix #1407

I've changed the selection validation logic in SelectToZoom to compute the manahattan distance. I'm no longer converting the selection to a Box and calling #hasMinSize, since Vector3 already provides a method for computing the manhattan distance.

Of course there are always edge cases to deal with and unforeseen consequences...


The edge case is zooming on a 0-width or 0-height box. To deal with this, I had a few options:

  1. Keep a hasMinSize(1) check in the validate prop.
    • This kinda goes against the premise of There is no way to zoom on a small slice #1407 that if the user releases the pointer, something should happen.
    • It also meant that we could see a dashed line when crossing the 0 width/height threshold (because that's the style of an invalid selection box). This style made more sense when the invalidity threshold was wider.
  2. Ensure the zoom box can never be empty via the transform prop.
    • This would have made the code of computeZoomSelection a bit messy.
  3. Ensure we never zoom on an empty box by enforcing a minimum zoom width/height of 1.
    • Seemed like the best compromise, so I went with this solution.

The unforeseen consequence had to do with SvgRect:

We work around the fact that SVG doesn't support drawing inside strokes by drawing a slightly smaller and slightly offset rectangle. The problem is that empty rectangles are simply hidden by browsers, so any SvgRect with a width or height lower than or equal to their stroke width would simply vanish.

This behaviour wasn't too problematic when we couldn't zoom on a very thin box, since at the point the selection box disappeared, the zoom box was invalid — so releasing the mouse did nothing. So I had to find a fix.

I decided to replace rect with path, which seems to work great, fortunately.

@axelboc axelboc requested a review from loichuder August 25, 2023 15:19
Copy link
Member

@loichuder loichuder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats for the smart workarounds, this is great 👍

@axelboc axelboc merged commit e53129e into main Aug 29, 2023
8 checks passed
@axelboc axelboc deleted the min-zoom branch August 29, 2023 07:03
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.

There is no way to zoom on a small slice
2 participants