Add splitscreen tools architecture documentation for #107

[cubap]

No description provided.

[cubap]

closes #107

[github-actions]

🚀 Deployed on https://69e790b55d26b012b12cfdaf--tpen-interfaces-preview.netlify.app

[thehabes]

I'm not sure where you will draw the line, but just make sure things are accurate as you need them to be.

Static Review Comments

Category	Issues Found
🔴 Critical	0
🟠 Major	3
🟡 Minor	3
🔵 Suggestions	2

Critical Issues 🔴

None.

Major Issues 🟠

Issue 1: Dismissal lifecycle is inaccurate — tools-dismiss is never dispatched

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md:132-136
Category: Accuracy / Logic Error

Problem:
The doc states the dismissal flow is:

1. User clicks close button or presses Escape key
2. Interface dispatches tools-dismiss event
3. Interface shell collapses split-screen layout
4. Tool-specific hide event dispatched: tpen-${toolName}-hide

This is wrong on multiple counts:

• tools-dismiss is never dispatched anywhere in the codebase. Both interface shells only listen for it (simple-transcription/index.js:395, interfaces/transcription/index.js:275). The close button and Escape key call closeSplitscreen() directly without dispatching any event.
• No tpen-{toolName}-hide event is dispatched on dismissal. The hide event is only dispatched by splitscreen-tool/index.js:61 during tool switching, not panel close.

Current Code (the actual close path in simple-transcription/index.js):

const closeSplitscreen = () => {
  if (!this.state.isSplitscreenActive) return
  this.state.isSplitscreenActive = false
  this.toggleSplitscreen()
  this.updateLines()
}

this.renderCleanup.onElement(this.shadowRoot, 'click', e => {
  if (e.target?.classList.contains('close-button')) closeSplitscreen()
})

this.renderCleanup.onWindow('keydown', (e) => {
  if (e.key === 'Escape') closeSplitscreen()
})

this.renderCleanup.onEvent(TPEN.eventDispatcher, 'tools-dismiss', closeSplitscreen)

Suggested Fix:

### Dismissal

1. User clicks close button, presses Escape key, or an external component dispatches `tools-dismiss` via eventDispatcher
2. Interface shell calls `closeSplitscreen()` directly — sets `isSplitscreenActive = false` and collapses layout
3. **Note**: No `tpen-{toolName}-hide` event is currently dispatched on panel close (only on tool switch). Tools that need cleanup on dismiss should listen for DOM visibility changes or the `tools-dismiss` eventDispatcher event.

---

Issue 2: tpen-{toolName}-show/hide events are dispatched by splitscreen-tool, not the interface shell

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md:148-153
Category: Accuracy

Problem:
The Event Contracts table states the direction for show/hide events is "Interface → Tool". In reality, these events are dispatched by tpen-splitscreen-tool (the dropdown selector), not the interface shell:

// splitscreen-tool/index.js:61-62
if (e.target.dataset.prev) eventDispatcher.dispatch(`tpen-${e.target.dataset.prev}-hide`)
eventDispatcher.dispatch(`tpen-${value}-show`)

Similarly, the tools-dismiss direction is listed as "Interface → Tools", but it is never dispatched by the interface — it is only listened for. Its actual dispatcher is currently unknown (possibly intended for future use or external callers).

Suggested Fix (Event Contracts table):

| Event | Direction | Detail | Purpose |
|-------|-----------|--------|---------|
| `splitscreen-toggle` | Selector → Interface | `{ selectedTool: string }` | Activate/switch split-screen tool |
| `tools-dismiss` | External → Interface | none | Request interface to close split-screen panel |
| `tpen-{toolName}-show` | Selector → Tool | none | Notify tool it's visible (on switch) |
| `tpen-{toolName}-hide` | Selector → Tool | none | Notify previous tool it's hidden (on switch only) |

---

Issue 3: Tool configuration schema is materially incomplete — missing url and custom.tagName

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md:81-91
Category: Accuracy / Completeness

Problem:
The documented tool configuration schema only shows toolName, label, location, and custom.enabled. However, the actual tool loading logic in simple-transcription/index.js:830-921 depends on two critical additional properties that determine how a tool is rendered:

• tool.url — Used to load iframe tools (line 856-918) and custom element scripts (line 831-853)
• tool.custom.tagName — Used to load native web component tools (line 830-853)

Without these properties documented, a developer cannot understand how tools are actually instantiated.

Suggested Fix:

{
  toolName: "dictionary",        // Unique identifier
  label: "Dictionary",            // Display name
  location: "pane",               // Type: "pane" | "sidebar" | "drawer" | "dialog"
  url: "https://example.com/tool.js", // Optional: URL for iframe src or module script
  custom: {
    enabled: true,                // Optional: defaults to true if omitted
    tagName: "tpen-dictionary",   // Optional: custom element tag name (requires url)
    // Tool-specific configuration
  }
}

