QOL: Arrow bending and termination by RogerZhang888 · Pull Request #3717 · source-academy/frontend

RogerZhang888 · 2026-03-31T07:59:28Z

Description

Arrows now have
• standardised radius of curvature (except when there are space constraints)
• Terminating segment length (so there is no abrupt "bent" arrow-head)

The arrows which pointed to arrays from the left were initially straight arrows. Now, they also follow Manhattan routing like the rest of the arrows.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
Code quality improvements

How to test

Run any Source program and step through the CSE Machine to see the arrows.

Checklist

I have tested this code
I have updated the documentation

…re where clicked arrows remain highlighted

we need to merge

…fferent types of arrows

…r arrows

gemini-code-assist

Code Review

This pull request introduces an arrow filtering system for the CSE Machine visualization, allowing users to toggle arrow visibility based on origin (e.g., text, frame, function). It also refines arrow routing logic, improves path rendering with better corner radius calculations, and adds a new arrow type for function descriptions. Key feedback includes addressing performance issues caused by object instantiation within draw methods, replacing any types with proper interfaces for type safety, simplifying redundant routing logic, and moving magic numbers to the configuration file.

gemini-code-assist · 2026-03-31T08:03:06Z

    if (this.enclosingFrame) {
      this._arrow = new ArrowFromFn(this).to(this.enclosingFrame) as ArrowFromFn;
    }
+    this.createBodyArrow();


Instantiating FnBodyTarget and ArrowFromFnToBody inside the draw method is a significant performance issue. Since draw is called on every render, and GenericArrow creates a new React.createRef() in its constructor, this will cause the arrow component to unmount and remount on every update, leading to flickering and high CPU usage. These objects should be instantiated once (e.g., in the constructor or handleNewReference) and updated only when coordinates change.

gemini-code-assist · 2026-03-31T08:03:06Z

    if (Layout.globalEnvNode.frame) {
      this._arrow = new ArrowFromFn(this).to(Layout.globalEnvNode.frame) as ArrowFromFn;
    }
+    this.createBodyArrow();


Similar to FnValue, creating the body arrow and its target inside the draw method will cause performance degradation and stability issues due to constant re-instantiation of objects and refs. Please move this logic out of the draw method.

gemini-code-assist · 2026-03-31T08:03:06Z

  isStringIdentifiable: boolean;
  faded: boolean;
  hidden: boolean;
+  parentFrame?: any; // Reference to the frame this text belongs to


Using any for parentFrame bypasses TypeScript's type checking. Since this property is used to access coordinates and dimensions (e.g., in ArrowFromText.tsx), it should be typed as IVisible or Frame to ensure type safety.

@RichDom2185 has also advised me in another of my recent PRs to not use the 'any' type in general. The exact comment with his message is here.

#3690 (comment)

It might be outdated! Do expand the "outdated" comment tabs

gemini-code-assist · 2026-03-31T08:03:06Z

+      if (to instanceof ArrayValue) {
+        // Route text-to-array arrows with Manhattan segments, ending at array's left-center.
+        steps.push((x, y) => [preTerminalX, y]);
+        steps.push((x, y) => [x, targetY]);
+        steps.push((x, y) => [to.x(), y]);
+      } else {
+        // Route all rightward text arrows orthogonally so special cases still stay Manhattan.
+        steps.push((x, y) => [preTerminalX, y]);
+        steps.push((x, y) => [x, targetY]);
+        steps.push((x, y) => [to.x(), y]);
+      }


The logic for routing arrows to ArrayValue and other values is identical in this block. The code can be simplified by removing the redundant conditional check.

// Route rightward text arrows orthogonally using Manhattan segments. steps.push((x, y) => [preTerminalX, y]); steps.push((x, y) => [x, targetY]); steps.push((x, y) => [to.x(), y]);

gemini-code-assist · 2026-03-31T08:03:06Z

+  protected isInteractive(): boolean {
+    return true;
+  }


Since the explicit check for printableMode was removed from the mouse event handlers, isInteractive should be updated to return false when in printable mode. This ensures that arrows do not respond to hover or click events in the print view, maintaining the previous behavior.

Suggested change

protected isInteractive(): boolean {

return true;

}

/** Subclasses can disable all pointer interactions for passive arrows. */

protected isInteractive(): boolean {

return !CseMachine.getPrintableMode();

}

This is extremely important and is similar to what I have already requested the Arrow to fix earlier on Telegram. The exact issue that hasnt been documented was as follows :

doc_2026-03-31_19-30-41.mp4

The arrows permanently become white in the Printable mode once clocked on and only revert back when clicked on some other arrow! This is also the same in the saved image actually which causes the arrow to become invisible.

The above highlighted Gemini's comment dictates a similar aspect of how arrows in general should not be interactive or if they are, the colors should change! It ofcourse should NOT be white.

gemini-code-assist · 2026-03-31T08:03:07Z

      this._x = frameX + newReference.frame.width() + Config.FrameMarginX;

-      this._y = newReference.y();
+      this._y = newReference.y() + 20;


The magic number 20 should be extracted into the Config object to improve maintainability and consistency across the visualization.

sentry · 2026-03-31T08:18:59Z

