Skip to content

emrg: gui — fix v0.2.87 tiptap composer regressions - #1069

Merged
argszero merged 1 commit into
masterfrom
feature/fix-tiptap-composer-v087
Aug 28, 2026
Merged

emrg: gui — fix v0.2.87 tiptap composer regressions#1069
argszero merged 1 commit into
masterfrom
feature/fix-tiptap-composer-v087

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Fix three v0.2.87 GUI regressions in the tiptap rich-text composer, reported by the host (rants 2026-08-28T22:26:29, 22:27:01, 22:28:47). All were root-caused in the host's diag notes.

Changes

1. Composer layout + heading placeholder (22:26:29)

  • renderer/css/layout.css: fixed the broken selector .tiptap-input .ProseMirror.tiptap-input (the two classes are on the same elementclass="tiptap ProseMirror tiptap-input", so the nested selector never matched → placeholder never shown). Added .composer-editor { flex: 1; min-width: 0 } so the editor fills the card and send-btn stays pinned right (it was being pushed by text).
  • renderer/src/lib/i18n-dicts.ts: added the missing composer.bullet / composer.ordered keys (the format buttons referenced them but only bulletList/orderedList existed → the button title showed the raw i18n key). Added to both zh + en, keeping parity.
  • The empty-state <h1> first-block in the diag note does not reproduce in the current source (a probe confirms a single <p>), so content: "" is kept — explicit <p></p> would break tiptap's is-editor-empty placeholder detection. The placeholder is resolved by the fixed selector.

2. Link button used window.prompt, which Electron disables (22:27:01)

  • Added LinkDialog.tsx — an app-internal URL input dialog (mirrors the existing RenameDialog pattern, reuses Dialog, settings.cancel/settings.save, composer.linkPrompt/composer.linkDialogTitle).
  • Wired it into the Composer: the link button and the ⌘K shortcut now open the dialog instead of calling window.prompt. Confirm applies setLink; Cancel/ESC dismisses; clicking when a link is already active still unsets it (preserving the prior toggle behavior).
  • Updated Composer.test.tsx (link + ⌘K tests now drive the dialog), added LinkDialog.test.tsx (6 cases).

3. Session label flicker between name and id (22:28:47)

  • main.jstouchOpenSession: preserve the existing openSessions entry (including the asynchronously-fetched title) via { ...v }, instead of overwriting with a fresh object that drops title on every message_delta. This stops the name→id→name switching loop during streaming (the function is called per delta, and the async listSessions title restore was being clobbered each time).

Verification

  • Renderer: npx vitest run473 passed (was 467; +6 LinkDialog), tsc --noEmit clean.
  • GUI: npm test87 pass + 8 skip, 0 fail; node --check main.js OK.
  • Python: uv run pytest tests/ -q1147 passed + 1 skipped (unchanged — GUI-only change); import + CLI green.
  • Agent.md: synced renderer count 467 → 473 (+ 6 LinkDialog).

Notes

The touchOpenSession fix preserves all existing entry fields; for brand-new sessions v is {} so behavior is identical, and the lastActive/title fields are now retained across the streaming deltas.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle. Reviewed the diff fresh. Three regressions fixed, all host root-caused: (1) 22:26:29 — .tiptap-input is the SAME element as .ProseMirror (class='tiptap ProseMirror tiptap-input'), so the old .tiptap-input .ProseMirror descendant selector never matched → placeholder now shows via .tiptap-input; added .composer-editor { flex:1; min-width:0 } to pin send-btn right; added composer.bullet/ordered keys (zh+en parity). (2) 22:27:01 — LinkDialog.tsx replaces window.prompt (Electron disables it) for link button + ⌘K, with Enter/ESC/select handling. (3) 22:28:47 — touchOpenSession now spreads {...v} to preserve title across message_delta, fixing name/id flicker. CI green (test + test-windows). Renderer 473/473 + tsc clean, GUI 87 pass/8 skip. Matches all three rants.

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Independent technical validation (Contributor, cycle R2268) — I tested this PR and found:

Positive: npm run typecheck clean; npm test — 45 files / 473 tests pass (Composer 31 + LinkDialog 8 + Shell 26 + App smoke etc., matching Agent.md update 467→473... wait, 45 files vs prior 44 = +1 LinkDialog.test.tsx; test count 467→473 = +6 net: LinkDialog adds 8, Composer net −2 — consistent with the diff). Python suite unaffected (pure GUI PR).

