Files
miniSCP/docs/gotchas.md
T
jgamboaandClaude 9c54a441a9 Scale stage 3: shortcuts service + Ctrl/Cmd +/-/0 for the scale
Groundwork for the vim-style bindings planned next: the app previously had NO
keyboard abstraction — `event.metaKey || event.ctrlKey` copy-pasted inline in
six places, no typing-target guard anywhere, every window root matching keys
by hand.

New @core/shortcuts.service.ts owns:
- isMod() (also accepts MouseEvent — Mod+click toggle-select uses the same
  rule) and isTypingTarget();
- ONE declarative keymap: resolve(event, scope) -> ShortcutId | null, scoped
  main / editor / manager. Layout rules are encoded once: digits, +/- and
  Alt-letter chords match on event.code (AZERTY digit rows, layouts where '+'
  needs Shift, macOS Option-Z producing Ω); plain letter chords match on
  event.key so they follow the user's layout.

Both app.ts, both editor-window.ts and both connection-manager.ts now resolve
through the service and switch on ids — behaviour-preserving for every
existing shortcut, including the deliberate quirks (Ctrl+Tab never meta
because Cmd-Tab is the macOS app switcher; Cmd+9 = last tab; editor Mod+W is
the Windows/Linux arm, macOS routes via the app menu). Escape and Tab stay
component-local on purpose: their meaning is per-window state (find bar open?
close prompt up? caret in the textarea?), not a global binding. The file
panels keep their contextual key state machine but delegate modifier tests.

On top of the table: Ctrl/Cmd +/-/0 steps/resets the interface scale in every
window kind, all three previously unclaimed. isTypingTarget is applied
per-shortcut, not globally in resolve(): the scale keys must not fire while
typing ('-' in a rename prompt must type '-'), but the editor WANTS Mod+S
mid-typing and the tab shortcuts keep their existing reach.

Docs: architecture.md's density prose now describes the uiScale lever;
gotchas.md gains the scale section — the disassembly evidence that native
page zoom is an empty stub on Linux (so nobody tries it again), the
everything-must-be-rem rule, the skinfix-outranks-variants trap the harness
caught (48px close button silently 29px), and the px-mirrors-in-TS class of
bug with the remPx() remedy.

Verified: tsc + build clean; live boot with the user's real legacy settings
file migrates textSize:large -> uiScale 1.2 and renders scaled. Not verified
here (needs interaction): the stepper clicks and the shortcut keys themselves.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-16 18:15:14 +02:00

21 KiB
Raw Blame History

Gotchas — landmines this codebase has already stepped on

Every one of these caused a real bug or debugging spiral. Read before touching the adjacent code.

Angular (v22, zoneless, signals, OnPush)

  • Never consume push streams through signal+effect. An effect that reads a signal and (directly or transitively) writes signals re-triggers itself — this froze the entire app on the first transfer. Push messages (transferUpdate etc.) go through plain callback registries in RpcService. Signal coalescing would also silently drop intermediate progress snapshots.
  • No signal writes during template render (NG0600). Anything that loads panels from code paths reachable during render defers with queueMicrotask (see PanelsService.newWorkspace).
  • [value] on <select> is applied before @for options render and silently selects nothing. Use per-option [selected] bindings (connection manager stack picker).
  • Template bindings with special characters: [class.bg-primary/15] works; [class.shadow-[...]] (brackets) does NOT parse — use [style.box-shadow]. Static class attributes may use arbitrary values freely.
  • Functions called from templates ARE reactive: signals read inside them register on the view's consumer (call-stack-based tracking). That's why iconClass(entry) reacting to settings.iconColors() works.
  • Dedicated tools > shell: IDE diagnostics between paired template/class edits are often stale mid-sequence — trust bun run build:ui (full AOT type-check), not the first red squiggle.
  • Tailwind v4 canonical class names: wrap-break-word not break-words, accent-primary not accent-[var(--primary)], z-60 not z-[60]. Every color used in a template must resolve through @theme inline.
  • The win7 skin layer outranks Tailwind utilities. 7.css is imported layer(skin), ordered above utilities, so any utility you add to a button/input/select/textarea/table is silently dead under that skin. If it matters, neutralize in the @layer skin block in app/skins/7like/7like.css — and note revert-layer only reaches utilities from inside skin; used from skinfix it falls back onto 7.css and is a no-op. This bit once already.
  • @import … layer(name) works through @tailwindcss/postcss (it wraps the imported AST), but the deep path is mandatory: 7.css ships no exports map and Tailwind's resolver reads only style/mainFields, so a bare @import "7.css" resolves to nothing. Use 7.css/dist/7.scoped.css.
  • Stylesheet url() must resolve at BUILD time: assets referenced from styles.css live under ui/src/ (esbuild emits them hashed into media/ and rewrites the url), NOT ui/public/ — a public path fails the build with "Could not resolve".
  • Two class attributes on one element: the second is silently dropped. No warning, no build error — the layout classes simply never apply. This collapsed the tab strip and two window headers at once (fe028a9), because a scripted edit appended class="…" to elements that already had one. When removing a conditional binding, check what the branch contained: the bindings there were genuinely dead, but the layout classes inside them were not, and nothing replaced them.
  • A component template: is a TS template literal, so no backticks inside it — not even in an HTML comment. A backtick in a comment terminates the literal and the errors point at the prose, not at the quoting.

