From 36fab711a210f81dfc861b10201adf49feff7019 Mon Sep 17 00:00:00 2001 From: KCM Date: Thu, 30 Apr 2026 09:33:34 -0500 Subject: [PATCH 1/2] docs: remove obsolete and outdated. --- docs/issue-99-workspaces-drawer-plan.md | 71 ------------------- ...rawer-workspace-context-separation-plan.md | 46 ------------ docs/render-pipeline-multitab-spec-plan.md | 51 ------------- 3 files changed, 168 deletions(-) delete mode 100644 docs/issue-99-workspaces-drawer-plan.md delete mode 100644 docs/pr-drawer-workspace-context-separation-plan.md delete mode 100644 docs/render-pipeline-multitab-spec-plan.md diff --git a/docs/issue-99-workspaces-drawer-plan.md b/docs/issue-99-workspaces-drawer-plan.md deleted file mode 100644 index 4656b19..0000000 --- a/docs/issue-99-workspaces-drawer-plan.md +++ /dev/null @@ -1,71 +0,0 @@ -# Issue #99 + Workspaces Drawer UX Plan - -## Goal - -Simplify PR/workspace lifecycle and the Workspaces drawer UX by: - -- removing disconnected state and Disconnect action paths -- using workspace terminology in the drawer (not context) -- separating new workspace initialization from workspace selection -- preserving strict explicit intent semantics (no implicit apply/mutation from select changes) - -## Decisions - -- New workspace is an explicit direct action via a `New workspace` button. -- `New workspace` must work for both repository scopes and `Local`. -- `Open` remains the explicit action for applying an existing stored workspace. -- If the selected repository has no stored workspaces, hide the workspace select and show the new-workspace path. - -## Implementation Steps - -1. Remove disconnected model paths (Issue #99) - -- Remove Disconnect control from UI. -- Remove disconnected event wiring and runtime callbacks. -- Remove disconnected public action paths. -- Normalize legacy `disconnected` records to `inactive` during restore/normalization. - -2. Redesign drawer flow - -- Replace starter option-in-select behavior with a dedicated `New workspace` button adjacent to repository select. -- Keep workspace select for stored workspaces only. -- Hide workspace select when no stored workspaces exist for the selected scope. -- Keep strict explicit selection semantics (no auto-apply from select/filter changes). - -3. Update copy and accessibility - -- Replace "Stored contexts" and related "context" wording with "Workspace" wording. -- Update status and aria labels consistently. - -4. Remove obsolete code paths - -- Remove starter prefix constants and parsing. -- Remove disconnected-only logic and stale styling/branches. - -5. Update tests - -- Remove/replace disconnected-focused scenarios. -- Update helpers/selectors to new workspace labels and `New workspace` action. -- Add/adjust scenarios for empty repository scope (select hidden) and explicit Open behavior for existing workspaces. - -6. Update docs - -- Update storage/state docs to remove disconnected semantics. -- Update drawer UX docs to reflect repository row + new workspace action flow. - -## Verification - -1. `npm run lint` -2. Targeted Playwright (Chromium first): - -- `playwright/github-pr-drawer/active-context-switch.spec.ts` -- `playwright/github-pr-drawer/open-pr-create.spec.ts` - -3. Broader Playwright run for workspace/PR drawer flows. -4. Manual verification in dev server for: - -- repository + new workspace row -- local new workspace creation -- hidden workspace select when no stored entries -- explicit Open required for existing entries -- no Disconnect control diff --git a/docs/pr-drawer-workspace-context-separation-plan.md b/docs/pr-drawer-workspace-context-separation-plan.md deleted file mode 100644 index c955287..0000000 --- a/docs/pr-drawer-workspace-context-separation-plan.md +++ /dev/null @@ -1,46 +0,0 @@ -# PR Drawer vs Workspace Contexts Remaining Backlog - -This document now tracks only work not yet implemented from the original separation plan. - -## Completed (for context) - -1. Workspaces button and dedicated Workspaces drawer exist. -2. PR drawer no longer exposes component/styles filename inputs. -3. Commit targets are derived from workspace tab metadata. -4. Checkbox copy uses entry-tab language. -5. Confirmation summary includes a Files to commit list. - -## Remaining Work - -### Phase B follow-up - -1. Add multi-select removal in Workspaces drawer. -2. Add richer filtering in Workspaces drawer. -3. Decide whether to keep a quick context-switch affordance in PR drawer. - -### Phase C enhancements - -1. Add open PR binding tools to Workspaces drawer. -2. Add context health indicators in the Workspaces list (dirty, synced, drift). -3. Add optional pin/favorite/recents support. -4. Evaluate optional tab include/exclude toggles for commit targets. - -### Confirmation summary UX polish - -1. For long file lists, cap visible rows and show a +N more summary. - -### Modularization follow-up - -Current implementation is primarily `src/modules/workspaces-drawer/drawer.js`. - -1. Split module if needed into smaller units: - - `state.js` - - `list-render.js` - - `actions.js` -2. Keep PR transactional logic isolated in PR modules. - -## Validation Coverage to Keep - -1. PR drawer tests remain focused on transactional workflows. -2. Workspaces drawer tests cover search/select/delete and future multi-select behavior. -3. Migration tests ensure existing stored contexts are retained. diff --git a/docs/render-pipeline-multitab-spec-plan.md b/docs/render-pipeline-multitab-spec-plan.md deleted file mode 100644 index b03d319..0000000 --- a/docs/render-pipeline-multitab-spec-plan.md +++ /dev/null @@ -1,51 +0,0 @@ -# Render Pipeline + Multi-Tab Vital Remaining Spec TODOs - -This file tracks only high-value gaps that still need coverage. - -## Already Covered (summary) - -1. Entry-tab role behavior, rename stability, and restore behavior. -2. Cross-tab import graph basics, missing-module and circular-import determinism. -3. Core diagnostics/status flows, including pending/error/neutral transitions. -4. Workspace persistence and per-repo context/config isolation baseline. - -## Vital Remaining TODOs - -## 1. Default Export Support Matrix (High Risk) - -Add explicit render/typecheck specs for: - -1. `export default class ...` in React mode. -2. `function App() { ... } export default App` behavior. -3. `const X = ...; export default X` behavior with entry-wrapper rules. -4. Unsupported default-export combinations producing deterministic diagnostics. - -## 2. Style Compile vs Lint Contract (High Risk) - -Lock down precedence and parity: - -1. Less error-path parity with Sass error behavior. -2. Compile diagnostics + lint diagnostics precedence/coexistence contract. -3. Clearing styles diagnostics does not affect component diagnostics/status. - -## 3. Status Aggregation Contract (High Risk) - -Add state-machine coverage for: - -1. Multiple simultaneous error sources aggregating counts correctly. -2. Clearing one scope updates only that scope and leaves other error states intact. - -## 4. Inactive Panel Mutation Guard (Medium Risk) - -Add keyboard/actionability spec that proves inactive editor panel input cannot mutate source. - -## 5. Update Obsolete PR Path-Validation Section (Doc/Test Hygiene) - -Old PR drawer filename-field validation cases are obsolete after tab-derived commit targets. - -1. Replace with tab-derived commit target validation/normalization tests. -2. Remove any remaining assumptions about component/styles filename fields in PR drawer flows. - -## Minimal Done Criteria - -This plan is complete when the five sections above are covered by Playwright tests and linked from the affected suites. From e2be5d088054bdf025e19b7f7145cc64bdd5ef53 Mon Sep 17 00:00:00 2001 From: KCM Date: Thu, 30 Apr 2026 09:34:09 -0500 Subject: [PATCH 2/2] fix: iframe protocol mismatch in receiver reads. --- playwright/layout-panels.spec.ts | 32 +++++++++++++++ .../iframe-preview-executor.js | 41 +++++++++++++++---- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/playwright/layout-panels.spec.ts b/playwright/layout-panels.spec.ts index be61b37..5647894 100644 --- a/playwright/layout-panels.spec.ts +++ b/playwright/layout-panels.spec.ts @@ -3,6 +3,7 @@ import { expectCollapseButtonState, expectPreviewHasRenderedContent, getCollapseButton, + getPreviewFrame, getToolsButton, waitForInitialRender, } from './helpers/app-test-helpers.js' @@ -61,6 +62,37 @@ test('dark theme defaults preview background to editor background', async ({ pag expect(toRgbChannels(colors.preview)).toEqual(toRgbChannels(colors.editor)) }) +test('changing preview background keeps applied preview styles', async ({ page }) => { + await waitForInitialRender(page) + + const previewFrameRoot = getPreviewFrame(page).locator('html') + + await expect(previewFrameRoot).toHaveCount(1) + const hasComponentStylesBefore = await previewFrameRoot.evaluate(() => { + const styleElement = document.getElementById('knighted-preview-styles') + if (!(styleElement instanceof HTMLStyleElement)) { + return false + } + + return styleElement.textContent?.includes('.counter-button') ?? false + }) + expect(hasComponentStylesBefore).toBe(true) + + await page.getByLabel('Background').fill('#b1aaaa') + + const hasComponentStylesAfter = await previewFrameRoot.evaluate(() => { + const styleElement = document.getElementById('knighted-preview-styles') + if (!(styleElement instanceof HTMLStyleElement)) { + return false + } + + return styleElement.textContent?.includes('.counter-button') ?? false + }) + expect(hasComponentStylesAfter).toBe(true) + + await expect(previewFrameRoot).toHaveCSS('background-color', 'rgb(177, 170, 170)') +}) + test('fixed layout keeps preview panel height within editor stack height', async ({ page, }) => { diff --git a/src/modules/preview-runtime/iframe-preview-executor.js b/src/modules/preview-runtime/iframe-preview-executor.js index ad93135..981dbae 100644 --- a/src/modules/preview-runtime/iframe-preview-executor.js +++ b/src/modules/preview-runtime/iframe-preview-executor.js @@ -56,6 +56,11 @@ const createIframeShellDocument = ({ channelId, parentOrigin, importMap }) => { entrySpecifier: '', reactRoot: null, renderedNodes: [], + visualConfig: { + cssText: '', + hostPadding: '', + backgroundColor: '', + }, } const __knightedRuntimeErrorFingerprints = new Set() @@ -110,6 +115,12 @@ const createIframeShellDocument = ({ channelId, parentOrigin, importMap }) => { } const __knightedApplyVisualConfig = ({ cssText = '', hostPadding = '', backgroundColor = '' }) => { + __knightedState.visualConfig = { + cssText: typeof cssText === 'string' ? cssText : '', + hostPadding: typeof hostPadding === 'string' ? hostPadding : '', + backgroundColor: typeof backgroundColor === 'string' ? backgroundColor : '', + } + let styleElement = document.getElementById('knighted-preview-styles') if (!(styleElement instanceof HTMLStyleElement)) { styleElement = document.createElement('style') @@ -117,17 +128,24 @@ const createIframeShellDocument = ({ channelId, parentOrigin, importMap }) => { document.head.append(styleElement) } - styleElement.textContent = __knightedToBaseStyles(hostPadding) + '\\n' + String(cssText) + styleElement.textContent = + __knightedToBaseStyles(__knightedState.visualConfig.hostPadding) + + '\\n' + + String(__knightedState.visualConfig.cssText) - if (typeof hostPadding === 'string' && hostPadding.trim().length > 0) { - document.documentElement.style.setProperty('--preview-host-padding', hostPadding.trim()) + if (__knightedState.visualConfig.hostPadding.trim().length > 0) { + document.documentElement.style.setProperty( + '--preview-host-padding', + __knightedState.visualConfig.hostPadding.trim(), + ) } else { document.documentElement.style.removeProperty('--preview-host-padding') } - if (typeof backgroundColor === 'string' && backgroundColor.length > 0) { - document.documentElement.style.backgroundColor = backgroundColor - document.body.style.backgroundColor = backgroundColor + if (__knightedState.visualConfig.backgroundColor.length > 0) { + document.documentElement.style.backgroundColor = + __knightedState.visualConfig.backgroundColor + document.body.style.backgroundColor = __knightedState.visualConfig.backgroundColor return } @@ -285,7 +303,16 @@ const createIframeShellDocument = ({ channelId, parentOrigin, importMap }) => { const data = event.data if (data.type === __knightedMessageTypes.configPatch) { - __knightedApplyVisualConfig(data) + const patch = + data && typeof data.payload === 'object' && data.payload !== null + ? data.payload + : data && typeof data === 'object' + ? data + : {} + __knightedApplyVisualConfig({ + ...__knightedState.visualConfig, + ...patch, + }) return }