+      const preTerminalX = Math.max(frameExitX, to.x() - terminalSegmentLength);
+
+      if (to instanceof ArrayValue) {
+        // Route text-to-array arrows with Manhattan segments, ending at array's left-center.
+        steps.push((x, y) => [preTerminalX, y]);
+        steps.push((x, y) => [x, targetY]);
+        steps.push((x, y) => [to.x(), y]);


Bug: The calculation for preTerminalX can cause an arrow to route past its target horizontally before correcting, creating a U-shaped overshoot when the target is near its parent frame.
_{Severity: MEDIUM}

Suggested Fix

The routing logic should ensure preTerminalX does not exceed to.x(). Consider using different routing logic when the target is located between the text's origin and the calculated frame exit point.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/features/cseMachine/components/arrows/ArrowFromText.tsx#L60-L66 Potential issue: When routing an arrow from text to a target on its right, the `preTerminalX` coordinate is calculated using `Math.max(frameExitX, to.x() - terminalSegmentLength)`. If the target is positioned close to or within its parent frame, `frameExitX` (the frame's right edge plus a buffer) can be greater than the target's x-coordinate `to.x()`. This causes `preTerminalX` to be set to `frameExitX`, forcing the arrow path to extend past the target horizontally before turning back, resulting in a visually incorrect U-shaped overshoot. This is a common scenario for bindings where the value is near the parent frame.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

Akshay-2007-1

For the following code segment or any other that requires a few like functions in the global frame pre computed,

const xs = list(1, 2, 3);
const ys = map(x => x + 1, xs);
display(ys);

Even at step 1, when we try to filter the arrows using the new button, the list of pre-executed and shown functions in the global frame just DISAPPEAR! They dont reappear on toggling the filter as well. Only when we change the step do they reappear! This is kind of similar to the issue #3712 which concerns the Layout team too!

The bug is not really “the arrows are wrong”, it is filtering arrows may accidentally wipe cached built-in function names and change what global functions are shown. It is shown below :

Video.Project.11.mp4

@ThatLi and @gigopogo, just out of curiosity, does this concern you? I mean its a gray area as it is triggered on using the filter button that is new and has not been merged yet!

Akshay-2007-1 · 2026-03-31T12:10:03Z

I would also like to add that on hovering over the Arrow Filter option, there is no tooltip content that is displayed!

For example, for clearDeadFrames, this is what happens on hovering :

Something similar would help new users know what the option does haha! Should be an extremely simple fix!

RogerZhang888 · 2026-04-01T08:38:58Z

Closing because I created a new PR that only has changes to the appearance of the arrows. Filtering is in a separate PR.

ThatLi · 2026-04-02T04:35:14Z

For the following code segment or any other that requires a few like functions in the global frame pre computed,
const xs = list(1, 2, 3);
const ys = map(x => x + 1, xs);
display(ys);
Even at step 1, when we try to filter the arrows using the new button, the list of pre-executed and shown functions in the global frame just DISAPPEAR! They dont reappear on toggling the filter as well. Only when we change the step do they reappear! This is kind of similar to the issue #3712 which concerns the Layout team too!

The bug is not really “the arrows are wrong”, it is filtering arrows may accidentally wipe cached built-in function names and change what global functions are shown. It is shown below :
Video.Project.11.mp4

@ThatLi and @gigopogo, just out of curiosity, does this concern you? I mean its a gray area as it is triggered on using the filter button that is new and has not been merged yet!

This was before #3712 was resolved in the recent PR #3714 so I'm not sure if the disappearance was due to the code from our side or the filtering done here, since I haven't tested the code here yet. But ideally the pre-defined functions should be there regardless of whether anything is toggled.

Wang Jiahua and others added 30 commits February 9, 2026 12:29

feat: colour-coding arrows

24cf03e

feat: removed colour-coding for arrows (all white), implemented featu…

b0cccb0

…re where clicked arrows remain highlighted

fix: removed whitespace

151c4e8

Merge branch 'source-academy:master' into master

1a2a294

fix: bugfix and linting

cd9ec5b

test

a6ffa89

Merge branch 'master' into master

4cde278

test2

b9c62d8

Merge branch 'master' of https://github.com/wjh3355/source-frontend-fork

8675d11

we need to merge

fix: Refactor arrow selection management and clean up code style

1de724b

fix: ran linter

7cf2e8c

fix: snapshots

1b15c73

fix: change to american english for the word "color"

889b5d0

fix: faded arrows now stay faded

4055da7

Merge remote-tracking branch 'upstream/master'

0fccc0c

test

1856ae7

test

e14db30

test

5770e18

fix: prematurely terminating arrows

99f360b

added global MinTerminalSegmentLength

50225ce

fix start

99bc4ed

added arrows from function objects to their body; added filter for di…

6b93490

…fferent types of arrows

Merge branch 'master' of github.com:wjh3355/source-frontend-fork

d81af57

made arrows highlight to red in printable mode; added more filters fo…

03f4382

…r arrows

standardardised radius of curvature for arrows

2613f9d

Merge remote-tracking branch 'upstream/master' into rz

aaf878c

QOL: improved arrow bending

43dbdaf

Merge remote-tracking branch 'upstream' into rz

1cdf263

updates

9e78919

update

107a5cc

gemini-code-assist Bot reviewed Mar 31, 2026

View reviewed changes

RogerZhang888 added 3 commits March 31, 2026 16:06

update

3053b33

update

76bd08f

update

0b2b815

sentry Bot reviewed Mar 31, 2026

View reviewed changes

RogerZhang888 added 2 commits March 31, 2026 16:24

update

5379d48

update

45d8428

Akshay-2007-1 assigned RogerZhang888 Mar 31, 2026

Akshay-2007-1 reviewed Mar 31, 2026

View reviewed changes

RogerZhang888 closed this Apr 1, 2026

Uh oh!

Conversation

RogerZhang888 commented Mar 31, 2026

Description

Type of change

How to test

Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Akshay-2007-1 Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Akshay-2007-1 Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

sentry Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Akshay-2007-1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Akshay-2007-1 commented Mar 31, 2026

Uh oh!

RogerZhang888 commented Apr 1, 2026

Uh oh!

ThatLi commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Akshay-2007-1 left a comment •

edited

Loading