feat(ui): move Extrude/Bevel to main toolbar, slim Inspector - #305
Conversation
Replaces the Topology section of the Inspector's Edit Mode Tools with two toolbar buttons placed next to the Primitives menu and AI Chat button. The bevel session parameters (segments spinner + profile graph) stay in the Inspector since they're used mid-drag. Buttons: - ⬆ Extrude — enabled only in face mode with non-empty face selection; on successful extrude, auto-switches to the Translate tool so the user can immediately move the new geometry. - ◢ Bevel — enabled in edge mode (with edge selection) or vertex mode (with vertex selection). Both hidden entirely when not in edit mode (the toolbar reclaims the space). When enabled: light-green to match the primitive icons (#7bbd2a idle, #9adc4a hover, #5a9a1a pressed). When disabled: muted gray (#b8b8b8) so they don't shout. Hiding uses the QAction returned by QToolBar::addWidget() rather than the QToolButton directly — hiding the widget alone doesn't reclaim layout space. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughTopology triggers (Extrude/Bevel) are removed from the Inspector QML and reintroduced as glyph buttons in the main objects toolbar; toolbar buttons show/hide and enable/disable based on edit mode, selection mode, and current selection. The winget publish workflow's token validation and submit/error handling were also refined. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MainWindow Toolbar (UI)
participant EMC as EditModeController
participant SEL as SelectionModel
participant TR as TransformOperator
UI->>EMC: Click Extrude button
activate EMC
EMC->>SEL: validate selection for face mode
SEL-->>EMC: selection OK / none
alt selection OK
EMC-->>TR: request switch to Translate after extrude
EMC->>EMC: perform extrudeSelection()
EMC-->>UI: extrude succeeded
TR-->>UI: transform set to Translate
else no selection / invalid mode
EMC-->>UI: extrude disabled / no-op
end
deactivate EMC
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/mainwindow.cpp`:
- Around line 629-653: The toolbar click handlers must record breadcrumbs via
SentryReporter::addBreadcrumb(category, message) with category "ui.action":
modify the extrudeButton connection (the lambda passed to connect that calls
EditModeController::extrudeSelection) to first call
SentryReporter::addBreadcrumb("ui.action", "objects.toolbar.extrude") and then
proceed to call c->extrudeSelection() and setTransformState as before; similarly
replace the direct connect for bevelButton (currently connecting to
editCtrlForTopo->bevelSelection) with a small lambda that calls
SentryReporter::addBreadcrumb("ui.action", "objects.toolbar.bevel") and then
invokes editCtrlForTopo->bevelSelection(), preserving existing behavior. Ensure
the breadcrumb messages use the "ui.action" category and clear identifiers like
"objects.toolbar.extrude" and "objects.toolbar.bevel".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 56fc2d36-c7cc-4595-b781-06c4b32b5b94
📒 Files selected for processing (2)
qml/PropertiesPanel.qmlsrc/mainwindow.cpp
The 2.28.2 release run of winget-publish failed with \"Resource not accessible by personal access token\" after wingetcreate had built the manifest. Root cause: the WINGET_TOKEN secret is a fine-grained PAT, which can't fork microsoft/winget-pkgs — wingetcreate --submit needs a classic PAT with 'public_repo' scope. Two changes: 1. Token validation now treats a missing X-OAuth-Scopes header as a fine-grained PAT and fails fast with a clear remediation link, instead of silently proceeding to the doomed submit call. 2. The submit loop parses wingetcreate's stderr. When it sees the 'Resource not accessible' string we bail after one attempt (retry is useless) and surface a pointed error telling the user to rotate the secret. The fix itself is operational (regenerate WINGET_TOKEN as a classic PAT with 'public_repo' at /settings/tokens/new and update the repo secret). These workflow changes make that failure mode obvious at the top of the logs instead of buried under three retries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wingetcreate picks up the default microsoft/winget-pkgs PR template
(a checklist) as the PR body — there's no --prBody flag. That's fine
for reviewers (it's what every submission looks like), but adds
zero context about which release this is and where to find notes.
After a successful submit we now parse the PR URL wingetcreate prints
('Pull request can be found here: .../pull/NNNN') and post a short
follow-up comment linking to the QtMeshEditor release notes and
flagging the PR as automated. Failure to post the comment is a
warning, not an error — the PR itself is the important deliverable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The winget review flow is run by wingetbot + Microsoft maintainers; contributor comments aren't the norm and could trip keyword-based automation in their pipeline. Keep the PR URL parsing so it shows up as a ::notice:: in our own build log for easy reference, but skip the API call against microsoft/winget-pkgs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to CodeRabbit review on PR #305. CLAUDE.md requires all user-facing toolbar clicks to be tracked via SentryReporter breadcrumbs; the new Extrude/Bevel buttons were missing them. The downstream operations already breadcrumb at the edit-mode level but that doesn't capture 'the action came from the toolbar'. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|



Summary
Addresses the "Inspector is getting overloaded" feedback by moving the topology action triggers (Extrude / Bevel) out of the Inspector and onto the main objects toolbar, next to the Primitives menu and AI Chat button.
Details
Toolbar buttons (new)
States
QActionreturned byQToolBar::addWidget()— hiding theQToolButtondirectly leaves a gap.#7bbd2aidle,#9adc4ahover,#5a9a1apressed. Disabled state:#b8b8b8muted gray.Inspector (updated)
The Topology section (Extrude button + Bevel button) is removed from
PropertiesPanel.qml'seditModeToolsComponent. The bevel session panel (segments + profile graph) stays in the Inspector since those parameters are only relevant mid-drag.Test plan
Notes
EditModeControllerQ_INVOKABLE actions. The underlying selection-count properties are already covered.Summary by CodeRabbit
UI Changes
Chores