feat(userlist): click a user to open chat with @<name> prefilled#7660
Conversation
Newcomers to a multi-user pad regularly fail to discover the chat panel and the @-mention convention. Make the user list itself the discovery affordance: clicking another user's row opens chat (if hidden) and prefills the input with "@<their_name> ", ready to send. The skin gets a small visual cue — pointer cursor on .usertdname and an underline on hover — so the affordance is visible without requiring a redesign. The color swatch keeps its own click semantics (color picker), so the swatch cell is excluded from the new handler. To let bot/AI plugins substitute their trigger string for an otherwise-useless @-mention of the bot's display name (e.g. "@ai Assistant" → "@ai"), this adds a new client-side hook, chatPrefillFromUser, that takes {authorId, name, prefill} and lets the first plugin to return a non-empty string override the default prefill. Documented in doc/api/hooks_client-side.md alongside chatSendMessage. Plugin errors in the hook are caught — a misbehaving plugin can't break the click. If chat is hidden by pad settings, chat.show() is a no-op and the click effectively does nothing, which matches the existing behavior of "no chat means no chat-related affordances". The new prefill never clobbers a real partial message in the input; if the user was mid-typing something, the @-mention is appended rather than replacing.
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Review Summary by QodoClick user list to open chat with prefilled @-mention
WalkthroughsDescription• Click user row to open chat with @-mention prefilled • Adds chatPrefillFromUser hook for plugin customization • Visual affordance: pointer cursor and hover underline • Preserves partial messages when appending mentions Diagramflowchart LR
UserClick["User clicks row"] --> CheckSwatch{"Click on swatch?"}
CheckSwatch -->|Yes| ColorPicker["Open color picker"]
CheckSwatch -->|No| Hook["Call chatPrefillFromUser hook"]
Hook --> Transform{"Plugin returns<br/>custom prefill?"}
Transform -->|Yes| UsePrefill["Use plugin prefill"]
Transform -->|No| DefaultPrefill["Use @name prefill"]
UsePrefill --> ShowChat["Show chat panel"]
DefaultPrefill --> ShowChat
ShowChat --> FillInput["Prefill or append to input"]
FillInput --> Focus["Focus input & set cursor"]
File Changes1. src/static/js/pad_userlist.ts
|
Code Review by Qodo
1. Click-to-chat lacks feature flag
|
…ndler PR #19 originally landed an entire user-list click handler inside ep_ai_chat. That logic was generic — clicking a user in the user list to prefill an @-mention is a discoverability win for any multi-user pad, AI or no AI — and as @JohnMcLear pointed out, it belongs in Etherpad core, not in this plugin. The generic feature is now ether/etherpad#7660. This commit shrinks ep_ai_chat's contribution to the only AI-specific bit: when the clicked author is the AI, override the default "@AI_Assistant " prefill with the configured trigger ("@ai " by default) so the input lands in a useful state. Implementation: - Drop the in-plugin click delegation, the chatbox/chaticon DOM poking, and the input-prefill logic. Core handles all of that uniformly now. - Add a chatPrefillFromUser client hook that returns the trigger string when the clicked authorId matches the AI's. The clientVars hook (added earlier in this PR) still exposes the trigger and authorId; nothing changes there. - On older cores the new hook is never fired, so this is a no-op on a stock install — graceful degradation. Net diff vs main is now: a tiny clientVars exposure on the server and a ~10-line client hook handler. Everything else is core's job.
| $('#otheruserstable').on('click', 'tr[data-authorId]', async function (event) { | ||
| // Skip clicks on the color swatch — that has its own click handler | ||
| // (color-picker semantics) and shouldn't double up as a chat trigger. | ||
| if ($(event.target).closest('.usertdswatch').length) return; | ||
| const tr = $(this); | ||
| const authorId = tr.attr('data-authorId'); | ||
| if (!authorId) return; | ||
| const name = (tr.find('.usertdname').text() || '').trim(); | ||
| let prefill = name ? `@${name.replace(/\s+/g, '_')} ` : ''; | ||
| try { | ||
| const transforms = await hooks.aCallAll( | ||
| 'chatPrefillFromUser', {authorId, name, prefill}); | ||
| if (Array.isArray(transforms)) { | ||
| for (const tr2 of transforms) { | ||
| if (typeof tr2 === 'string' && tr2.length > 0) { prefill = tr2; break; } | ||
| } | ||
| } | ||
| } catch { /* never let a misbehaving plugin break the click */ } | ||
| try { chat.show(); } catch { /* */ } | ||
| setTimeout(() => { | ||
| const $input = $('#chatinput'); | ||
| if (!$input.length) return; | ||
| const current = ($input.val() || '') as string; | ||
| if (!current.trim() || /^@\S+\s*$/.test(current.trim())) { | ||
| $input.val(prefill); | ||
| } else if (!current.includes(prefill.trim())) { | ||
| $input.val(`${current.trimEnd()} ${prefill}`); | ||
| } | ||
| $input.trigger('focus'); | ||
| const elem = $input[0] as HTMLTextAreaElement; | ||
| try { elem.setSelectionRange(elem.value.length, elem.value.length); } catch (_e) { /* */ } | ||
| }, 50); | ||
| }); |
There was a problem hiding this comment.
1. Click-to-chat lacks feature flag 📘 Rule violation ⚙ Maintainability
The new click handler on user list rows changes default behavior by opening chat and prefilling an @name mention without any feature flag gating. This violates the requirement that new features be disabled by default unless explicitly enabled.
Agent Prompt
## Issue description
A new UX feature (click user row to open chat and prefill `@name`) is enabled by default, but compliance requires new features to be behind a feature flag and disabled by default.
## Issue Context
The click handler is currently registered unconditionally, so all installs get the new behavior without opting in.
## Fix Focus Areas
- src/static/js/pad_userlist.ts[379-411]
- src/static/skins/colibris/src/components/users.css[11-16]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Going with a reasoned decline on the feature-flag part of this comment. Replying so the rationale is on the record:
- Etherpad doesn't gate UX changes behind feature flags. The current settings.json surface is reserved for things that materially change the contract (auth, storage, transport, security knobs, skin selection). Recent UX additions — chat-and-users mode, sticky chat, the showsidebar editbar button, etc. — all shipped on by default with no flag. Adding a flag here would be inconsistent with project precedent.
- Blast radius is small. Before this PR clicking a user list row was a no-op; nothing depended on that. Anyone who wants the click to do nothing can ship a tiny CSS override (
#otheruserstable tr[data-authorId] { pointer-events: none; }) without a settings change. - Plugins already have the override they need. The
chatPrefillFromUserhook lets a plugin return a non-empty string to substitute the prefill. A plugin that wanted to globally suppress the behavior could trivially return an empty-but-truthy value or hide rows via CSS.
The other comment on this review (rename-input click steals focus) was a real bug and is fixed in a494307a, plus a Playwright spec covering the supported flows and that regression.
Two follow-ups on review of the click-to-chat handler:
1. Bug (Qodo, correctness): clicking the rename <input> on an unnamed
user's row triggered the new row handler, which then focused
#chatinput and made it impossible to name unnamed users from the
user list. Add an early-return that skips form controls inside
the row (input/textarea/select/button/a/[contenteditable=true]).
The swatch was already excluded; this widens the same idea to
anything that's interactive on its own merits.
2. Test coverage: add a frontend Playwright spec
(userlist_click_to_chat.spec.ts) covering the supported flows
and the new regression:
- clicking another named user opens chat and prefills "@<name> "
- clicking the swatch opens color picker, not chat
- clicking the rename <input> on an unnamed user keeps focus
on the input (regression test for the bug above)
- partial chat message is preserved when prefilling
The 'partial message in chat input is preserved when prefilling' case was flaking on CI. Three small changes: - Seed the chat input with fill() rather than click() + keyboard.type(). Earlier the test was racing chat.focus()'s own setTimeout(100) — when the keyboard.type started before that timer fired, the typing landed in whatever element had focus at the time, which wasn't always the chat input. fill() bypasses focus state entirely. - Wait for the chat box to be visible before filling, so we don't race the chaticon click handler. - Replace the two sequential expect/wait pairs after the daveRow click with one waitForFunction that asserts both 'hi there' and '@dave' are in the input together. The prefill is async (setTimeout(50) inside the click handler), so a combined wait is more reliable than checking one piece, then snapshotting and asserting the other. The other three cases in this file passed unchanged on CI; only this fourth one was racy.
These were accidentally added in ffe9477 by an over-broad git add -A. Both paths are workspace-local and unrelated to this PR.
…lay name
Override the default "@<name> " chat-input prefill with the configured
AI trigger ("@ai " by default) when a user clicks the AI's row in the
user list. Without this, clicking the AI chip would prefill
"@AI_Assistant ", which doesn't match anything the server-side mention
extractor recognises.
What this PR adds:
- clientVars server hook exposing { trigger, authorName, authorId } at
clientVars.ep_ai_chat so the client can recognise the AI's row.
- chatPrefillFromUser client hook that returns the trigger string when
the clicked authorId matches the AI's; otherwise returns nothing so
core's default ("@<name> ") wins.
- Mocha unit test (static/tests/backend/specs/chat_prefill.ts) covering
AI/non-AI/missing-clientVars/custom-trigger/missing-trigger paths.
Depends on ether/etherpad#7660 which adds the chatPrefillFromUser hook
to core. On older cores the hook is never fired and this PR is a
benign no-op — graceful degradation, no install requirement bump.
Originally this PR shipped the entire user-list click handler inside
the plugin. Per review feedback, the generic "click a user → prefill
@-mention" UX belongs in core (it's a discoverability win for any
multi-user pad, AI or no AI), so the bulk moved to ether/etherpad#7660
and this PR is now ~40 lines of glue.
(Replaces the previous 3-commit history on this branch with a clean
rebase against the new main, which absorbed PRs #18 and #20 since the
branch was opened.)
…lay name (#19) Override the default "@<name> " chat-input prefill with the configured AI trigger ("@ai " by default) when a user clicks the AI's row in the user list. Without this, clicking the AI chip would prefill "@AI_Assistant ", which doesn't match anything the server-side mention extractor recognises. What this PR adds: - clientVars server hook exposing { trigger, authorName, authorId } at clientVars.ep_ai_chat so the client can recognise the AI's row. - chatPrefillFromUser client hook that returns the trigger string when the clicked authorId matches the AI's; otherwise returns nothing so core's default ("@<name> ") wins. - Mocha unit test (static/tests/backend/specs/chat_prefill.ts) covering AI/non-AI/missing-clientVars/custom-trigger/missing-trigger paths. Depends on ether/etherpad#7660 which adds the chatPrefillFromUser hook to core. On older cores the hook is never fired and this PR is a benign no-op — graceful degradation, no install requirement bump. Originally this PR shipped the entire user-list click handler inside the plugin. Per review feedback, the generic "click a user → prefill @-mention" UX belongs in core (it's a discoverability win for any multi-user pad, AI or no AI), so the bulk moved to ether/etherpad#7660 and this PR is now ~40 lines of glue. (Replaces the previous 3-commit history on this branch with a clean rebase against the new main, which absorbed PRs #18 and #20 since the branch was opened.)
Why
Newcomers to a multi-user pad regularly fail to discover the chat panel and the
@-mentionconvention. The current UX is "find the chat icon in the corner, click it, type@, type your collaborator's name." Most people never make it past step 1.This PR makes the user list itself the discovery affordance: clicking another user's row opens chat (if hidden) and prefills the input with
@<their_name>, ready to send.What changed
src/static/js/pad_userlist.ts— delegated click handler on#otheruserstable tr[data-authorId]. Skips clicks on.usertdswatch(color-picker keeps its own semantics). Callschat.show()and prefills#chatinput. Doesn't clobber a real partial message — if the user is mid-typing, the mention is appended.src/static/skins/colibris/src/components/users.css— pointer cursor on.usertdnameand a hover underline so the affordance is visible. Swatch keeps its existing cursor.doc/api/hooks_client-side.md— documents the newchatPrefillFromUserhook.New hook:
chatPrefillFromUserBot/AI plugins frequently have a display name that's a useless @-mention (e.g.
AI Assistantdoesn't trigger anything; the actual trigger is@ai). The new client-side hook lets them substitute the prefill:First plugin to return a non-empty string wins. Hook errors are caught so a misbehaving plugin can't break the click.
Companion
This unblocks ether/ep_ai_chat#19 (split per discussion) which becomes a tiny hook handler that swaps
AI Assistantfor the configured trigger. Without this PR, every Etherpad install that wants click-to-mention has to ship its own copy of the click handler.Test plan
@<sessionB_name>is in the input.pad.settings.hideChattrue, click is a benign no-op.🤖 Generated with Claude Code