The interface scale (root font-size lever)

  • electrobun's native page zoom is a silent no-op on Linux — do not reach for it. webviewSetPageZoom in the shipped libNativeWrapper.so (v1.18.1, and the _cef.so, and the copy in our own build output) is a compiled empty-body stub: the disassembly has no call instruction, and webkit_web_view_set_zoom_level is not even in the library's imports. webviewGetPageZoom returns a hardcoded 1.0 from .rodata, so the failure is unobservable through the API — set 2.0, read back "1.0 = success". The JSDoc's "(WebKit only)" means macOS WKWebView. This is why the scale is CSS.
  • Everything the UI sizes with must be rem. The scale works by setting the root font-size, so a px value freezes while everything around it grows. The three px reservoirs were: the panel-chrome tokens (--row-h etc.), 93 text-[11px] arbitraries, and 7.css itself — 370 px values, zero rem, with --w7-font reaching only 15 rules' typography. 7like.css's skinfix carries a scale bridge re-expressing 7.css's vars and hardcoded box metrics in rem. 1px hairlines and focus rects stay px deliberately (crisp at any scale; the tr border exists to stop selection shifting rows).
  • Restating a 7.css rule in skinfix also outranks 7.css's VARIANTS of that rule — by layer, regardless of specificity. The title-bar-controls restatement silently shrank the close button from 48px to 29px (7.css widens it via button[aria-label=Close]), and the tablist padding flattened the active tab's 4px raise. Caught only by the harness pixel diff. When bridging a rule, grep 7.scoped.css for every variant that overrides the same properties and restate those too.
  • px constants in TS that mirror rem-sized CSS are latent scale bugs. The shadlike context menu's MENU_W = 224 was a hardcoded mirror of w-56 (14rem): box grew, math didn't, flip fired at the wrong edge. Convert through @core/metrics remPx() at use time — it reads the LIVE root font-size, and per call, since the scale changes mid-session. The editor's canvas-based charWidth is cached keyed on the computed font string for the same reason.

