Skip to content

Commit bb76d6e

Browse files
fix!: remove MarkerSvg and uses (#8991)
* fix: delete MarkerSvg (marker drawer) * fix: delete marker and cursor SVG elements * chore: format * chore: lint
1 parent acdad98 commit bb76d6e

File tree

16 files changed

+1
-1455
lines changed

16 files changed

+1
-1455
lines changed

core/block_svg.ts

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ import type {IFocusableTree} from './interfaces/i_focusable_tree.js';
5050
import {IIcon} from './interfaces/i_icon.js';
5151
import type {INavigable} from './interfaces/i_navigable.js';
5252
import * as internalConstants from './internal_constants.js';
53-
import {MarkerManager} from './marker_manager.js';
5453
import {Msg} from './msg.js';
5554
import * as renderManagement from './render_management.js';
5655
import {RenderedConnection} from './rendered_connection.js';
@@ -1679,7 +1678,6 @@ export class BlockSvg
16791678
this.tightenChildrenEfficiently();
16801679

16811680
dom.stopTextWidthCache();
1682-
this.updateMarkers_();
16831681
}
16841682

16851683
/**
@@ -1699,44 +1697,6 @@ export class BlockSvg
16991697
if (this.nextConnection) this.nextConnection.tightenEfficiently();
17001698
}
17011699

1702-
/** Redraw any attached marker or cursor svgs if needed. */
1703-
protected updateMarkers_() {
1704-
if (this.workspace.keyboardAccessibilityMode && this.pathObject.cursorSvg) {
1705-
this.workspace.getCursor()!.draw();
1706-
}
1707-
if (this.workspace.keyboardAccessibilityMode && this.pathObject.markerSvg) {
1708-
// TODO(#4592): Update all markers on the block.
1709-
this.workspace.getMarker(MarkerManager.LOCAL_MARKER)!.draw();
1710-
}
1711-
for (const input of this.inputList) {
1712-
for (const field of input.fieldRow) {
1713-
field.updateMarkers_();
1714-
}
1715-
}
1716-
}
1717-
1718-
/**
1719-
* Add the cursor SVG to this block's SVG group.
1720-
*
1721-
* @param cursorSvg The SVG root of the cursor to be added to the block SVG
1722-
* group.
1723-
* @internal
1724-
*/
1725-
setCursorSvg(cursorSvg: SVGElement) {
1726-
this.pathObject.setCursorSvg(cursorSvg);
1727-
}
1728-
1729-
/**
1730-
* Add the marker SVG to this block's SVG group.
1731-
*
1732-
* @param markerSvg The SVG root of the marker to be added to the block SVG
1733-
* group.
1734-
* @internal
1735-
*/
1736-
setMarkerSvg(markerSvg: SVGElement) {
1737-
this.pathObject.setMarkerSvg(markerSvg);
1738-
}
1739-
17401700
/**
17411701
* Returns a bounding box describing the dimensions of this block
17421702
* and any blocks stacked below it.

core/field.ts

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import type {IKeyboardAccessible} from './interfaces/i_keyboard_accessible.js';
3131
import type {INavigable} from './interfaces/i_navigable.js';
3232
import type {IRegistrable} from './interfaces/i_registrable.js';
3333
import {ISerializable} from './interfaces/i_serializable.js';
34-
import {MarkerManager} from './marker_manager.js';
3534
import type {ConstantProvider} from './renderers/common/constants.js';
3635
import type {KeyboardShortcut} from './shortcut_registry.js';
3736
import * as Tooltip from './tooltip.js';
@@ -113,18 +112,6 @@ export abstract class Field<T = any>
113112
private tooltip: Tooltip.TipInfo | null = null;
114113
protected size_: Size;
115114

116-
/**
117-
* Holds the cursors svg element when the cursor is attached to the field.
118-
* This is null if there is no cursor on the field.
119-
*/
120-
private cursorSvg: SVGElement | null = null;
121-
122-
/**
123-
* Holds the markers svg element when the marker is attached to the field.
124-
* This is null if there is no marker on the field.
125-
*/
126-
private markerSvg: SVGElement | null = null;
127-
128115
/** The rendered field's SVG group element. */
129116
protected fieldGroup_: SVGGElement | null = null;
130117

@@ -1358,64 +1345,6 @@ export abstract class Field<T = any>
13581345
return false;
13591346
}
13601347

