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

feat: Elbow arrow segment fixing & positioning #8952

Merged
merged 300 commits into from
Jan 17, 2025
Merged

Conversation

mtolmacs
Copy link
Collaborator

@mtolmacs mtolmacs commented Dec 26, 2024

380760733-c5004ca1-e626-4967-a953-f85a0269f4bf

What's in this PR?

  • Main goal was to allow manual positioning of elbow arrow segments on either horizontal or vertical direction (depending on its original heading)
  • The movement of segments happen via showing a control knob at every segment midpoint, reusing the idiom from editable linear elements
  • Secondary goal was to keep the modified segment unchanged, except the directional displacement (so the length of the moved segment is fixed)
  • The fixed segments can be "released" when double clicking on the hollow control know at the middle of the segment
  • Additionally this PR consolidates all elbow arrow modification into mutateElement() so the complexity is hidden for other developers.
  • This PR also bring a fix for "misaligned" but very close parallel segments, avoiding a visual glitch

Key implementation details

  • packages/excalidraw/element/routing.ts is renamed to packages/excalidraw/element/elbowarrow.ts

  • The LinearElementEditor class contains handlers for segment midpoint logic, but due to a likely state mismatch / race condition a completely new midpoint handling was implemented for elbow arrows.

  • Elbow arrow creation and updates needed different inputs, so the LinearElementEditor::movePoints() function was extensively updated to cover both elbow arrows and simple arrows.

  • Due to a UX requirement where an elbow arrow is bound on both ends and the second segment is fixed, then the start (or end) of the arrow is dragged to a perpendicular side of the bound element, there are two new properties on elbow arrow elements, startIsSpecial and endIsSpecial respectively.
    a

  • To store the fixed segments data on the elbow arrow elements a new property is introduced, called fixedSegments. It is an array of objects, the objects contain

    • a segment index, which is the point index of the last point of a fixed segment in the points property (so it goes from 1 to length -1)
    • start, which is the element LocalPoint coordinate of the start of the fixed segment
    • end, which is the element LocalPoint coordinate of the end of the fixed segment
  • The elbow arrow generating algorithm on its own creates suboptimal arrow points with duplicates and hooks, which are simplified in a post processing step. This simplification results in loss of information for subsequent modification requests, so there are a lot of subtle heuristics to "guess" the lost information, which would be required to keep a stable visual experience.

  • The visual debugging is enhanced and refactored to make development easier (beyond elbow arrows)

Where to start a review

elbowarrow.ts

  • Start with reviewing the fist half of packages/excalidraw/element/elbowarrow.ts. The code in this file is organized around a single main entrypoint, updateElbowArrowPoints(...). This is called from mutateElement(...) to calculate the changes needed, then returns the element updates required in the form of ElementUpdate<ExcalidrawElbowArrowElement>, which is then woven into the original logic of mutateElement(...)
  • The updateElbowArrowPoints(...) function is a dispatch function to handle 6 different use cases individually.
  • The original elbow arrow routing code is refactored into two separate functions, which are almost identical to the original code: getElbowArrowData(...) which just calculates the common parameters used by all the functions and routeElbowArrow(...) which contain the routing algorithm. The modifications in these compared to the original is minimal, only affecting spacing around bound elements.

mutateElement.ts

  • This contains the call to updateElbowArrowPoints(...) in elbowarrow.ts as a separate branch before the original code. This branch prepares the function arguments required for the elbow arrow code, then applies the resolved changes before handing over the control flow to the old code.
  • Due to the fact that elbow arrows need additional information in some use cases, a new optional options argument is added to the interface of mutateElement(...) which affects the public API.

LinearElementEditor.ts

  • Beyond the changes to movePoints(...) there are small modifications to the original code to handle elbow arrows
  • Additional functions introduced to cover the new use cases, moveFixedSegment(...) and deleteFixedSegment(...)
  • There are helper functions added to support these two main use case covering functions, which are also called from components/App.tsx handlers.

packages/excalidraw/components/App.tsx

  • A new segment midpoint handler code is introduced due to the previously mentioned state handling bug
  • These modifications also hook into the segment movement and delete functions in LinearElementEditor.
  • Due to the special needs of elbow arrows, there are modifications to resize, flip, drag and restore functionalities for the elements (dragElement.ts, resizeElement.tsand restore.ts

fractionalIndex.ts

  • Due to the change in calling signature of mutateElement(...) changes were needed in syncInvalidIndices(...) and syncMovedIndices(...)

mtolmacs and others added 30 commits October 24, 2024 18:24
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
…draw/excalidraw into feat/elbow-arrow-movable-segments
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
Signed-off-by: Mark Tolmacs <mark@lazycat.hu>
@dwelle
Copy link
Member

dwelle commented Jan 17, 2025

Let's do this! 🔥🚀 Thanks @mtolmacs! ❤️

@mtolmacs mtolmacs merged commit 91ebf8b into master Jan 17, 2025
10 checks passed
@mtolmacs mtolmacs changed the title feat: Elbow segment move feat: Elbow arrow segment fixing & positioning Jan 17, 2025
@mtolmacs mtolmacs deleted the feat/elbow-segment-move branch January 17, 2025 17:07
zsviczian added a commit to zsviczian/excalidraw that referenced this pull request Jan 18, 2025
feat: Elbow arrow segment fixing & positioning (excalidraw#8952)
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.

Straight elbow errows end up with untiable knots Rectangular Arrows Alignment
7 participants