The two skin trees (ui/src/app/skins/)

  • .win7 is not a name we may change. 7.scoped.css is precompiled with every selector prefixed by that literal class, so the skin id in AppSettings stays "win7" even though its folder is 7like/. The folder names are cosmetic; the class and the persisted setting value are not.
  • Reach core/ through the @core/* alias, never a relative path. Skin components sit at varying depths and get moved; the alias is what let 67 imports survive two relocations untouched. A ../../core/... path is a future merge conflict.
  • @font-face url() resolves relative to the file that declares it. When the win7 rules moved three directories deeper, ./fonts/... would silently have 404'd — build still green, app just renders a fallback font. The fonts therefore live inside the skin that uses them.
  • The @layer statement must stay the first statement in ui/src/styles.css. It is one of only two rule types allowed above @import, and it is what sorts skin above utilities. Every @import (Tailwind and both skin sheets) must also precede all other rules.
  • Duplicate selectors across skins are safe. app-file-panel exists in both trees; standalone components resolve through their importer's imports: array, never a global registry, and the trees are never mounted together.
  • The build DOES type-check components nothing imports. ngtsc compiles the whole program, so a skin that is currently unreachable still fails bun run build:ui on a type or template error. Verified deliberately by injecting an error into an unreferenced file — worth knowing before assuming a green build proves nothing about the inactive skin.

The win7 skin (7.css)

  • Unlayered declarations beat every @layer, regardless of specificity. html, body { background: … } in styles.css is unlayered, so no skinfix rule can override it — the win7 transparent-corner reset has to be unlayered too. The frame's rounded corners only read as rounded because nothing paints an opaque full-bleed ground behind them; anything that does fills the corners and the window becomes a rectangle with a curve drawn on it.
  • 7.css restates border-color at (0,3,1) via .win7 button:not(:disabled):not(:hover) and its :hover twin. A two-class selector like .win7 .w7-flat (0,2,0) loses to it, so paint suppression for flat buttons lives in skinfix where layer order settles it. The .w7-toolbar arm only ever won by tying on specificity and landing later in the same layer — an accident w7-flat did not share, which is why it rendered a bordered slab while its neighbours went flat.
  • overflow-x: auto also clips vertically. CSS refuses to pair visible with a non-visible axis, so the computed overflow-y becomes auto too. 7.css raises the active tab with margin-top: -2px; inside the horizontally scrolling tab strip that put its top border outside the scroll box and deleted it silently. The strip reserves padding-top: 2px to compensate — mirroring the padding-left: 3px 7.css already reserves for the same tab's -3px left margin.
  • 7.css hardcodes light-mode literals outside its own var system — color: #222 on tablist buttons, black/white bevels in the ListView and menu. Each one has to be routed through a --w7-* token declared in :root.win7 and overridden in both dark blocks (the [data-theme] attribute one and the prefers-color-scheme one; system dark sets no attribute).
  • Its selectors are exact, and a near-miss looks exactly like "7.css doesn't support this". menu[role=tablist] button is a DESCENDANT selector, so every button inside the strip becomes a tab — the + and the right-hand tools must live outside it. .window.glass > .title-bar is direct-child, so an inner wrapper breaks the glass coupling. A wrong guess here already produced one hand-written lookalike that was deleted.

Electrobun

  • Window close is not cancellable (response type {}; only the app-level before-quit supports event.response = {allow:false}). All unsaved-state guards must either intercept app-controlled close paths (⌘W, buttons) or survive the close (editor draft mirroring).
  • One rpc instance per window — BrowserView binding steals the rpc's transport; a shared instance silently sends to the wrong webview.
  • RPC before socket open is silently lost. Everything gates on the socket-open promise (RpcService ready).
  • The emitter has no off(). Window event closures must not capture records you want garbage collected.
  • electrobun build defaults to the dev channel; release builds need --env=stable. The stable app name differs from the dev one (miniSCP vs miniSCP-dev), including the app-data dir implications when testing.
  • Tray: never call setImage after construction on macOS — the template treatment is lost and the icon renders black on dark menu bars.
  • ApplicationMenu is macOS-only in our usage; accelerators on Windows are single-character only.
  • ContextMenu.showContextMenu is a silent no-op on Linux (electrobun's own dist/api/bun/proc/linux.md says so; the app also prints "Application menus are not supported on Linux" at boot). Because the UI suppresses the webview's own right-click menu, that left EVERY panel context menu dead on Linux — no error, nothing happens. The menu is plain HTML now (ui/src/app/skins/*/context-menu.ts) on every platform; don't reintroduce the native one for "nativeness" on mac/win without keeping a Linux path.

