fix(composer): Enter defers to an open mention/slash combobox (LC-171) #216

Merged

nrupard merged 2 commits from fix/lc-171-composer-enter-guard into main 2026-05-26 17:44:39 +02:00

Conversation 0 Commits 2 Files changed 2 +47

nrupard commented 2026-05-26 17:34:51 +02:00

Owner

Copy link

Closes LC-171. Raised in the LC-168 review: could Enter, with a composer combobox popover open, both submit the form AND insert the highlighted option?

Resolution

No browser in the dev env, but the question is decidable by the DOM event model. There are three Enter-handling keydown listeners, all on the same textarea:

• the inline onkeydown (submits via requestSubmit()), registered at parse time;
• the mention combobox handler (addEventListener(..., true)), registered later;
• the slash combobox handler (addEventListener(..., true)), registered later still.

At the target element, listeners fire in registration order regardless of the capture flag, and stopPropagation() (what the comboboxes call) does not stop other listeners on the same target - only stopImmediatePropagation() does. So the inline handler runs first and calls requestSubmit() with the partial text (e.g. @ali) before the combobox inserts the resolved option, which hx-on::after-request then wipes. The double-fire is real per spec; the visible symptom is a broken half-typed send.

Fix

The inline handler short-circuits before submitting when an aria-selected option exists in either popover (#lc-mention-popover / #lc-slash-popover):

if (document.querySelector('#lc-mention-popover li[role=option][aria-selected=true], #lc-slash-popover li[role=option][aria-selected=true]')) return;

Properties:

• Ordering-independent: it does not rely on which listener runs first, so it is correct whatever a given browser does.
• Shared path: it lives in the one inline handler both comboboxes share, so it covers mention and slash identically with no divergence (the issue explicitly warned against fixing slash alone).
• No stray newline: the combobox keydown handler still preventDefaults the Enter.

Test

A regression test in routes_drafts.rs renders the composer and asserts both that the guard selector is present and that it precedes requestSubmit() in the markup, since the ordering of the short-circuit is what makes it correct.

Verification

just test / just test-saas: all binaries green except the known routes_uploads concurrent-load flake (documented in CLAUDE.md), which passes 12/12 in isolation in both modes. This change touches only room/composer.html and the drafts test - nothing upload-related. cargo clippy / cargo fmt clean.

Closes LC-171. Raised in the LC-168 review: could Enter, with a composer combobox popover open, both submit the form AND insert the highlighted option? ## Resolution No browser in the dev env, but the question is decidable by the DOM event model. There are three Enter-handling keydown listeners, all on the same textarea: - the inline `onkeydown` (submits via `requestSubmit()`), registered at parse time; - the mention combobox handler (`addEventListener(..., true)`), registered later; - the slash combobox handler (`addEventListener(..., true)`), registered later still. At the target element, listeners fire in registration order regardless of the capture flag, and `stopPropagation()` (what the comboboxes call) does not stop other listeners on the same target - only `stopImmediatePropagation()` does. So the inline handler runs first and calls `requestSubmit()` with the partial text (e.g. `@ali`) before the combobox inserts the resolved option, which `hx-on::after-request` then wipes. The double-fire is real per spec; the visible symptom is a broken half-typed send. ## Fix The inline handler short-circuits before submitting when an `aria-selected` option exists in either popover (`#lc-mention-popover` / `#lc-slash-popover`): ```js if (document.querySelector('#lc-mention-popover li[role=option][aria-selected=true], #lc-slash-popover li[role=option][aria-selected=true]')) return; ``` Properties: - Ordering-independent: it does not rely on which listener runs first, so it is correct whatever a given browser does. - Shared path: it lives in the one inline handler both comboboxes share, so it covers mention and slash identically with no divergence (the issue explicitly warned against fixing slash alone). - No stray newline: the combobox keydown handler still `preventDefault`s the Enter. ## Test A regression test in `routes_drafts.rs` renders the composer and asserts both that the guard selector is present and that it precedes `requestSubmit()` in the markup, since the ordering of the short-circuit is what makes it correct. ## Verification `just test` / `just test-saas`: all binaries green except the known `routes_uploads` concurrent-load flake (documented in CLAUDE.md), which passes 12/12 in isolation in both modes. This change touches only `room/composer.html` and the drafts test - nothing upload-related. `cargo clippy` / `cargo fmt` clean.

nrupard added 1 commit 2026-05-26 17:34:51 +02:00

fix(composer): Enter defers to an open mention/slash combobox (LC-171) ... 1ca080611c

Raised in the LC-168 review. The composer textarea's inline onkeydown submits the form on Enter. The mention and slash comboboxes add capture-phase keydown handlers on the SAME textarea that preventDefault + stopPropagation + insert the highlighted option on Enter. Because all three listeners are on the same target, they fire in registration order regardless of the capture flag, and the inline handler is registered first (at parse time, before the post-parse addEventListener calls). stopPropagation does not stop other listeners on the same target (only stopImmediatePropagation does), so on Enter with a popover open the inline handler runs first and calls requestSubmit() with the partial text (e.g. "@ali") before the combobox inserts the resolved option, which after-request then wipes. No browser in the dev env to reproduce, but the ordering is decidable by spec and the failure mode is a broken half-typed send.

Fix: the inline handler short-circuits before submitting when an aria-selected option exists in either popover (#lc-mention-popover / #lc-slash-popover). This is ordering-independent (it does not rely on which listener runs first) and lives in the shared inline handler, so it covers both comboboxes identically with no slash/mention divergence. The combobox keydown handler still preventDefaults the Enter, so no stray newline is inserted.

Regression test pins both the guard's presence and that it precedes requestSubmit in the rendered composer, since order is what makes it correct.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard added 1 commit 2026-05-26 17:39:58 +02:00

test(composer): make the LC-171 Enter-guard assertion reformat-tolerant ... 786b6e605a

Review nit: the test matched the full guard selector literal, so reordering the two popover clauses or reflowing whitespace inside the selector would fail the test even though the guard is functionally intact. Assert each popover's selected-option anchor independently and pin the short-circuit-before-requestSubmit ordering, which is the invariant that actually prevents the double-fire.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard merged commit cd1b01df3b into main 2026-05-26 17:44:39 +02:00

nrupard deleted branch fix/lc-171-composer-enter-guard 2026-05-26 17:44:39 +02:00

nrupard referenced this pull request from a commit 2026-05-26 17:44:39 +02:00

Merge pull request 'fix(composer): Enter defers to an open mention/slash combobox (LC-171)' (#216) from fix/lc-171-composer-enter-guard into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

psa-systems/lets-chat!216

No description provided.