feat(HappyHare): replace right-click menu with regular click menu#2437
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactored MMU gate interaction from long-press/contextmenu handlers to unified click-based flow. Gate components expose ChangesMMU Gate Edit Flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/panels/Mmu/MmuUnitGate.vue`:
- Around line 212-215: The handleClickGate method calls
selectGate(this.gateIndex) but selectGate is defined to take no arguments (it
reads this.gateIndex itself); remove the extraneous parameter so call
selectGate() instead (or alternatively update selectGate to accept an index and
use it everywhere, but prefer the simpler fix of changing handleClickGate to
call selectGate() and ensure no other callers rely on the old signature).
🧹 Nitpick comments (2)
src/components/dialogs/MmuEditGateMapDialog.vue (1)
83-92: Minor: redundantundefinedcheck.The
initialGateprop is typed asnumber | nullwith a default ofnull, so it can never beundefined. The!== undefinedguard on line 87 is unnecessary.Suggested simplification
`@Watch`('showDialog') onShowDialogChanged(val: boolean) { if (!val) return - if (this.initialGate !== null && this.initialGate !== undefined) { + if (this.initialGate !== null) { this.selectedGate = this.initialGate } else { this.selectedGate = TOOL_GATE_UNKNOWN } }src/components/panels/Mmu/MmuUnitGate.vue (1)
8-11: No-op@select-spoolhandler used solely for CSS hover class.The
() => {}handler exists only to makehasSelectSpoolListenerreturntrueinMmuUnitGateSpool, which drives the.hasSelectSpoolhover CSS class. Since the actual click is handled by the parent div's@click="handleClickGate"via DOM bubbling, this works but is indirect. A dedicated boolean prop (e.g.,:show-hover="true") would be more explicit.That said, if this pattern was already used before the rename, keeping it consistent is reasonable.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/panels/Mmu/MmuUnitGateSpool.vue`:
- Around line 216-227: The current checks in the isEspoolerRewind and
isEspoolerAssist getters only verify that the mmuEspoolers array exists, but an
empty array is truthy and accessing a non-existent index returns undefined,
causing the condition to fail and incorrectly skip to the legacy branch. Replace
the simple existence checks with optional chaining that verifies the actual
indexed value exists: change `if (this.mmuEspoolers)` to `if
(this.mmuEspoolers?.[this.gateIndex])` in both the isEspoolerRewind getter and
the isEspoolerAssist getter, so the condition only returns true when an actual
value is present at the gateIndex position.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6db587b5-784a-4cbf-a999-bb695022b344
📒 Files selected for processing (4)
src/components/panels/Mmu/MmuUnitGate.vuesrc/components/panels/Mmu/MmuUnitGateSpool.vuesrc/components/panels/MmuPanel.vuesrc/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- src/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/panels/MmuPanel.vue
- src/components/panels/Mmu/MmuUnitGate.vue
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/panels/Mmu/MmuUnitGateSpool.vue`:
- Around line 216-227: The current checks in the isEspoolerRewind and
isEspoolerAssist getters only verify that the mmuEspoolers array exists, but an
empty array is truthy and accessing a non-existent index returns undefined,
causing the condition to fail and incorrectly skip to the legacy branch. Replace
the simple existence checks with optional chaining that verifies the actual
indexed value exists: change `if (this.mmuEspoolers)` to `if
(this.mmuEspoolers?.[this.gateIndex])` in both the isEspoolerRewind getter and
the isEspoolerAssist getter, so the condition only returns true when an actual
value is present at the gateIndex position.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6db587b5-784a-4cbf-a999-bb695022b344
📒 Files selected for processing (4)
src/components/panels/Mmu/MmuUnitGate.vuesrc/components/panels/Mmu/MmuUnitGateSpool.vuesrc/components/panels/MmuPanel.vuesrc/locales/en.json
✅ Files skipped from review due to trivial changes (1)
- src/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/panels/MmuPanel.vue
- src/components/panels/Mmu/MmuUnitGate.vue
🛑 Comments failed to post (1)
src/components/panels/Mmu/MmuUnitGateSpool.vue (1)
216-227:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect the mmuEspoolers contract/defaults and related legacy espooler state. # Expectation: If mmuEspoolers can be [] or missing a gate entry, MmuUnitGateSpool should fall back per gate instead of returning immediately. rg -n -C4 '\b(mmuEspoolers|espooler_active)\b' --glob '*.ts' --glob '*.vue'Repository: mainsail-crew/mainsail
Length of output: 2754
🏁 Script executed:
# Find the type definition of espooler in the mmu state/mixin rg -n 'espooler' src/components/mixins/mmu.ts -A2 -B2Repository: mainsail-crew/mainsail
Length of output: 437
🏁 Script executed:
# Search for espooler type definitions and interfaces rg -n 'type.*[Ee]spoiler|interface.*[Ee]spoiler' --glob '*.ts'Repository: mainsail-crew/mainsail
Length of output: 48
🏁 Script executed:
# Find MmuEspoolerState definition rg -n 'MmuEspoolerState' --glob '*.ts' -B2 -A2Repository: mainsail-crew/mainsail
Length of output: 928
Check
mmuEspoolersfor both existence and value before accessing indexed properties.The property
espooleris typed asMmuEspoolerState[](optional array). Lines 217 and 224 useif (this.mmuEspoolers)before indexing, but an empty array is truthy; accessing[this.gateIndex]on an empty or incomplete array returnsundefined, causing the condition to returnfalseinstead of falling back to the legacyespooler_activebranch.Fix by checking for a defined value at the index:
if (this.mmuEspoolers?.[this.gateIndex]) return this.mmuEspoolers[this.gateIndex] === 'rewind'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/panels/Mmu/MmuUnitGateSpool.vue` around lines 216 - 227, The current checks in the isEspoolerRewind and isEspoolerAssist getters only verify that the mmuEspoolers array exists, but an empty array is truthy and accessing a non-existent index returns undefined, causing the condition to fail and incorrectly skip to the legacy branch. Replace the simple existence checks with optional chaining that verifies the actual indexed value exists: change `if (this.mmuEspoolers)` to `if (this.mmuEspoolers?.[this.gateIndex])` in both the isEspoolerRewind getter and the isEspoolerAssist getter, so the condition only returns true when an actual value is present at the gateIndex position.
…UnitGateMenuItem components
meteyou
changed the title
2 participants
Description
This PR removes the right-click context menu and replaces it with a regular click. This simplifies operation especially on mobile devices and was advocated by the community. The sensitivity of menu items is set based on current MMU state/capabilities and now necessarily extends to the bypass gate. This PR groups all changes the the context menu including the ability to invoke none gcode command operations like opening the filament editor dialog with the selected gate as default. The cursor type is pointer for gate selection and menu in the main panel
Note: There will be a minor merge conflict with PR #2429 in MmuUnitGate.vue since both modify the same template line. The correct resolution is (note
mb-n5,@clickhandling and cursorType class):Related Tickets & Documents
This PR replaces closed PR #2430
Mobile & Desktop Screenshots/Recordings
[optional] Are there any post-deployment tasks we need to perform?
n/a