Commit Graph

4 Commits

Author SHA1 Message Date
Christian Byrne
66e776774a feat: add default keybindings for toggle mode and assets panel (#8593)
## Summary
Add default keyboard shortcuts for mode toggle and assets panel access.

## Changes
- **Ctrl+Shift+A**: Toggle between Simple Mode and Graph Mode
(`Comfy.ToggleLinear`)
- **A**: Open/toggle assets panel (`Workspace.ToggleSidebarTab.assets`)
- Show keybinding in ModeToggle button tooltip (e.g. "Simple Mode
(Ctrl+Shift+A)")

## Keybinding Rationale
- `A` follows the existing pattern: `w` (workflows), `n` (node-lib), `m`
(model-lib)
- `Ctrl+Shift+A` chosen because:
  - Ctrl+Shift is the standard modifier pattern for toggle commands
  - "A" mnemonic for "App mode"
  - Does not conflict with existing keybindings

## Testing
- Verified typecheck passes
- Verified lint passes

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Tooltips now show dynamic keyboard shortcut suffixes for mode and tab
controls.
  * Added keyboard shortcut: A — toggles the assets sidebar.
  * Added keyboard shortcut: Ctrl+Shift+A — toggles linear mode.

* **Localization**
* Added a localized shortcut suffix template so displayed shortcuts
respect translations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8593-feat-add-default-keybindings-for-toggle-mode-and-assets-panel-2fc6d73d36508172bd6ed3378f43de55)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
2026-02-03 16:06:33 -08:00
Christian Byrne
32da91ea32 perf: code-split xterm bundle and gate terminal features for cloud (#8528)
## Summary

Code-splits the ~400KB xterm bundle and excludes terminal features from
cloud distribution where they are not needed.

### Changes

- **bottomPanelStore.ts**: Gate terminal tab registration behind
`__DISTRIBUTION__ !== 'cloud'` check with dynamic import, enabling
tree-shaking
- **keybindingService.ts**: Skip logs-terminal keybinding registration
for cloud distribution
- **vite.config.mts**: Add `vendor-xterm` code splitting group

### Bundle Impact

| Distribution | xterm in bundle | Terminal tabs |
|--------------|-----------------|---------------|
| cloud |  No (~400KB saved) | None |
| localhost |  Yes (vendor-xterm chunk) | Logs terminal |
| desktop |  Yes (vendor-xterm chunk) | Logs + Command terminal |

### Verification Script

Run locally to verify xterm exclusion works correctly:

```bash
#!/bin/bash
set -e

echo "=== Verifying xterm bundle exclusion ==="
echo

# Clear Nx cache to ensure fresh builds
pnpm nx reset 2>/dev/null

# Build cloud distribution
echo "Building CLOUD distribution..."
rm -rf dist
DISTRIBUTION=cloud pnpm build --mode production 2>/dev/null

echo "Cloud build - checking for xterm:"
if grep -r "xterm" dist/assets/*.js >/dev/null 2>&1; then
  echo "   FAIL: xterm found in cloud bundle"
  grep -l "xterm" dist/assets/*.js | head -5
else
  echo "   PASS: xterm NOT in cloud bundle"
fi
echo

# Build localhost distribution
echo "Building LOCALHOST distribution..."
pnpm nx reset 2>/dev/null
rm -rf dist
DISTRIBUTION=localhost pnpm build --mode production 2>/dev/null

echo "Localhost build - checking for xterm:"
if grep -r "xterm" dist/assets/*.js >/dev/null 2>&1; then
  echo "   PASS: xterm found in localhost bundle"
  echo "  Files containing xterm:"
  grep -l "xterm" dist/assets/*.js | while read f; do
    size=$(wc -c < "$f")
    echo "    $(basename $f) ($(numfmt --to=iec $size))"
  done
else
  echo "   FAIL: xterm NOT in localhost bundle"
fi
echo

echo "=== Verification complete ==="
```

**Note**: Nx cache must be reset between builds with different
`DISTRIBUTION` values or it may return stale results.

## Test Plan

- [x] Quality gates pass (typecheck, lint, format, tests)
- [x] Cloud build verified: xterm NOT present
- [x] Localhost build verified: xterm present as vendor-xterm chunk

Fixes COM-14129

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8528-perf-code-split-xterm-bundle-and-gate-terminal-features-for-cloud-2fa6d73d365081a093ecc74ca7ce6e7c)
by [Unito](https://www.unito.io)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Refactor**
* Terminal tab declarations adjusted (no user-visible behavior change).
* Bottom panel tabs now load asynchronously; panel toggle falls back to
shortcuts when terminal tabs aren’t loaded.

* **Chores**
* Terminal tabs and related keybindings suppressed in cloud deployments.
  * Core commands wired to a new utility to support widget promotion.

* **Tests**
* E2E tests updated to handle async terminal-tab loading and minor test
cleanup.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: GitHub Action <action@github.com>
Co-authored-by: Alexander Brown <drjkl@comfy.org>
2026-02-03 15:15:37 -08:00
Christian Byrne
e1385b0d18 refactor: use singleton mock pattern for useDialogStore in escape test (#8538)
## Summary

Refactors `keybindingService.escape.test.ts` to use the singleton mock
pattern with reactive state, following project testing guidance.

## Changes

- **What**: Replaced `vi.fn(() => ({dialogStack: []}))` anti-pattern
with `reactive()` array defined inline in `vi.mock()` factory. Tests now
use array mutation (`.push()`, `.length = 0`) instead of
`mockReturnValue` calls.

## Review Focus

Pattern aligns with docs/testing/unit-testing.md section "Mocking
Composables with Reactive State".

Fixes #8467

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8538-refactor-use-singleton-mock-pattern-for-useDialogStore-in-escape-test-2fb6d73d3650812a95c6e223e38d50ad)
by [Unito](https://www.unito.io)
2026-02-01 22:09:10 -08:00
Christian Byrne
47113b117e refactor: migrate keybindings to DDD structure (#8369)
## Summary

Migrate keybindings domain to `src/platform/keybindings/` following DDD
principles.

## Changes

- **What**: Consolidate keybinding-related code (types, store, service,
defaults, reserved keys) into a single domain module with flat structure
- Extracted `KeyComboImpl` and `KeybindingImpl` classes into separate
files
- Updated all consumers to import from new location
- Colocated tests with source files
- Updated stores/README.md and services/README.md to remove migrated
entries

## Review Focus

- Verify all import paths were updated correctly
- Check that the flat structure is appropriate (vs nested core/data/ui
layers)

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-8369-refactor-migrate-keybindings-to-DDD-structure-2f66d73d36508120b169dc737075fb45)
by [Unito](https://www.unito.io)

---------

Co-authored-by: Subagent 5 <subagent@example.com>
Co-authored-by: Amp <amp@ampcode.com>
2026-01-29 18:40:58 -08:00