Repository navigation
fix(studio): truly compact 4I+4V, equal phasor/wave area and atomic input commits - #279
Conversation
… partial edits on debounce
…only model commit
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…lot height invariant
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7c2fb705f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| and "SplitView.preferredWidth: root.compactLayout ? 450 : 470" in responsive_main | ||
| and "SplitView.maximumWidth: 535" in responsive_main | ||
| and "SplitView.preferredHeight: 310" in responsive_main | ||
| and "SplitView.maximumHeight: 265" not in responsive_main |
There was a problem hiding this comment.
This negative assertion conflicts with lines 383–385 of the same test, which still require SplitView.maximumHeight: 265 and SplitView.fillHeight: false. At this commit, python3 apps/arstack_studio/tests/check_operator_workspace.py therefore exits at the earlier assertion, and .github/workflows/arstack-studio-qt.yml:185 runs this script in Qt CI, so every such workflow run fails before reaching the new contract. Update or remove the stale earlier assertion together with this one.
Useful? React with 👍 / 👎.
| anchors.fill: parent | ||
| z: 0 | ||
| acceptedButtons: Qt.LeftButton | ||
| onClicked: root.forceActiveFocus() |
There was a problem hiding this comment.
When an operator edits a magnitude or phase and clicks unused injection-panel space, this calls forceActiveFocus() on root, which is an ApplicationWindow/Window, not an Item, and does not provide that method. The click consequently raises a QML runtime error instead of moving focus, so the field's editingFinished handler does not run and a live board can retain the old value despite the visible edit. Move focus to root.contentItem or to a dedicated focusable Item.
Useful? React with 👍 / 👎.
| SplitView.fillHeight: false | ||
| // Phasor and waveform each own half of available | ||
| // height; no hidden 265px cap on the phasor plot. | ||
| SplitView.minimumHeight: 245 |
There was a problem hiding this comment.
At the supported 620 px minimum window height with telemetry expanded, the outer layout leaves about 421 px for workspaceSplit after the 78 px workflow bar, margins, telemetry's 88 px minimum, and split handles. The nested visualization now requires 245 + 245 + 9 = 499 px for the two charts and its handle, so its constraints cannot be satisfied and one chart must shrink below its declared minimum or be clipped. Reduce these minima, raise the window minimum, or make this state scrollable.
Useful? React with 👍 / 👎.
…indow.forceActiveFocus
P0 GUI layout + deterministic table edit bench handoff — 2026-10-21Exact head Real operator screenshot after #276 still showed too-long 4I+4V table and significantly smaller Phasor than Waveform. Final patch is deliberately narrower than operator screenshot #2: injection target 465px, hard cap 490px (min445px), remainder of desktop width to visualization; both Phasor and Waveform have identical preferredHeight=310, minimumHeight=245 and fillHeight=true, old Phasor max265/fillHeight=false eliminated. Operator remains able to resize SplitView. SignalMatrix now validates keystrokes immediately but does NOT dispatch model/board changes on the old 90ms debounce timer. Qt Quick CI HEAD 4/4 SUCCESS: Product Naming Guard; ARStack Studio Qt (workspace contract, session harness, headless QML launch); Windows Lifecycle; Windows Native Preview (matching ESP32-P4 firmware build, packaged GUI smoke, Inno installer install/upgrade/uninstall/legacy NSIS migration). No physical acceptance claim. Windows setup.exe, direct file: extracted from https://github.com/masarray/arstack61850/actions/runs/39595249227/artifacts/12343278606, 31,036,802 bytes, SHA256 Next physical acceptance: compare at 960, 1150, 1240, 1920 widths; Phasor/Waveform balanced on screen; tap Enter/Tab/other field/empty panel, decimal comma/dot, invalid input; START with unsaved focused field; verify actual exported scenario and board readback. User demands global-professional UI visual review; this still needs real screenshots. No merge or release to main until bench evidence. |
1 participant
Operator screenshot 2026-10-21 shows injection table still sprawling and Phasor capped to ~225px/265px while waveform receives most available height; editing field must reliably apply after Enter, Tab/blur, or click empty space. This PR further caps 4I+4V surface at 535px, gives remaining desktop horizontal width to visualization, removes Phasor's 265px cap and splits vertical chart layout equally via both fillHeight and preferredHeight=310/minimumHeight=245. Adds an empty-surface focus sink. Matrix now validates on keystroke without dispatching partial values on 90ms timer; commit occurs at editingFinished with existing decimal comma/dot normalizer and existing fast model/board logic. Regression contracts assert sizing and no debounce sends. Stacked on open PR #276; no main merge or hardware PASS. Validate Qt and Windows native build and bench 960/1150/1240px and mixed comma/dot + edit/blur/Enter while SV RUNNING.