Add a note explaining the rendering decision tree:

1. If custom.tagName AND url → load script from url, then render <tagName>
2. If url AND no tagName AND location === 'pane' → render as iframe with src = url
3. Otherwise → fallback message

---

Minor Issues 🟡

Issue 4: iframe communication is described as "one-way" but the code is bidirectional

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md:192
Category: Accuracy

Problem:
The Legacy Tools section states:

Communication is one-way: interface → tool via URL

The actual implementation uses bidirectional postMessage:

• Interface → Tool: iframe.contentWindow.postMessage(...) sends MANIFEST_CANVAS_ANNOTATIONPAGE_ANNOTATION, CANVASES, CURRENT_LINE_INDEX, and SELECT_ANNOTATION messages (simple-transcription/index.js:873-899)
• Tool → Interface: #handleToolMessages(event) handles incoming CURRENT_LINE_INDEX, RETURN_LINE_ID, SELECT_ANNOTATION, and NAVIGATE_TO_LINE messages (simple-transcription/index.js:929-958)

Suggested Fix:

- Communication is bidirectional via `postMessage`:
  - Interface → Tool: sends manifest, canvas, page, line context on load and line changes
  - Tool → Interface: sends line navigation requests (`NAVIGATE_TO_LINE`, `SELECT_ANNOTATION`, etc.)
- Origin validation is enforced for incoming messages

---

Issue 5: Activation lifecycle step 5 is misleading

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md:129
Category: Accuracy

Problem:
Step 5 of the activation flow states:

Tool-specific show event dispatched: tpen-${toolName}-show

This is listed as a separate step after "Interface updates state," implying the interface dispatches it. In reality, the show event is dispatched by splitscreen-tool in the same change event handler that fires splitscreen-toggle (line 56-62 of splitscreen-tool/index.js). Steps 2 and 5 happen in the same synchronous handler, not sequentially after the interface processes step 3-4.

Suggested Fix:
Reorder to reflect the actual sequence:

1. User selects tool from splitscreen dropdown
2. `splitscreen-tool` dispatches `splitscreen-toggle` DOM event with `{ selectedTool: toolName }`
3. `splitscreen-tool` dispatches `tpen-${previousTool}-hide` (if switching) and `tpen-${toolName}-show` via eventDispatcher
4. Interface shell receives bubbled `splitscreen-toggle` event
5. Interface updates state, toggles layout, and loads tool content

---

Issue 6: Doc references only tpen-simple-transcription but the same patterns exist in interfaces/transcription/

File: components/workspace-tools/SPLITSCREEN_TOOLS_ARCHITECTURE.md (throughout)
Category: Completeness

Problem:
The document consistently references tpen-simple-transcription as the sole interface shell, but interfaces/transcription/index.js implements the same splitscreen patterns (same event listeners, same closeSplitscreen/openSplitscreen logic, same loadRightPaneContent method). The doc should acknowledge both consumers.

Suggested Fix:
Add a brief note in the "Interface Shell" section or the intro:

**Note**: The splitscreen patterns are implemented in both `tpen-simple-transcription`
(`components/simple-transcription/index.js`) and the standard transcription interface
(`interfaces/transcription/index.js`). Both follow the same event contracts.

---

Suggestions 🔵

Suggestion 1: Document the postMessage protocol for iframe tools

The iframe integration is a non-trivial protocol with multiple message types (MANIFEST_CANVAS_ANNOTATIONPAGE_ANNOTATION, CANVASES, CURRENT_LINE_INDEX, SELECT_ANNOTATION, RETURN_LINE_ID, NAVIGATE_TO_LINE). Third-party tool developers would benefit from a message contract table similar to the event contracts table. This could live in this doc or a separate iframe tools reference doc.

---

Suggestion 2: Add a simple ASCII or mermaid diagram for the event flow

The lifecycle sections describe a multi-step flow across 3+ components. A sequence diagram (even simple ASCII) would make the event routing much clearer than prose, especially given the nuance of who dispatches what.

[Copilot]

Pull request overview

Adds a new architecture document describing how “splitscreen tools” are configured and integrated into the TPEN transcription interfaces, including tool modes, lifecycle, and event contracts.

Changes:

• Introduces SPLITSCREEN_TOOLS_ARCHITECTURE.md to document tool types (pane/sidebar/drawer/dialog) and their intended behaviors.
• Documents tool configuration via TPEN.activeProject.tools and the rendering decision tree (custom element vs iframe vs fallback).
• Describes activation/dismissal flow and event contracts (splitscreen-toggle, tools-dismiss, tpen-{tool}-show/hide).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.