Sccarda/c erearchitecting#3416
Draft
ScottCarda-MS wants to merge 88 commits into
Draft
Conversation
…ding/writing tests easier
| const svg = container.querySelector("svg.qviz") as SVGElement; | ||
|
|
||
| const overlay = document.createElementNS( | ||
| "http://www.w3.org/2000/svg", |
| isControl: boolean, | ||
| ) => { | ||
| // Generate svg element to wrap around ghost element | ||
| const svgElem = document.createElementNS("http://www.w3.org/2000/svg", "svg"); |
| const ghostY = svgHeight; | ||
|
|
||
| const ghostLayer = document.createElementNS( | ||
| "http://www.w3.org/2000/svg", |
| */ | ||
| const _dropzoneLayer = (context: Context) => { | ||
| const dropzoneLayer = document.createElementNS( | ||
| "http://www.w3.org/2000/svg", |
| }); | ||
|
|
||
| // Generate svg container to store gate elements | ||
| const svgElem = document.createElementNS("http://www.w3.org/2000/svg", "svg"); |
| const buttonX = 1; | ||
|
|
||
| const runButtonGroup = document.createElementNS( | ||
| "http://www.w3.org/2000/svg", |
| runButtonGroup.setAttribute("role", "button"); | ||
|
|
||
| // Rectangle background | ||
| const rect = document.createElementNS("http://www.w3.org/2000/svg", "rect"); |
| rect.setAttribute("class", "svg-run-button-rect"); | ||
|
|
||
| // Text label | ||
| const text = document.createElementNS("http://www.w3.org/2000/svg", "text"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Circuit Editor: internal re-architecture (Data → Actions → Renderer → Editor)
At a glance
source/npm/qsharp/ux/circuit-vis/**+source/npm/qsharp/test/**only.mjstests. No Rust, no VS Code host changes.index.tsexports the same surfacetest/circuits-cases/What this PR does
Three things, in dependency order:
Splits the monoliths into a layered architecture. The old editor was
four large files with tangled responsibilities. They are removed and
replaced with a strict, one-directional layering:
The hard rule:
data/andactions/never touch the DOM. The rendererreads data to produce SVG; the editor glues DOM events to action calls.
Adds group-aware, multi-target drag-and-drop editing. Editing inside
expanded groups, moving multi-target gates as rigid units, shift-to-extend
a group onto new wires, and the dropzone geometry that makes those
reachable. This is the capability the re-architecture existed to enable —
it is woven into the core move algorithm, not layered beside it.
Hardens classical-register / measurement integrity. Moving or deleting
a measurement that later gates depend on now cascades correctly (with a
confirm prompt) instead of crashing or leaving dangling classical
references.
Files removed (old monoliths)
circuitManipulation.ts(−668)events.ts(−1,095)panel.ts(−437)draggable.ts(−531, old version)contextMenu.ts(−335, old version)Their responsibilities now live in the layered modules below.
Why this is one PR
The core is a single atomic refactor. The new
data / actions / renderer / editorfiles are mutually dependent by design — for example, the core movealgorithm (
actions/circuit-actions/move.ts) imports the classical-registeranalysis, the entrypoint (
sqore.ts) imports the view-state type, and theeditor shell wires in the renderer. There is no by-file grouping of the final
tree that yields smaller PRs which each compile and pass tests on their
own: a hypothetical "data-layer-only" PR has no consumers, and a
"core-without-classical" PR wouldn't build because core files import it.
Splitting further would require hand-authoring intermediate versions of the
shared files that never actually existed and were never tested — trading a
large-but-truthful PR for several smaller-but-fictional ones. We chose to keep
the core honest and reviewable instead of artificially fragmented. The size is
inherent to the change, not a packaging choice.
How to review this efficiently
There is a companion reading guide checked into the branch:
REVIEW-ORDER.md(bottom-up, dependency-ordered) plusARCHITECTURE.md(the structural map + two end-to-end flow walkthroughs). Read
ARCHITECTURE.mdfirst, then followREVIEW-ORDER.md.Suggested order and the natural stopping points:
data/) — small, pure value types (Location,CircuitModel,ViewState). The vocabulary everything else is written in.actions/+actions/circuit-actions/) — pure mutations;the correctness core. Cross-reference the
test/circuit-editor/circuit-actions/suite as you go — each topic filemaps to a cluster of functions here.
→ Good checkpoint: you've now seen the entire pure core.
renderer/) —Circuit→ SVG; reads data, never mutates.editor/+editor/controllers/) — DOM glue. Read the sharedInteractionContextfirst, then the controllers.→ Good checkpoint: trace one end-to-end flow from
ARCHITECTURE.md.sqore.ts,utils.ts,index.ts) — ties it together.state-viz/) — parallel subsystem, only import-path updateshere; safe to skim last.
What to focus your scrutiny on
data/andactions/stay DOM-free?.targetsauthority: every mutator inactions/should keep the eager.targetscache correct on the way out (the decision record for why thecache is eager lives in
test/circuit-editor/circuitTargets.bench.md).model/interactionandre-render via one
renderFn, with no controller-to-controller coupling.resetTransient).sqore.ts— the host's circuit objectis never mutated by rendering.
What you can safely skim
state-viz/diffs — mechanical import-path updates from the move(
../events.js→../editor/events.js,../circuit.js→../data/circuit.js). No behavior change.test/circuits-cases/— generated.Testing
418 circuit-editor tests across 29
.mjsfiles, all passing(
data,actions/circuit-actions,renderer, controllers,sqore),plus snapshot fixtures under
test/circuits-cases/.The action layer is the most heavily covered: the
test/circuit-editor/circuit-actions/suite pins every move / control /extend-cascade / classical-ref-remap / clone-move path.
Run locally from
source/npm/qsharp/: