From d09fa116bb54350a8298dba3d85ca8826ceefc03 Mon Sep 17 00:00:00 2001 From: CZX Date: Sun, 14 Apr 2024 20:36:10 +0800 Subject: [PATCH 01/15] Improve frame positioning and fix some bugs with arrow animations --- src/features/cseMachine/CseMachineConfig.ts | 2 + .../__snapshots__/CseMachine.tsx.snap | 20 ++--- .../animationComponents/BranchAnimation.tsx | 2 +- .../FrameCreationAnimation.tsx | 14 +-- .../cseMachine/components/Binding.tsx | 7 +- src/features/cseMachine/components/Frame.tsx | 86 ++++++++++--------- .../cseMachine/components/values/FnValue.tsx | 6 ++ .../components/values/GlobalFnValue.tsx | 8 +- 8 files changed, 84 insertions(+), 61 deletions(-) diff --git a/src/features/cseMachine/CseMachineConfig.ts b/src/features/cseMachine/CseMachineConfig.ts index bdbd1c006d..4c210aca2d 100644 --- a/src/features/cseMachine/CseMachineConfig.ts +++ b/src/features/cseMachine/CseMachineConfig.ts @@ -11,6 +11,7 @@ export const Config = Object.freeze({ FrameMinWidth: 100, FramePaddingX: 20, FramePaddingY: 30, + FrameMinGapX: 80, FrameMarginX: 30, FrameMarginY: 10, FrameCornerRadius: 3, @@ -18,6 +19,7 @@ export const Config = Object.freeze({ FnRadius: 15, FnInnerRadius: 3, FnTooltipOpacity: 0.3, + FnTooltipTextPadding: 5, DataMinWidth: 20, DataUnitWidth: 40, diff --git a/src/features/cseMachine/__tests__/__snapshots__/CseMachine.tsx.snap b/src/features/cseMachine/__tests__/__snapshots__/CseMachine.tsx.snap index 9a2414132b..7c96f56183 100644 --- a/src/features/cseMachine/__tests__/__snapshots__/CseMachine.tsx.snap +++ b/src/features/cseMachine/__tests__/__snapshots__/CseMachine.tsx.snap @@ -178,7 +178,7 @@ exports[`CSE Machine Control Stash correctly renders: Control is truncated prope listening={false} > { + this.frameArrows = this.frameArrowAnimations = []; + } + + draw(): React.ReactNode { + // Bindings arrows only gets created when drawn, so `frameArrows` is initialised here instead + const xDiff = this.frame.x() - this.origin.x(); + const yDiff = this.frame.y() - this.origin.y(); + this.frameArrows = this.frame.bindings.flatMap(binding => { if ( binding.value instanceof ArrayValue && isEnvEqual(binding.value.data.environment, this.frame.environment) @@ -111,9 +118,6 @@ export class FrameCreationAnimation extends Animatable { opacity: 0 }); }); - } - - draw(): React.ReactNode { return ( {this.controlTextAnimation.draw()} diff --git a/src/features/cseMachine/components/Binding.tsx b/src/features/cseMachine/components/Binding.tsx index 187f020e08..3920ababa4 100644 --- a/src/features/cseMachine/components/Binding.tsx +++ b/src/features/cseMachine/components/Binding.tsx @@ -65,12 +65,11 @@ export class Binding extends Visible { // derive the width from the right bound of the value this._width = isMainReference(this.value, this) - ? this.value.x() + - this.value.width() - + ? this.value.x() - this.x() + (this.value instanceof FnValue || this.value instanceof GlobalFnValue - ? this.value.tooltipWidth - : 0) + ? this.value.totalWidth + : this.value.width()) : this.key.width(); this._height = Math.max(this.key.height(), this.value.height()); diff --git a/src/features/cseMachine/components/Frame.tsx b/src/features/cseMachine/components/Frame.tsx index 3c5d6221bd..91d0138994 100644 --- a/src/features/cseMachine/components/Frame.tsx +++ b/src/features/cseMachine/components/Frame.tsx @@ -1,3 +1,4 @@ +import { isPrimitive } from '@sentry/utils'; import React from 'react'; import { Group, Rect } from 'react-konva'; @@ -13,7 +14,8 @@ import { getUnreferencedObjects, isClosure, isDataArray, - isPrimitiveData, + isDummyKey, + isSourceObject, isUnassigned } from '../CseMachineUtils'; import { ArrowFromFrame } from './arrows/ArrowFromFrame'; @@ -64,45 +66,18 @@ export class Frame extends Visible implements IHoverable { readonly leftSiblingFrame: Frame | null ) { super(); - this._width = Config.FrameMinWidth; + this.level = envTreeNode.level as Level; + this.parentFrame = envTreeNode.parent?.frame; this.environment = envTreeNode.environment; Frame.envFrameMap.set(this.environment.id, this); - this.parentFrame = envTreeNode.parent?.frame; - this._x = this.level.x(); - // derive the x coordinate from the left sibling frame - this.leftSiblingFrame && - (this._x += - this.leftSiblingFrame.x() + this.leftSiblingFrame.totalWidth + Config.FrameMarginX); - // ensure x coordinate cannot be less than that of parent frame - this.parentFrame && (this._x = Math.max(this._x, this.parentFrame.x())); - this.name = new Text( - frameNames.get(this.environment.name) || this.environment.name, - this.x(), - this.level.y(), - { maxWidth: this.width() } - ); - this._y = this.level.y() + this.name.height() + Config.TextPaddingY / 2; - - // width of the frame = max width of the bindings in the frame + frame padding * 2 (the left and right padding) - let maxBindingWidth = 0; - for (const [key, data] of Object.entries(this.environment.head)) { - const bindingWidth = - Math.max(Config.TextMinWidth, getTextWidth(key + Config.ConstantColon)) + - Config.TextPaddingX + - (isUnassigned(data) - ? Math.max(Config.TextMinWidth, getTextWidth(Config.UnassignedData)) - : isPrimitiveData(data) - ? Math.max(Config.TextMinWidth, getTextWidth(String(data))) - : 0); - maxBindingWidth = Math.max(maxBindingWidth, bindingWidth); - } - this._width = maxBindingWidth + Config.FramePaddingX * 2; - - // initializes bindings (keys + values) - let prevBinding: Binding | null = null; - let totalWidth = this._width; + this._x = this.leftSiblingFrame + ? this.leftSiblingFrame.x() + this.leftSiblingFrame.totalWidth + Config.FrameMarginX + : this.level.x(); + // ensure x coordinate cannot be less than that of parent frame + if (this.parentFrame) this._x = Math.max(this._x, this.parentFrame.x()); + this._y = this.level.y() + Config.FontSize + Config.TextPaddingY / 2; // get all keys and object descriptors of each value inside the head const entries = Object.entries(Object.getOwnPropertyDescriptors(this.environment.head)); @@ -143,26 +118,57 @@ export class Frame extends Visible implements IHoverable { writable: false }; // The key is a number string to "disguise" as a dummy binding - // TODO: revamp the dummy binding behavior, don't rely on numeric keys entries.push([`${i++}`, descriptor]); } + // Find the correct width of the frame before creating the bindings + this._width = Config.FrameMinWidth; + let totalWidth = this._width + Config.FrameMinGapX; + for (const [key, data] of entries) { + if (isDummyKey(key)) continue; + const constant = + this.environment.head[key]?.description === 'const declaration' || !data.writable; + let bindingTextWidth = getTextWidth( + key + (constant ? Config.ConstantColon : Config.VariableColon) + ); + if (isUnassigned(data.value)) { + bindingTextWidth += Config.TextPaddingX + getTextWidth(Config.UnassignedData); + } else if (isPrimitive(data.value)) { + bindingTextWidth += + Config.TextPaddingX + + getTextWidth( + isSourceObject(data.value) + ? data.value.toReplString() + : JSON.stringify(data.value) || String(data.value) + ); + } + this._width = Math.max(this._width, bindingTextWidth + Config.FramePaddingX * 2); + totalWidth = Math.max(totalWidth, this._width + Config.FrameMinGapX); + } + + // Create all the bindings and values + let prevBinding: Binding | null = null; for (const [key, data] of entries) { - // If the value is unassigned, retrieve declaration type from its description, otherwise, retrieve directly from the data's property const constant = this.environment.head[key]?.description === 'const declaration' || !data.writable; const currBinding: Binding = new Binding(key, data.value, this, prevBinding, constant); - this.bindings.push(currBinding); prevBinding = currBinding; + this.bindings.push(currBinding); totalWidth = Math.max(totalWidth, currBinding.width() + Config.FramePaddingX); } this.totalWidth = totalWidth; // derive the height of the frame from the the position of the last binding this._height = prevBinding - ? prevBinding.y() + prevBinding.height() + Config.FramePaddingY - this.y() + ? prevBinding.y() - this.y() + prevBinding.height() + Config.FramePaddingY : Config.FramePaddingY * 2; + this.name = new Text( + frameNames.get(this.environment.name) ?? this.environment.name, + this.x(), + this.level.y(), + { maxWidth: this.width() } + ); this.totalHeight = this.height() + this.name.height() + Config.TextPaddingY / 2; if (this.parentFrame) this.arrow = new ArrowFromFrame(this).to(this.parentFrame); diff --git a/src/features/cseMachine/components/values/FnValue.tsx b/src/features/cseMachine/components/values/FnValue.tsx index 3cc9f9637d..94d283336d 100644 --- a/src/features/cseMachine/components/values/FnValue.tsx +++ b/src/features/cseMachine/components/values/FnValue.tsx @@ -46,6 +46,7 @@ export class FnValue extends Value implements IHoverable { readonly tooltipWidth: number; readonly exportTooltip: string; readonly exportTooltipWidth: number; + readonly totalWidth: number; readonly labelRef: RefObject