Feature: Multi Point Arrows

[GasimGasimzada]

Currently, I have gotten some preliminary things working. Here are the features that I want to get to working condition

• Reenable shift resizing the arrow
• Tune collision detection algorithm
• Re-enable SHIFT when dragging the arrow segments
• When dragging initially, cancel the shape once dragging is done
• Allow dragging shapes
• Deleting the arrow while adding points shouldn't reset the state of the arrow. Currently, you need to change the shape for it to work again.
• Fix resize for arrow
• Change hit test for the new arrow
• Use Curves instead of lines
• Select arrow when in multi element mode
• Change color of the circle when it is on an existing point
• Adding a new point to the arrow (double click on a Point that gets displayed)
• Switch back to roughjs line drawing instead of using path (path draws a different type of shape than a line).

[vercel]

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/vjeux/excalidraw/lupi62niw
✅ Preview: https://excalidraw-git-feature-multi-point-arrows.vjeux.now.sh

[vjeux]

Zwibbler (https://zwibbler.com/demo/) has this interesting UI interaction where if you mouse down and drag, it creates a single directional arrow. But if you click (at the beginning), then you can drag (without mouse down) to select the next point, and again... until you double click to confirm the arrow.

It feels really well thought out and I think we should do something similar.

[vjeux]

They allow you to after the fact move the intermediate points, but honestly I've never used it, I would just recreate the arrow from scratch, so we should likely focus on the creation path for now.

[vjeux]

[vjeux]

For the hit test, you can use this algorithm to find the closest point on the bezier curve ( https://stackoverflow.com/a/44993719/232122 ) and then you can just compute the distance between that point and the mouse to find when you have to consider it a hit.

[vjeux]

Any update? :)

[GasimGasimzada]

@vjeux I am sorry. I just got married and it has been couple of stressful days due to paperwork etc. I will get back into it tomorrow.

[vjeux]

Waaattt!!! Congratulations!!! This is awesome <3

[GasimGasimzada]

Thank you😊😍

[GasimGasimzada]

Does anyone know why this was changed?

<<<<<<< HEAD
              height = e.shiftKey
                ? Math.abs(width) * Math.sign(height)
                : height;

              draggingElement.width = width;
              draggingElement.height = height;

              // add points for drawing when it is the arrow
              if (element.type === "arrow") {
                draggingElement.points = [
                  [0, 0],
                  [width, height]
                ];
              }
=======
              draggingElement.height =
                e.shiftKey && this.state.elementType !== "selection"
                  ? Math.abs(width) * Math.sign(height)
                  : height;
>>>>>>> master

I have merged master into this branch but I am still not sure about this part of the code. Are you not setting the width anymore?

[GasimGasimzada]

I am going to keep updating this PR with my changes using GIFS :) Right now, multi arrow already works but there are more things that I need to fix. I am going to update my original comment to reflect the new TODO list.

[dwelle]

I just discovered another thing: during resize of multi-point arrow, it sometimes disappears, and while the element is still kept in the elements array, it's not selectable anymore (you need to clear the canvas). During export it has small dimensions (shows as small square).

[GasimGasimzada]

For the last part, if we implement a matrix based scaler, this issue will be resolved. The resizer works a bit weird because there are a lot of additions happening. That's why it keeps moving down instead of staying in place (in reality it shouldn't move).

[dwelle]

It's not that it moves down (which we discussed previously and decided to leave the fix for future PR), but that it sometimes disappears completely (sometimes even well before it reaches width/height of 0). Such arrow then doesn't render at all, but is still kept as an element in elements array, and appears during export unrendered too:

and when you add another element:

[GasimGasimzada]

Okay, I found the issue but I can't figure out the problem. It is in isVisibleElement. For example, I have an element that is in Y = 650 (including scroll). On the other hand, the canvas Size is 500. Because it thinks that the element is outside the bounds of the canvas, there is an error.

[dwelle]

Why do you exempt arrow shape from the normalization using scrollX/scrollY (in isVisibleElement())? That could be the issue.

EDIT: it's not the issue though :/.

[dwelle]

Seeing some weird things when logging the arrow coords during the resize inside isVisibleElement() helper:

It looks like very second call returns the same coords, and y1/y2 are identical (until mouseup) 🤔

[GasimGasimzada]

This is a bit problematic approach but the best one that I come up with. The issue here is that when shape of the arrow does not exist (e.g during resizing we remove the shape for regeneration), I was using element position as a fallback value. Now, I am using points array that is used to generate the arrow shape. However, these points are not identical because bezier curve points are different than the stored points.

I have tested it with different test cases and it seems to be working. If there is an issue, we can add a small threshold to make bounds similar to roughjs curve. Additionally, once we implement matrix based scaling, I think this issue will be resolved because we won't be regenerating these shapes every time.

[dwelle]

The change works great (AFAIK). IMO we could have gone even with disabling the optimization (as you did at first), but if the new change works, even better. Either way, as you say, it'll be fully resolved when the matrix based scaling is implemented in the future.

Alright, I think we're ready to ship?

[GasimGasimzada]

Yes! 🎉 This was one huge PR 😄

[dwelle]

Woohoo! This has been a wild ride (for you much much more I'm sure). ❤️ It's time to rejoice.

[GasimGasimzada]

I am going to celebrate now! 😄