1361-
/**
1362-
* Add the cursor SVG to this fields SVG group.
1363-
*
1364-
* @param cursorSvg The SVG root of the cursor to be added to the field group.
1365-
* @internal
1366-
*/
1367-
setCursorSvg(cursorSvg: SVGElement) {
1368-
if (!cursorSvg) {
1369-
this.cursorSvg = null;
1370-
return;
1371-
}
1372-
1373-
if (!this.fieldGroup_) {
1374-
throw new Error(`The field group is ${this.fieldGroup_}.`);
1375-
}
1376-
this.fieldGroup_.appendChild(cursorSvg);
1377-
this.cursorSvg = cursorSvg;
1378-
}
1379-
1380-
/**
1381-
* Add the marker SVG to this fields SVG group.
1382-
*
1383-
* @param markerSvg The SVG root of the marker to be added to the field group.
1384-
* @internal
1385-
*/
1386-
setMarkerSvg(markerSvg: SVGElement) {
1387-
if (!markerSvg) {
1388-
this.markerSvg = null;
1389-
return;
1390-
}
1391-
1392-
if (!this.fieldGroup_) {
1393-
throw new Error(`The field group is ${this.fieldGroup_}.`);
1394-
}
1395-
this.fieldGroup_.appendChild(markerSvg);
1396-
this.markerSvg = markerSvg;
1397-
}
1398-
1399-
/**
1400-
* Redraw any attached marker or cursor svgs if needed.
1401-
*
1402-
* @internal
1403-
*/
1404-
updateMarkers_() {
1405-
const block = this.getSourceBlock();
1406-
if (!block) {
1407-
throw new UnattachedFieldError();
1408-
}
1409-
const workspace = block.workspace as WorkspaceSvg;
1410-
if (workspace.keyboardAccessibilityMode && this.cursorSvg) {
1411-
workspace.getCursor()!.draw();
1412-
}
1413-
if (workspace.keyboardAccessibilityMode && this.markerSvg) {
1414-
// TODO(#4592): Update all markers on the field.
1415-
workspace.getMarker(MarkerManager.LOCAL_MARKER)!.draw();
1416-
}
1417-
}
1418-
14191348
/** See IFocusableNode.getFocusableElement. */
14201349
getFocusableElement(): HTMLElement | SVGElement {
14211350
if (!this.fieldGroup_) {

core/flyout_button.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -348,15 +348,6 @@ export class FlyoutButton
348348
}
349349
}
350350

351-
/**
352-
* Required by IASTNodeLocationSvg, but not used. A marker cannot be set on a
353-
* button. If the 'mark' shortcut is used on a button, its associated callback
354-
* function is triggered.
355-
*/
356-
setMarkerSvg() {
357-
throw new Error('Attempted to set a marker on a button.');
358-
}
359-
360351
/**
361352
* Do something when the button is clicked.
362353
*

core/interfaces/i_ast_node_location_svg.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,4 @@ import type {IASTNodeLocation} from './i_ast_node_location.js';
1111
/**
1212
* An AST node location SVG interface.
1313
*/
14-
export interface IASTNodeLocationSvg extends IASTNodeLocation {
15-
/**
16-
* Add the marker SVG to this node's SVG group.
17-
*
18-
* @param markerSvg The SVG root of the marker to be added to the SVG group.
19-
*/
20-
setMarkerSvg(markerSvg: SVGElement | null): void;
21-
22-
/**
23-
* Add the cursor SVG to this node's SVG group.
24-
*
25-
* @param cursorSvg The SVG root of the cursor to be added to the SVG group.
26-
*/
27-
setCursorSvg(cursorSvg: SVGElement | null): void;
28-
}
14+
export interface IASTNodeLocationSvg extends IASTNodeLocation {}

core/keyboard_nav/line_cursor.ts

Lines changed: 0 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,8 @@ import {isFocusableNode} from '../interfaces/i_focusable_node.js';
3030
import type {INavigable} from '../interfaces/i_navigable.js';
3131
import * as registry from '../registry.js';
3232
import {RenderedConnection} from '../rendered_connection.js';
33-
import type {MarkerSvg} from '../renderers/common/marker_svg.js';
34-
import type {PathObject} from '../renderers/zelos/path_object.js';
3533
import {Renderer} from '../renderers/zelos/renderer.js';
36-
import * as dom from '../utils/dom.js';
3734
import {WorkspaceSvg} from '../workspace_svg.js';
38-
import {ASTNode} from './ast_node.js';
3935
import {BlockNavigationPolicy} from './block_navigation_policy.js';
4036
import {ConnectionNavigationPolicy} from './connection_navigation_policy.js';
4137
import {FieldNavigationPolicy} from './field_navigation_policy.js';
@@ -574,40 +570,6 @@ export class LineCursor extends Marker {
574570
return super.getCurNode();
575571
}
576572

