Add splitscreen tools architecture documentation for #107

[cubap]

No description provided.

[cubap]

closes #107

[github-actions]

🚀 Deployed on https://69b8402e09131f55a71e43c9--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.

[Copilot]

The “Sidebar Tools” description says this mode is a fixed-width, sliding, non-resizable panel. In the current interfaces, tools with location === "sidebar" are still rendered in the same resizable split-pane layout (no slide-in behavior and no location-specific sizing), so this section should be updated to match the actual behavior or explicitly label it as a planned/not-yet-implemented mode.

Suggested change

	**Visual behavior**: Fixed-width panel that slides in from the right, non-resizable.
	**Use cases**: Compact tools, configuration panels, quick reference materials.
	**Current implementation**:
	- Location identifier: `"sidebar"`
	- Shares activation mechanism with pane tools
	**Visual behavior (planned)**: Fixed-width panel that slides in from the right, non-resizable.
	**Current behavior**: Rendered in the same resizable split-pane layout as `"pane"` tools; no slide-in animation or location-specific sizing yet.
	**Use cases**: Compact tools, configuration panels, quick reference materials.
	**Current implementation**:
	- Location identifier: `"sidebar"`
	- Shares activation mechanism with pane tools
	- Uses the same split-pane layout and resizing behavior as `"pane"` tools
	- `location === "sidebar"` is currently a semantic/future-facing flag; dedicated sidebar layout is not yet implemented

[Copilot]

This “Drawer Tools” section points to components/magnifier-tool/index.js as the current implementation, but the magnifier is implemented as a floating overlay (toggled via the toolbar button) rather than a drawer/tray. The actual drawer-style implementation in this codebase is tpen-page-tool, which opens a .drawer and dispatches drawer-opened/drawer-closed events; consider updating this section accordingly.

Suggested change

	**Visual behavior**: Floating tray that slides over content, dismissible without losing context.
	**Use cases**: Magnifier, inspector, temporary overlays.
	**Current implementation**:
	- [components/magnifier-tool/index.js](../magnifier-tool/index.js)
	- Activated via specific tool buttons or shortcuts
	**Visual behavior**: Content-attached drawer (e.g., from bottom or side) that slides over part of the workspace while keeping the underlying context visible.
	**Use cases**: Page-level tools that need temporary, dismissible space without a full split-screen (e.g., navigation aids, page metadata, inline helpers).
	**Current implementation**:
	- Implemented by `tpen-page-tool`, which opens/closes a `.drawer` element
	- Dispatches `drawer-opened` / `drawer-closed` events for coordination with the host interface
	- Activated via page-level toolbar controls or equivalent triggers

[Copilot]

The magnifier tool isn’t “drawer-style” and, within MagnifierTool, there is no close button (it’s hidden via Escape or by toggling from the workspace toolbar). To avoid misleading implementers, update these bullets to reflect the current magnifier UI/interaction model, and consider listing tpen-page-tool as the drawer tool example instead.

Suggested change

	3. **tpen-magnifier-tool** - Drawer-style image magnification overlay
	- Activated via toolbar button
	- Dismissible via Escape key or close button
	- Reference: [components/magnifier-tool/index.js](../magnifier-tool/index.js)
	4. **Individual tool implementations** (e.g., tpen-page-tool, tpen-quicktype-tool)
	3. **tpen-magnifier-tool** - Image magnification overlay
	- Activated via toolbar button
	- Dismissible via Escape key or by toggling the magnifier button in the workspace toolbar
	- Reference: [components/magnifier-tool/index.js](../magnifier-tool/index.js)
	4. **Individual tool implementations** (e.g., tpen-page-tool (drawer-style), tpen-quicktype-tool)