Linux

  • ERR_DLOPEN_FAILED on bun run start means a missing system library, not broken app code. libNativeWrapper.so links libayatana-appindicator3.so.1 hard (for the tray), so the app cannot start at all without it — even though the tray is a minor feature. Diagnose with ldd build/dev-linux-x64/*/bin/libNativeWrapper.so | grep 'not found'; the libasar.so => not found line in that output is a red herring (it ships beside the wrapper and resolves at load). Install list per distro is in platform-notes.md.
  • Black window on NVIDIA + Failed to create GBM buffer of size WxH: Invalid argument (usually with X11 Error: GLXBadWindow (code 170)). The app boots, logs "miniSCP started", reports no error — and paints nothing. WebKitGTK's DMABUF renderer can't allocate buffers on the NVIDIA proprietary driver; WEBKIT_DISABLE_DMABUF_RENDERER=1 is the upstream workaround.
    • The app cannot apply this to itself. WebKit reads the variable when it forks its web process, and Bun's process.env writes do not propagate to child processes (verified: a spawned child sees it unset). Setting it in src/bun/index.ts — even in a module imported before electrobun/bun — does nothing. It must be in the environment before the app process starts.
    • Hence every launch path injects it: scripts/linux-gpu.ts (launchEnv()) for bun run dev / bun run start, and the miniSCP.sh wrapper that release.ts writes beside bin/launcher in the tarball. A new launch path must call launchEnv() or it will render black on NVIDIA.
    • Gated on /proc/driver/nvidia so Intel/AMD keep hardware compositing, and an explicit WEBKIT_DISABLE_DMABUF_RENDERER from the user always wins.
    • A "does it boot?" log check does NOT catch this — grep for the GBM line, or look at the window.
  • bun run start leaves the old instance running, and import -window miniSCP grabs the OLDEST match. Instances pile up across rebuilds, so every screenshot silently comes from a stale build and the fix you just made looks like it did nothing. Cost three wrong readings in one session, including a "the CSS is in the bundle but does not apply" dead end. Kill first (pkill -f dev-linux-x64), confirm with pgrep -cf 'dev-linux-x64/miniSCP-dev/bin/launcher', then launch and capture. Note pkill -f 'electrobun dev' also matches the shell running it and kills your own command (exit 144) — match on the build path instead.
  • X11 capture cannot answer "is this pixel transparent?" for the frameless window. import -window grabs the window's own pixels with no compositing, so a transparent corner and an opaque black one both come out black; png32: just fills alpha with 1 and proves nothing. XWayland's root window is not the composited desktop, so import -window root fails too. Frame transparency and the maximized corners need a human looking at the screen — say so rather than reporting a screenshot as confirmation.
  • Benign startup noise on Linux — non-fatal, the app runs normally: libayatana-appindicator is deprecated. Please use libayatana-appindicator-glib (upstream electrobun's choice, not ours).
  • Window-open flash prevention is three-layer — remove any one and the white/inverted blink returns: (1) windows are created hidden: true and revealed by showWhenReady on the webview's first dom-ready (800 ms fallback), (2) the bun preload bakes the persisted data-theme/tone/accent attributes before first paint (buildPreload), (3) ui/src/index.html carries an inline <style> with the html background per theme — its oklch values MUST stay in sync with the --background tokens in styles.css. The win7 skin adds a fourth value to keep in sync: buildPreload also bakes the .win7 class, and index.html carries a matching html.win7 arm for all four theme states. Miss it and every win7 window flashes white.
  • The skin is a LAUNCH-TIME choice. Never apply one to a running window. This shipped broken once: SettingsService.applySkin toggled the .win7 class the moment the setting changed, which was harmless when there was one component tree and became a broken window as soon as there were two — 7.css painted onto shadlike markup that has no .window.glass, no .title-bar, no menu[role=tablist] and no <table> ListView. Each skin's CSS is written against its OWN tree's markup; the class must only ever be written by buildPreload, for the tree that window actually booted.
    • A reload does not rescue it. The preload is a persistent WebKitUserScript injected at DOCUMENT_START, so its string is frozen at webview construction — webview.loadURL() re-runs it with the OLD __APP_SKIN. (updatePreloadScriptToWebView exists in the FFI but is reachable only from electrobun's internal <electrobun-webview> handler.)
    • titleBarStyle/transparent are construction-time only — no setters exist, so a live window can never be re-framed. win7 → modern is the dangerous direction: the modern tree draws no title bar, so it would land in a frameless transparent window with no way to move, minimize or close.
    • Don't "fix" this by recreating the main window either: exitOnLastWindowClosed is true, the main rpc is captured by six push closures, the tray captures win, listeners cannot be removed (no off()) — and it would drop every session and tab.
    • Bun pins it: initSessionSkin freezes the skin before any window exists, and buildPreload/chromeOptions read that, never the live settings. Skip the pin and a window opened after a pending change boots the other skin.
  • Dev vs prod URL: getMainViewUrl() resolves the ng-serve dev server only when DEV_SERVER_URL is set AND the updater channel is dev AND the server answers a HEAD request; secondary windows must reuse the resolved URL, never hardcode views://.
  • Upstream Bun panic (FFI_Callback_threadsafe_call → web_worker.spin): rare segfault under long sessions; upstream electrobun×Bun issue, restart recovers, nothing app-side to fix.

Domain logic

  • sessionId ≠ filesystem identity. Two panels on the same server are two SSH sessions. Every same-file/nested-path guard compares fsKey (resolvedIP:port / "local"); comparing sessionId caused a truncate-to-zero data-loss bug. When fsKeys differ but paths collide, the probe-file check unmasks aliased hosts. Never "simplify" this back.
  • createWriteStream truncates BEFORE the source is read — that's why self-copy is destructive, and why guards must run before any stream opens.
  • The synthetic '..' row: type 'dir', must be excluded from selection, targets(), filtering match-count, kind/rights/owner display, and drag sources. Any new row-level feature must decide what '..' does.
  • Semaphore.wake() must reserve slots synchronously; waking waiters that increment asynchronously loses budget under concurrent release.
  • Settings: the backend spread-merges ({...DEFAULT_SETTINGS, ...data}) so new keys auto-default, but the UI SettingsService reads field-by-field — a new setting silently stays at default until wired in all five spots.
  • The editor draft mirror sends full content per tick (2 MB cap upstream); don't shrink the 400 ms debounce.
  • panel.touched gates localDir auto-navigation; connectToken guards against a slow connect resolving into a panel the user already reused.

i18n

  • Translations live in src/shared/i18n/*.ts (TS, not JSON) — deliberate: it sidesteps resolveJsonModule differences between the Angular esbuild and Bun builds and gives full typing. en.ts is the source of truth; other locales are partial and fall back to English then the raw key.
  • i18n.t() is reactive because it reads the locale signal — that's what makes live language switching work with no pipe. If you cache a translated string in a plain field it WON'T update on switch; call t() in the template (or in a signal-reading getter) instead. Same for i18n.date()/size().
  • Dictionary values are TS string literals: French/other apostrophes break single-quoted entries ('…c'est…'). Use double quotes for any value containing an apostrophe. Plurals are { one, other } maps (not ICU strings); zh/ja supply only other.
  • Native menus can't use the Angular service — menu.ts goes through backend i18n.ts, and the app menu must be rebuilt when the locale changes (wired in index.ts settingsChanged), or it keeps the old language. The context menu no longer has this problem: it is Angular now and reads I18nService directly.
  • eu/zh/ja are AI drafts pending native review (see the i18n README).
  • Silent auto-refresh must snapshot sessionId+cwd and discard a mismatched result. A normal refresh() is masked by busy (input blocked), but the silent variant (poll/watch) runs while the user can navigate — without the guard, a poll that started in dir A resolves and stamps A's listing over the dir B the user just opened. It also never calls scrollCursorIntoView (scroll must stay put) and returns early if busy() or already refreshing.

Build / tooling

  • Angular CLI rejects Bun's Node-compat version — scripts/node-version-shim.ts is preloaded via bunfig.toml at the root AND in ui/. Removing either breaks ng.
  • ssh2's optional native deps must stay external: ["cpu-features", "*.node"] in electrobun.config.ts — bundling them breaks the build; ssh2 falls back to pure JS at runtime.
  • bun run --cwd ui build (flag order matters; bun --cwd ui run build does not do what you want).
  • version.gen.ts is gitignored and regenerated by postinstall + every build script; a fresh checkout fails tsc until bun install (or any script) has run once. Never commit it — it would bump the commit count it records.
  • Icon pipeline is scripts/gen-icons.ts (resvg) — regenerate after touching assets/logo.svg or assets/tray/tray-glyph.svg; the .ico and tray PNGs are build inputs, not build outputs.