577-
/**
578-
* Sets the object in charge of drawing the marker.
579-
*
580-
* We want to customize drawing, so rather than directly setting the given
581-
* object, we instead set a wrapper proxy object that passes through all
582-
* method calls and property accesses except for draw(), which it delegates
583-
* to the drawMarker() method in this class.
584-
*
585-
* @param drawer The object ~in charge of drawing the marker.
586-
*/
587-
override setDrawer(drawer: MarkerSvg) {
588-
const altDraw = function (
589-
this: LineCursor,
590-
oldNode: ASTNode | null,
591-
curNode: ASTNode | null,
592-
) {
593-
// Pass the unproxied, raw drawer object so that drawMarker can call its
594-
// `draw()` method without triggering infinite recursion.
595-
this.drawMarker(oldNode, curNode, drawer);
596-
}.bind(this);
597-
598-
super.setDrawer(
599-
new Proxy(drawer, {
600-
get(target: typeof drawer, prop: keyof typeof drawer) {
601-
if (prop === 'draw') {
602-
return altDraw;
603-
}
604-
605-
return target[prop];
606-
},
607-
}),
608-
);
609-
}
610-
611573
/**
612574
* Set the location of the cursor and draw it.
613575
*
@@ -631,124 +593,6 @@ export class LineCursor extends Marker {
631593
}
632594
}
633595

634-
/**
635-
* Draw this cursor's marker.
636-
*
637-
* This is a wrapper around this.drawer.draw (usually implemented by
638-
* MarkerSvg.prototype.draw) that will, if newNode is a BLOCK node,
639-
* instead call `setSelected` to select it (if it's a regular block)
640-
* or `addSelect` (if it's a shadow block, since shadow blocks can't
641-
* be selected) instead of using the normal drawer logic.
642-
*
643-
* TODO(#142): The selection and fake-selection code was originally
644-
* a hack added for testing on October 28 2024, because the default
645-
* drawer (MarkerSvg) behaviour in Zelos was to draw a box around
646-
* the block and all attached child blocks, which was confusing when
647-
* navigating stacks.
648-
*
649-
* Since then we have decided that we probably _do_ in most cases
650-
* want navigating to a block to select the block, but more
651-
* particularly that we want navigation to move _focus_. Replace
652-
* this selection hack with non-hacky changing of focus once that's
653-
* possible.
654-
*
655-
* @param oldNode The previous node.
656-
* @param curNode The current node.
657-
* @param realDrawer The object ~in charge of drawing the marker.
658-
*/
659-
private drawMarker(
660-
oldNode: ASTNode | null,
661-
curNode: ASTNode | null,
662-
realDrawer: MarkerSvg,
663-
) {
664-
// If old node was a block, unselect it or remove fake selection.
665-
if (oldNode?.getType() === ASTNode.types.BLOCK) {
666-
const block = oldNode.getLocation() as BlockSvg;
667-
if (!block.isShadow()) {
668-
// Selection should already be in sync.
669-
} else {
670-
block.removeSelect();
671-
}
672-
}
673-
674-
if (this.isZelos && oldNode && this.isValueInputConnection(oldNode)) {
675-
this.hideAtInput(oldNode);
676-
}
677-
678-
const curNodeType = curNode?.getType();
679-
const isZelosInputConnection =
680-
this.isZelos && curNode && this.isValueInputConnection(curNode);
681-
682-
// If drawing can't be handled locally, just use the drawer.
683-
if (curNodeType !== ASTNode.types.BLOCK && !isZelosInputConnection) {
684-
realDrawer.draw(oldNode, curNode);
685-
return;
686-
}
687-
688-
// Hide any visible marker SVG and instead do some manual rendering.
689-
realDrawer.hide();
690-
691-
if (isZelosInputConnection) {
692-
this.showAtInput(curNode);
693-
} else if (curNode && curNodeType === ASTNode.types.BLOCK) {
694-
const block = curNode.getLocation() as BlockSvg;
695-
if (!block.isShadow()) {
696-
// Selection should already be in sync.
697-
} else {
698-
block.addSelect();
699-
block.getParent()?.removeSelect();
700-
}
701-
}
702-
703-
// Call MarkerSvg.prototype.fireMarkerEvent like
704-
// MarkerSvg.prototype.draw would (even though it's private).
705-
(realDrawer as any)?.fireMarkerEvent?.(oldNode, curNode);
706-
}
707-
708-
/**
709-
* Check whether the node represents a value input connection.
710-
*
711-
* @param node The node to check
712-
* @returns True if the node represents a value input connection.
713-
*/
714-
private isValueInputConnection(node: ASTNode) {
715-
if (node?.getType() !== ASTNode.types.INPUT) return false;
716-
const connection = node.getLocation() as RenderedConnection;
717-
return connection.type === ConnectionType.INPUT_VALUE;
718-
}
719-
720-
/**
721-
* Hide the cursor rendering at the given input node.
722-
*
723-
* @param node The input node to hide.
724-
*/
725-
private hideAtInput(node: ASTNode) {
726-
const inputConnection = node.getLocation() as RenderedConnection;
727-
const sourceBlock = inputConnection.getSourceBlock() as BlockSvg;
728-
const input = inputConnection.getParentInput();
729-
if (input) {
730-
const pathObject = sourceBlock.pathObject as PathObject;
731-
const outlinePath = pathObject.getOutlinePath(input.name);
732-
dom.removeClass(outlinePath, 'inputActiveFocus');
733-
}
734-
}
735-
736-
/**
737-
* Show the cursor rendering at the given input node.
738-
*
739-
* @param node The input node to show.
740-
*/
741-
private showAtInput(node: ASTNode) {
742-
const inputConnection = node.getLocation() as RenderedConnection;
743-
const sourceBlock = inputConnection.getSourceBlock() as BlockSvg;
744-
const input = inputConnection.getParentInput();
745-
if (input) {
746-
const pathObject = sourceBlock.pathObject as PathObject;
747-
const outlinePath = pathObject.getOutlinePath(input.name);
748-
dom.addClass(outlinePath, 'inputActiveFocus');
749-
}
750-
}
751-
752596
/**
753597
* Updates the current node to match the selection.
754598
*

0 commit comments

Comments
 (0)