-
-
Notifications
You must be signed in to change notification settings - Fork 67
Feature/variable browser rework #638
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
base: master
Are you sure you want to change the base?
Changes from all commits
f42363e
8c5f471
0216610
3508530
740842d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| export class Connector { | ||
| type: string; | ||
| position: (x: number, y: number, width: number, height: number) => [number,number]; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { ElementType } from './elementtype'; | ||
| import { Connector } from './connector'; | ||
| import { Dimensions } from './dimensions'; | ||
|
|
||
| export class DataPoint { | ||
| label: string; | ||
| x?: number; | ||
| y?: number; | ||
| fx?: number; | ||
| fy?: number; | ||
| value: number; | ||
| exponent: number; | ||
| rotation: number; | ||
| anchorx?: number; | ||
| anchory?: number; | ||
| type: ElementType; | ||
| connectors: Connector[]; | ||
| canvasPosition; | ||
| symbol; | ||
| dimensions: Dimensions; | ||
| hidden = false; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| export class Dimensions { | ||
| length: number; | ||
| boundingbox: number[]; | ||
| labelbox: number[]; | ||
| valuebox: number[]; | ||
| exponentbox: number[]; | ||
| valueline?: number[][]; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { Connector } from './connector'; | ||
| import { Dimensions } from './dimensions'; | ||
|
|
||
| export class ElementType { | ||
| name: string; | ||
| points: (x: number, y: number, width: number, height: number) => string; | ||
| size: number[]; | ||
| connectors: Connector[]; | ||
| getDimensions: (length: number) => Dimensions; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <svg:path #linepath [style.display]="invisibleLine ? 'none' : 'inherit'"></svg:path> | ||
|
|
||
| <svg:circle #startpoint [attr.cx]="start[0]" [attr.cy]="start[1]" r="0.00001"></svg:circle> | ||
| <svg:circle #endpoint [attr.cx]="end[0]" [attr.cy]="end[1]" r="0.00001"></svg:circle> | ||
|
|
||
| <svg:circle #midpoint dragdrop *ngFor="let p of dotPositions; let i = index" | ||
| [dragfunctions]="dragfunctions(p, i)" [attr.cx]="p.x" [attr.cy]="p.y" r="7"></svg:circle> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,113 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| import { CommonModule } from '@angular/common'; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { ElementRef, Input, OnChanges, ViewChild, Component, SimpleChanges, Output, EventEmitter, ViewChildren } from '@angular/core'; | ||||||||||||||||||||||||||||||||||||||||||||||
| import * as d3 from 'd3'; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Component({ | ||||||||||||||||||||||||||||||||||||||||||||||
| selector: '[d3line]', | ||||||||||||||||||||||||||||||||||||||||||||||
| templateUrl: './d3line.component.html', | ||||||||||||||||||||||||||||||||||||||||||||||
| styleUrls: ['./d3line.component.css'], | ||||||||||||||||||||||||||||||||||||||||||||||
| imports: [CommonModule] | ||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: No, the imports metadata field is not valid on a component without standalone: true. It is only available for standalone components. Additionally, a non-standalone component cannot be imported directly into another standalone component's imports array. Non-standalone components must be declared and exported from an NgModule, and that NgModule must be imported into the standalone component's imports array instead. Citations:
🏁 Script executed: # Find the SvgCanvasComponent file
fd -e ts SvgCanvasRepository: highperformancecoder/minsky Length of output: 53 🏁 Script executed: # Search for D3LineComponent imports in the codebase
rg "D3LineComponent" --type ts -B 2 -A 2Repository: highperformancecoder/minsky Length of output: 1957 🏁 Script executed: # Check the SvgCanvasComponent definition to see its imports
rg "selector.*svg-canvas" --type ts -B 5 -A 15Repository: highperformancecoder/minsky Length of output: 5169 Mark this component as standalone before importing it directly.
Suggested fix `@Component`({
selector: '[d3line]',
+ standalone: true,
templateUrl: './d3line.component.html',
styleUrls: ['./d3line.component.css'],
imports: [CommonModule]
})📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| export class D3LineComponent implements OnChanges { | ||||||||||||||||||||||||||||||||||||||||||||||
| @ViewChild('linepath', {static: true}) | ||||||||||||||||||||||||||||||||||||||||||||||
| linepath: ElementRef; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @ViewChild('startpoint', {static: true}) | ||||||||||||||||||||||||||||||||||||||||||||||
| startpoint: ElementRef; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @ViewChild('endpoint', {static: true}) | ||||||||||||||||||||||||||||||||||||||||||||||
| endpoint: ElementRef; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @ViewChildren('midpoint') | ||||||||||||||||||||||||||||||||||||||||||||||
| midpoints; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||
| start: [number,number,number]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||
| end: [number,number,number]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||
| dotsAt: number[]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| dotPositions: any[] = []; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||
| invisibleLine = false; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Input() | ||||||||||||||||||||||||||||||||||||||||||||||
| draggable = true; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @Output() | ||||||||||||||||||||||||||||||||||||||||||||||
| pointMoved = new EventEmitter<any>(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| constructor() { | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| ngOnChanges(changes: SimpleChanges) { | ||||||||||||||||||||||||||||||||||||||||||||||
| if(this.start && this.end && (changes.start || changes.end)) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const points: any = [this.start]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const pointDistance = Math.sqrt(Math.pow(this.end[0] - this.start[0], 2) + Math.pow(this.end[1] - this.start[1], 2)) / 4; | ||||||||||||||||||||||||||||||||||||||||||||||
| if(this.start[2] !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||
| points.push([this.start[0] + Math.cos(this.start[2]) * pointDistance, this.start[1] + Math.sin(this.start[2]) * pointDistance]); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| if(this.end[2] !== undefined) { | ||||||||||||||||||||||||||||||||||||||||||||||
| points.push([this.end[0] + Math.cos(this.end[2]) * pointDistance, this.end[1] + Math.sin(this.end[2]) * pointDistance]); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| points.push(this.end); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| this.linepath.nativeElement.setAttribute('d', d3.line().curve(d3.curveBasis)(points)); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| if(changes.dotsAt) { | ||||||||||||||||||||||||||||||||||||||||||||||
| this.handleDotsChange(); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| handleDotsChange() { | ||||||||||||||||||||||||||||||||||||||||||||||
| const path = this.linepath.nativeElement; | ||||||||||||||||||||||||||||||||||||||||||||||
| const length = path.getTotalLength(); | ||||||||||||||||||||||||||||||||||||||||||||||
| this.dotPositions = this.dotsAt.map(percentage => { | ||||||||||||||||||||||||||||||||||||||||||||||
| return path.getPointAtLength(percentage * length); | ||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| dragfunctions(p, i) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const startBox = this.startpoint.nativeElement.getBBox(); | ||||||||||||||||||||||||||||||||||||||||||||||
| const endBox = this.endpoint.nativeElement.getBBox(); | ||||||||||||||||||||||||||||||||||||||||||||||
| const start = [startBox.x + startBox.width / 2, startBox.y + startBox.height / 2]; | ||||||||||||||||||||||||||||||||||||||||||||||
| const end = [endBox.x + endBox.width / 2, endBox.y + endBox.height / 2]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||
| dragstarted: () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| dragged: () => { | ||||||||||||||||||||||||||||||||||||||||||||||
| if(this.draggable) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const x = d3.event.x; | ||||||||||||||||||||||||||||||||||||||||||||||
| const y = d3.event.y; | ||||||||||||||||||||||||||||||||||||||||||||||
Check noticeCode scanning / CodeQL Unused variable, import, function or class Note
Unused variable y.
|
||||||||||||||||||||||||||||||||||||||||||||||
| const point = [x,1000]; | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const a1 = Math.max(0, Math.PI / 2 - this.getAngle(end, start, point)); | ||||||||||||||||||||||||||||||||||||||||||||||
| const a2 = Math.max(0, Math.PI / 2 - this.getAngle(start, end, point)); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| this.dotsAt[i] = a1 / (a1 + a2); | ||||||||||||||||||||||||||||||||||||||||||||||
| this.pointMoved.emit(this.dotsAt[i]); | ||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+86
to
+96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect the repo's D3 version and locate legacy global-event usage.
fd -i 'package.json' . -x rg -n '"d3"\s*:' {}
fd -i 'package-lock.json' . -x rg -n '"d3"\s*:' {}
rg -n -C2 "d3\.event|dragstarted:|dragged:|dragended:" gui-js/libs/ui-components/src/lib/svg-canvasRepository: highperformancecoder/minsky Length of output: 6980 Replace hardcoded Y coordinate with live drag position. Line 89 captures The codebase uses D3 v5.16.0, where Fix- const point = [x,1000];
+ const point = [x, y];📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: CodeQL[notice] 89-89: Unused variable, import, function or class 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||
| dragended: () => { | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| getAngle(x, y, z) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const a = [z[0] - y[0], z[1] - y[1]]; | ||||||||||||||||||||||||||||||||||||||||||||||
| const b = [x[0] - y[0], x[1] - y[1]]; | ||||||||||||||||||||||||||||||||||||||||||||||
| const al = Math.sqrt(Math.pow(a[0],2)+Math.pow(a[1],2)); | ||||||||||||||||||||||||||||||||||||||||||||||
| const bl = Math.sqrt(Math.pow(b[0],2)+Math.pow(b[1],2)); | ||||||||||||||||||||||||||||||||||||||||||||||
| const dotproduct = a[0] * b[0] + a[1] * b[1]; | ||||||||||||||||||||||||||||||||||||||||||||||
| return Math.acos(dotproduct / (al * bl)); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| :host { | ||
| height: 100%; | ||
| width: 100%; | ||
| display: flex; | ||
| flex-direction: column; | ||
| justify-content: space-evenly; | ||
| align-items: center; | ||
| overflow: visible; | ||
| } | ||
|
|
||
| .namelabel, .descriptionlabel, .valuelabel { | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| -webkit-user-select: none; | ||
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| text-align: center; | ||
| } | ||
|
|
||
| .descriptionlabel { | ||
| flex-grow: 6; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,14 @@ | ||||||||||||||
| <div class="latex" latex [equation]="data.symbol" [style.fontSize]="fontSize"> | ||||||||||||||
| </div> | ||||||||||||||
|
|
||||||||||||||
| <div *ngIf="lines > 1" class="valuelabel" [style.fontSize]="fontSize"> | ||||||||||||||
| {{data.value.toFixed(4)}} | ||||||||||||||
| </div> | ||||||||||||||
|
Comment on lines
+4
to
+6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard against undefined The 🛡️ Proposed fix: add null guard <div *ngIf="lines > 1" class="valuelabel" [style.fontSize]="fontSize">
- {{data.value.toFixed(4)}}
+ {{data.value != null ? data.value.toFixed(4) : ''}}
</div>Alternatively, consider using the optional chaining operator if the Angular compiler version supports it: {{data.value?.toFixed(4) ?? ''}}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| <div *ngIf="lines > 2" class="namelabel" [style.fontSize]="fontSize"> | ||||||||||||||
| {{data.name}} | ||||||||||||||
| </div> | ||||||||||||||
|
|
||||||||||||||
| <div *ngIf="lines > 3" class="descriptionlabel" [style.fontSize]="fontSize"> | ||||||||||||||
| {{data.description}} | ||||||||||||||
| </div> | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,22 @@ | ||||||||||||||||||||
| import { Component, Input } from '@angular/core'; | ||||||||||||||||||||
| import { LatexDirective } from '../../directives/latex.directive'; | ||||||||||||||||||||
| import { CommonModule } from '@angular/common'; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Component({ | ||||||||||||||||||||
| selector: 'elementlabel', | ||||||||||||||||||||
| templateUrl: './elementlabel.component.html', | ||||||||||||||||||||
| styleUrls: ['./elementlabel.component.css'], | ||||||||||||||||||||
| standalone: true, | ||||||||||||||||||||
| imports: [LatexDirective, CommonModule] | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| export class ElementLabelComponent { | ||||||||||||||||||||
| @Input() | ||||||||||||||||||||
| data; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Input() | ||||||||||||||||||||
| lines: number; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| get fontSize() { | ||||||||||||||||||||
| return `${20 / this.lines}px`; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+19
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Division by zero when The template passes 🐛 Proposed fix get fontSize() {
+ if (!this.lines || this.lines <= 0) {
+ return '20px'; // Default font size
+ }
return `${20 / this.lines}px`;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Align this model with the field name the rest of the PR uses.
VariablePaneComponentcreates these objects withname(gui-js/libs/ui-components/src/lib/variable-pane/variable-pane.component.tsLine 118), andSvgCanvasComponentlater editsd.name(gui-js/libs/ui-components/src/lib/svg-canvas/svg-canvas.component.tsLines 469-482). Keeping onlylabelhere means the declared type no longer matches the runtime shape, which is an easy way to end up with stale/broken name bindings.🤖 Prompt for AI Agents