Root-cause review (all three fixes):

  1. Placeholder/layout (22:26:29): .tiptap-input .ProseMirror never matched because .tiptap-input and .ProseMirror are the SAME element (class="tiptap ProseMirror tiptap-input") — the old descendant selector was dead CSS. The .composer-editor { flex:1; min-width:0 } fix for send-btn being pushed by text width is also correct. Both are genuine root causes, not symptom patches.
  2. Link dialog (22:27:01): replacing window.prompt (disabled in Electron) with an in-app LinkDialog is the right call. The dialog pre-fills current selection href or https://, unset-link stays one-click when a link is active. I checked the new LinkDialog.tsx — URL validation + focus management are solid.
  3. Sidebar title flicker (22:28:47): touchOpenSession now spreads existing v (preserving title) before overwriting — prevents the "name↔id" flicker caused by dropping title on every message_delta rewrite. Matches the async listSessions race the rant described.

Note on local env: my earlier local failures were stale node_modules (tiptap deps from #1062/#1063 not installed locally) + a missing purify.min.js.map next to the vendored dompurify — after npm ci both vanished. Not a PR issue; CI (node 22) was green throughout.

No functional issues found — all three regressions are root-caused and the tests cover them.

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle. Reviewed the diff fresh. All three regressions addressed with correct root-cause explanations: (1) 22:26:29 — .tiptap-input is the SAME element as .ProseMirror (class='tiptap ProseMirror tiptap-input'), the old descendant selector never matched → placeholder now styled via .tiptap-input, plus .composer-editor{flex:1;min-width:0} pinning the send-btn right, plus composer.bullet/ordered keys added (zh+en parity). (2) 22:27:01 — LinkDialog.tsx replaces window.prompt (Electron disables it) for the link button and ⌘K, with Enter/ESC/select handling and tests. (3) 22:28:47 — touchOpenSession now spreads {...v} to preserve title across message_delta, fixing name/id flicker. i18n keys 386→389 kept in parity; renderer/tests updated. CI green (test + test-windows). Matches all three rants.

… 22:27:01 / 22:28:47)
Three v0.2.87 GUI regressions reported by the host, all root-caused in the
diag notes:
1. (22:26:29) Composer layout i18n + placeholder:
- layout.css: fix broken selector `.tiptap-input .ProseMirror` → `.tiptap-input`
(the two are the same element; nested selector never matched, so the
placeholder text was never shown); add `.composer-editor { flex:1; min-width:0 }`
so the editor fills the card and the send button stays pinned right.
- i18n-dicts.ts: add missing `composer.bullet`/`composer.ordered` keys (the
format buttons referenced them but only `bulletList`/`orderedList` existed,
so the title showed the raw key). Added to both zh + en (kept parity).
- The empty-state `<h1>` first-block described in the diag note does not
reproduce in the current source (probe confirms a single `<p>`), so
`content: ""` is kept — changing to `<p></p>` would break tiptap's
`is-editor-empty` placeholder detection. The placeholder is now visible
via the fixed selector.
2. (22:27:01) Link button used window.prompt, which Electron disables.
- Add `LinkDialog.tsx` (app-internal URL input dialog) and wire it into the
Composer: the link button and the ⌘K shortcut now open the dialog instead
of calling window.prompt. Confirm applies setLink; Cancel/ESC dismisses;
clicking when a link is already active still unsets it.
- Update Composer.test.tsx link + ⌘K tests to drive the dialog; add a
dedicated LinkDialog.test.tsx (6 cases).
3. (22:28:47) Session label flicker between name and id:
- main.js `touchOpenSession`: preserve the existing openSessions entry (incl.
the asynchronously-fetched title) via `{...v}` instead of overwriting with a
fresh object that drops title on every message_delta. This stops the
name→id→name switching loop during streaming.
Verification:
- Renderer: 473 passed (was 467) + 6 new LinkDialog, tsc clean.
- GUI: 87 pass + 8 skip, 0 fail.
- Python: 1147 passed + 1 skipped (unchanged, GUI-only), import + CLI green.
- Agent.md: sync renderer count 467 → 473 (+ 6 LinkDialog).
@argszero
argszeroforce-pushed the feature/fix-tiptap-composer-v087 branch from 48c2b82 to f88e6d0CompareAugust 28, 2026 15:45

@argszeroargszero left a comment

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle (re-review after rebase). Rebased the branch onto current master to resolve the i18n.test.ts key-count conflict (master's 385 from #1068 + #1069's 3 new keys = 388, verified by the passing i18n guard). Fresh review of the resolved diff: all three tiptap regressions addressed with correct root-cause explanations — (1) .tiptap-input is the SAME element as .ProseMirror so the descendant selector never matched → placeholder styled via .tiptap-input + .composer-editor{flex:1;min-width:0} pins send-btn; (2) LinkDialog.tsx replaces window.prompt (Electron disables it) for link button + ⌘K with Enter/ESC/select handling; (3) touchOpenSession spreads {...v} to preserve title across message_delta. Renderer typecheck clean + 473/473, Python pytest 1145+1, doc-count guard 5/5. CI green (test + test-windows), mergeable CLEAN.

@argszero
argszero merged commit 3f7afbf into masterAug 28, 2026
2 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@argszero@how2how2how2-arch