fix: delete MarkerSvg (marker drawer)

Author: rachel-fenichel

core/block_svg.ts

@@ -1679,7 +1679,6 @@ export class BlockSvg
     this.tightenChildrenEfficiently();
 
     dom.stopTextWidthCache();
-    this.updateMarkers_();
   }
 
   /**
@@ -1699,22 +1698,6 @@ export class BlockSvg
     if (this.nextConnection) this.nextConnection.tightenEfficiently();
   }
 
-  /** Redraw any attached marker or cursor svgs if needed. */
-  protected updateMarkers_() {
-    if (this.workspace.keyboardAccessibilityMode && this.pathObject.cursorSvg) {
-      this.workspace.getCursor()!.draw();
-    }
-    if (this.workspace.keyboardAccessibilityMode && this.pathObject.markerSvg) {
-      // TODO(#4592): Update all markers on the block.
-      this.workspace.getMarker(MarkerManager.LOCAL_MARKER)!.draw();
-    }
-    for (const input of this.inputList) {
-      for (const field of input.fieldRow) {
-        field.updateMarkers_();
-      }
-    }
-  }
-
   /**
    * Add the cursor SVG to this block's SVG group.
    *

core/field.ts

@@ -1396,26 +1396,6 @@ export abstract class Field<T = any>
     this.markerSvg = markerSvg;
   }
 
-  /**
-   * Redraw any attached marker or cursor svgs if needed.
-   *
-   * @internal
-   */
-  updateMarkers_() {
-    const block = this.getSourceBlock();
-    if (!block) {
-      throw new UnattachedFieldError();
-    }
-    const workspace = block.workspace as WorkspaceSvg;
-    if (workspace.keyboardAccessibilityMode && this.cursorSvg) {
-      workspace.getCursor()!.draw();
-    }
-    if (workspace.keyboardAccessibilityMode && this.markerSvg) {
-      // TODO(#4592): Update all markers on the field.
-      workspace.getMarker(MarkerManager.LOCAL_MARKER)!.draw();
-    }
-  }
-
   /** See IFocusableNode.getFocusableElement. */
   getFocusableElement(): HTMLElement | SVGElement {
     if (!this.fieldGroup_) {

core/keyboard_nav/line_cursor.ts

@@ -30,7 +30,6 @@ import {isFocusableNode} from '../interfaces/i_focusable_node.js';
 import type {INavigable} from '../interfaces/i_navigable.js';
 import * as registry from '../registry.js';
 import {RenderedConnection} from '../rendered_connection.js';
-import type {MarkerSvg} from '../renderers/common/marker_svg.js';
 import type {PathObject} from '../renderers/zelos/path_object.js';
 import {Renderer} from '../renderers/zelos/renderer.js';
 import * as dom from '../utils/dom.js';
@@ -574,40 +573,6 @@ export class LineCursor extends Marker {
     return super.getCurNode();
   }
 
-  /**
-   * Sets the object in charge of drawing the marker.
-   *
-   * We want to customize drawing, so rather than directly setting the given
-   * object, we instead set a wrapper proxy object that passes through all
-   * method calls and property accesses except for draw(), which it delegates
-   * to the drawMarker() method in this class.
-   *
-   * @param drawer The object ~in charge of drawing the marker.
-   */
-  override setDrawer(drawer: MarkerSvg) {
-    const altDraw = function (
-      this: LineCursor,
-      oldNode: ASTNode | null,
-      curNode: ASTNode | null,
-    ) {
-      // Pass the unproxied, raw drawer object so that drawMarker can call its
-      // `draw()` method without triggering infinite recursion.
-      this.drawMarker(oldNode, curNode, drawer);
-    }.bind(this);
-
-    super.setDrawer(
-      new Proxy(drawer, {
-        get(target: typeof drawer, prop: keyof typeof drawer) {
-          if (prop === 'draw') {
-            return altDraw;
-          }
-
-          return target[prop];
-        },
-      }),
-    );
-  }
-
   /**
    * Set the location of the cursor and draw it.
    *
@@ -630,125 +595,7 @@ export class LineCursor extends Marker {
       );
     }
   }
-
-  /**
-   * Draw this cursor's marker.
-   *
-   * This is a wrapper around this.drawer.draw (usually implemented by
-   * MarkerSvg.prototype.draw) that will, if newNode is a BLOCK node,
-   * instead call `setSelected` to select it (if it's a regular block)
-   * or `addSelect` (if it's a shadow block, since shadow blocks can't
-   * be selected) instead of using the normal drawer logic.
-   *
-   * TODO(#142): The selection and fake-selection code was originally
-   * a hack added for testing on October 28 2024, because the default
-   * drawer (MarkerSvg) behaviour in Zelos was to draw a box around
-   * the block and all attached child blocks, which was confusing when
-   * navigating stacks.
-   *
-   * Since then we have decided that we probably _do_ in most cases
-   * want navigating to a block to select the block, but more
-   * particularly that we want navigation to move _focus_.  Replace
-   * this selection hack with non-hacky changing of focus once that's
-   * possible.
-   *
-   * @param oldNode The previous node.
-   * @param curNode The current node.
-   * @param realDrawer The object ~in charge of drawing the marker.
-   */
-  private drawMarker(
-    oldNode: ASTNode | null,
-    curNode: ASTNode | null,
-    realDrawer: MarkerSvg,
-  ) {
-    // If old node was a block, unselect it or remove fake selection.
-    if (oldNode?.getType() === ASTNode.types.BLOCK) {
-      const block = oldNode.getLocation() as BlockSvg;
-      if (!block.isShadow()) {
-        // Selection should already be in sync.
-      } else {
-        block.removeSelect();
-      }
-    }
-
-    if (this.isZelos && oldNode && this.isValueInputConnection(oldNode)) {
-      this.hideAtInput(oldNode);
-    }
-
-    const curNodeType = curNode?.getType();
-    const isZelosInputConnection =
-      this.isZelos && curNode && this.isValueInputConnection(curNode);
-
-    // If drawing can't be handled locally, just use the drawer.
-    if (curNodeType !== ASTNode.types.BLOCK && !isZelosInputConnection) {
-      realDrawer.draw(oldNode, curNode);
-      return;
-    }
-
-    // Hide any visible marker SVG and instead do some manual rendering.
-    realDrawer.hide();
-
-    if (isZelosInputConnection) {
-      this.showAtInput(curNode);
-    } else if (curNode && curNodeType === ASTNode.types.BLOCK) {
-      const block = curNode.getLocation() as BlockSvg;
-      if (!block.isShadow()) {
-        // Selection should already be in sync.
-      } else {
-        block.addSelect();
-        block.getParent()?.removeSelect();
-      }
-    }
-
-    // Call MarkerSvg.prototype.fireMarkerEvent like
-    // MarkerSvg.prototype.draw would (even though it's private).
-    (realDrawer as any)?.fireMarkerEvent?.(oldNode, curNode);
-  }
-
-  /**
-   * Check whether the node represents a value input connection.
-   *
-   * @param node The node to check
-   * @returns True if the node represents a value input connection.
-   */
-  private isValueInputConnection(node: ASTNode) {
-    if (node?.getType() !== ASTNode.types.INPUT) return false;
-    const connection = node.getLocation() as RenderedConnection;
-    return connection.type === ConnectionType.INPUT_VALUE;
-  }
-
-  /**
-   * Hide the cursor rendering at the given input node.
-   *
-   * @param node The input node to hide.
-   */
-  private hideAtInput(node: ASTNode) {
-    const inputConnection = node.getLocation() as RenderedConnection;
-    const sourceBlock = inputConnection.getSourceBlock() as BlockSvg;
-    const input = inputConnection.getParentInput();
-    if (input) {
-      const pathObject = sourceBlock.pathObject as PathObject;
-      const outlinePath = pathObject.getOutlinePath(input.name);
-      dom.removeClass(outlinePath, 'inputActiveFocus');
-    }
-  }
-
-  /**
-   * Show the cursor rendering at the given input node.
-   *
-   * @param node The input node to show.
-   */
-  private showAtInput(node: ASTNode) {
-    const inputConnection = node.getLocation() as RenderedConnection;
-    const sourceBlock = inputConnection.getSourceBlock() as BlockSvg;
-    const input = inputConnection.getParentInput();
-    if (input) {
-      const pathObject = sourceBlock.pathObject as PathObject;
-      const outlinePath = pathObject.getOutlinePath(input.name);
-      dom.addClass(outlinePath, 'inputActiveFocus');
-    }
-  }
-
+  
   /**
    * Updates the current node to match the selection.
    *

core/keyboard_nav/marker.ts

@@ -17,7 +17,6 @@ import {Field} from '../field.js';
 import {FlyoutButton} from '../flyout_button.js';
 import type {INavigable} from '../interfaces/i_navigable.js';
 import {RenderedConnection} from '../rendered_connection.js';
-import type {MarkerSvg} from '../renderers/common/marker_svg.js';
 import {Coordinate} from '../utils/coordinate.js';
 import {WorkspaceSvg} from '../workspace_svg.js';
 import {ASTNode} from './ast_node.js';
@@ -33,33 +32,9 @@ export class Marker {
   /** The current location of the marker. */
   protected curNode: INavigable<any> | null = null;
 
-  /**
-   * The object in charge of drawing the visual representation of the current
-   * node.
-   */
-  private drawer: MarkerSvg | null = null;
-
   /** The type of the marker. */
   type = 'marker';
 
-  /**
-   * Sets the object in charge of drawing the marker.
-   *
-   * @param drawer The object in charge of drawing the marker.
-   */
-  setDrawer(drawer: MarkerSvg) {
-    this.drawer = drawer;
-  }
-
-  /**
-   * Get the current drawer for the marker.
-   *
-   * @returns The object in charge of drawing the marker.
-   */
-  getDrawer(): MarkerSvg | null {
-    return this.drawer;
-  }
-
   /**
    * Gets the current location of the marker.
    *
@@ -77,28 +52,10 @@ export class Marker {
   setCurNode(newNode: INavigable<any> | null) {
     const oldNode = this.curNode;
     this.curNode = newNode;
-    this.drawer?.draw(this.toASTNode(oldNode), this.toASTNode(this.curNode));
-  }
-
-  /**
-   * Redraw the current marker.
-   *
-   * @internal
-   */
-  draw() {
-    const node = this.toASTNode(this.curNode);
-    this.drawer?.draw(node, node);
-  }
-
-  /** Hide the marker SVG. */
-  hide() {
-    this.drawer?.hide();
   }
 
   /** Dispose of this marker. */
   dispose() {
-    this.drawer?.dispose();
-    this.drawer = null;
     this.curNode = null;
   }
 

core/marker_manager.ts

@@ -50,11 +50,6 @@ export class MarkerManager {
     if (this.markers.has(id)) {
       this.unregisterMarker(id);
     }
-    const drawer = this.workspace
-      .getRenderer()
-      .makeMarkerDrawer(this.workspace, marker);
-    marker.setDrawer(drawer);
-    this.setMarkerSvg(drawer.createDom());
     this.markers.set(id, marker);
   }
 
@@ -105,15 +100,7 @@ export class MarkerManager {
    * @param cursor The cursor used to move around this workspace.
    */
   setCursor(cursor: LineCursor) {
-    this.cursor?.getDrawer()?.dispose();
     this.cursor = cursor;
-    if (this.cursor) {
-      const drawer = this.workspace
-        .getRenderer()
-        .makeMarkerDrawer(this.workspace, this.cursor);
-      this.cursor.setDrawer(drawer);
-      this.setCursorSvg(drawer.createDom());
-    }
   }
 
   /**
@@ -157,17 +144,6 @@ export class MarkerManager {
     }
   }
 
-  /**
-   * Redraw the attached cursor SVG if needed.
-   *
-   * @internal
-   */
-  updateMarkers() {
-    if (this.workspace.keyboardAccessibilityMode && this.cursorSvg) {
-      this.workspace.getCursor()!.draw();
-    }
-  }
-
   /**
    * Dispose of the marker manager.
    * Go through and delete all markers associated with this marker manager.

core/renderers/common/block_rendering.ts

@@ -33,7 +33,6 @@ import {Types} from '../measurables/types.js';
 import {Drawer} from './drawer.js';
 import type {IPathObject} from './i_path_object.js';
 import {RenderInfo} from './info.js';
-import {MarkerSvg} from './marker_svg.js';
 import {PathObject} from './path_object.js';
 import {Renderer} from './renderer.js';
 
@@ -94,7 +93,6 @@ export {
   InRowSpacer,
   IPathObject,
   JaggedEdge,
-  MarkerSvg,
   Measurable,
   NextConnection,
   OutputConnection,