Fix placement, gear/Help, and dock⇄desktop visibility bugs#300
Conversation
|
@claude can you review this pull request? |
|
Claude finished @AllTerrainDeveloper's task in 4m 21s —— View job PR Review: Fix placement, gear/Help, and dock⇄desktop visibility bugsTodo List
This is a well-structured bug-fix PR. Each fix is correctly targeted at the root cause, the test coverage is solid, and the documentation is kept in sync. Below are my findings, grouped by severity. ✅ What's correct and well-done
Empty-array PHP regex for
🟡 Minor observations1. dockPromotedPositions: { ...DEFAULTS.dockPromotedPositions },A spread creates a shallow copy. If 2. The inline type written here — cb: ( snapshot: { itemVisibility: Record<string, ItemVisibility> } ) => void— is structurally compatible with the real 3. The same inline shape for 4. No unit tests for The new spatial-layout tests in 5. wrap.querySelector('input, select, textarea, button, fieldset')A Screen Options wrap that contains only a "Close" / "Apply" button and nothing else would still trigger the gear. In practice WordPress always renders inputs alongside submit buttons, so this is not a real regression risk — but the intent is "has renderable content," and 🔴 Nothing blockingNo correctness bugs, no public-API breaks, no test failures. The one PHP change that deserves a second look — the SummaryAll five bugs described in the PR body have correct, minimal fixes. Tests cover the two most load-bearing new behaviours (spatial-layout pinned icon, dockOrder colon preservation). The observations above are minor: one latent aliasing risk in |
…em visibility logic
Summary
Fixes a cluster of app-placement, screen-meta, and item-visibility bugs surfaced by a behavioral audit. Two were user-reported (My WordPress missing from the wallpaper; gear/Help buttons showing with nothing behind them); the rest are adjacent correctness issues found while tracing those paths. Every finding was reproduced in code and adversarially verified before fixing.
No public-API breakage. The only documented surface touched is the
desktop-mode-screen-metabridge message, whose change is backwards-compatible (an emptypanels: []was already a valid payload) —docs/javascript-reference.mdis updated in the same PR.Why
Changes
🔴 High
Spatial layout no longer drops the pinned My WordPress icon —
src/desktop-layout.tsrepaintIcons()'s spatial branch now keeps server icons that are eitherpinned(framework-owned, e.g. My WordPress) or carry an explicitdesktop/bothplacement override. Non-pinned plugin icons with no override stay suppressed (the deliberate "core surface" design). This also fixes the OS Settings "On the desktop" picker silently no-opping in Spatial.Right-click "Hide" stops teleporting items —
src/item-visibility-menu.ts"Hide from dock/desktop" now derives the target from the item's current placement via a new
computeHideTarget()(both→ the other rail; single-rail →hidden) instead of unconditionally writing the opposite rail. Native rail is derived from the rail-synthesis id prefix.🟠 Medium
Stale gear/Help cleared on navigation —
src/iframe-bridge-standalone.ts,includes/render/chromeless-bridge.phpBoth bridges now always announce
desktop-mode-screen-meta(including an emptypanels: []) so the parent removes stale buttons when a page exposes no screen meta after an in-place same-slug navigation.Visibility menu is state-aware —
src/item-visibility-menu.ts"Also show on <rail>" is hidden when the item is already on
both(it was a no-op re-write).OS Settings → Apps & Icons repaints live —
src/settings/sections/apps-icons.tsThe tab subscribes to
wp.desktop.subscribeOsSettingsand repaints on external writes (e.g. a right-click visibility change), self-unsubscribing once the panel is torn down. Previously the row dropdown showed stale placement until the panel was rebuilt.Reset no longer corrupts the DEFAULTS singleton —
src/settings/state.ts,src/settings/panel.tsstructuredDefaults()now deep-clones the collection fields (itemVisibility,dockOrder,dockPromotedPositions), andonResetuses it instead of a shallow{ ...DEFAULTS }. Fixes the custom-gradient editor mutating the module-levelDEFAULTSfor the rest of the session after a Reset.dockOrdersurvives the PHP round-trip —includes/os-settings.phpCross-rail tiles persist with a rail-synthesis prefix (
desktop:<id>/dock:<id>).sanitize_key()stripped the colon, silently resetting the user's order (and risking an id collision) on reload. ThedockOrdersanitizer now preserves the colon while still rejecting anything outside the JS id charset. (itemVisibility / dockPromotedPositions keys are canonical and unaffected.)🟡 Low
Content-less Screen Options / Help no longer surface a dead button — both bridges
A panel is announced only when its toggle link is present and the panel has renderable content (form controls for Screen Options; non-empty tab/sidebar text for Help).
Contract drift corrected —
src/settings/types.ts,docs/javascript-reference.mdThe
itemVisibilitydocblock wrongly claimed'both'is never persisted; it is (PHP whitelists it, the menu stores it). Doc updated, plus the empty-arrayscreen-metasemantics.dockPromotedPositionsno longer leaks —src/settings/desktop-shortcuts-sync.tsWhen a promoted item is demoted/hidden, its persisted drag position is pruned (reentrancy-safe), so stale coordinates can't accumulate toward the 256-entry cap or silently resurrect on a future re-promote.
Testing
npm run typecheck✓npm run lint✓npm run test:js✓ — 1548 passed (added spatial-layout coverage: pinned icon kept, explicit promotion kept, non-pinned/no-override still dropped)npm run build✓php -l✓ on both changed PHP filesnpm run test:php✓ — 926 tests / 2175 assertions (addeddockOrdercolon-preservation + normalization tests)Notes for reviewers
pinnedframework icons and explicitly-promoted icons survive — to avoid unilaterally changing the documented "core surface" behavior. If we'd rather Spatial mirror Classic/Unified and show all desktop-resolved server icons, it's a one-line change to the filter.🤖 Generated with Claude Code