From 25c43140bc3ff6c5b39fd3cb53d79b995ee48b75 Mon Sep 17 00:00:00 2001 From: John McLear Date: Sun, 3 May 2026 12:40:44 +0800 Subject: [PATCH] feat(userlist): click a user to open chat with @ prefilled (#7660) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(userlist): click a user to open chat with @ prefilled 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 "@ ", 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. * fix(userlist): don't steal rename focus + add Playwright coverage Two follow-ups on review of the click-to-chat handler: 1. Bug (Qodo, correctness): clicking the rename 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 "@ " - clicking the swatch opens color picker, not chat - clicking the rename on an unnamed user keeps focus on the input (regression test for the bug above) - partial chat message is preserved when prefilling * test: stabilise the partial-message preservation case 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. * fix: don't commit local .claude worktrees / var state These were accidentally added in ffe947706 by an over-broad git add -A. Both paths are workspace-local and unrelated to this PR. --- doc/api/hooks_client-side.md | 28 +++ src/static/js/pad_userlist.ts | 46 +++++ .../skins/colibris/src/components/users.css | 13 ++ .../specs/userlist_click_to_chat.spec.ts | 175 ++++++++++++++++++ 4 files changed, 262 insertions(+) create mode 100644 src/tests/frontend-new/specs/userlist_click_to_chat.spec.ts diff --git a/doc/api/hooks_client-side.md b/doc/api/hooks_client-side.md index 4b0f1cce6..d6b36681c 100644 --- a/doc/api/hooks_client-side.md +++ b/doc/api/hooks_client-side.md @@ -354,6 +354,34 @@ Context properties: * `message`: The message object that will be sent to the Etherpad server. +## `chatPrefillFromUser` + +Called from: `src/static/js/pad_userlist.ts` + +Called when the user clicks an entry in the user list. The default behavior is to +open the chat panel and prefill the input with `@ `, where `` is that +user's display name (with whitespace replaced by underscores). Plugins can return +a different prefill string from their callback — the first non-empty string +returned wins. + +Typical use is by AI/bot plugins whose author display name (e.g. "AI Assistant") +isn't a useful @-mention; the plugin can substitute its trigger string instead. + +Context properties: + +* `authorId`: The clicked user's author id. +* `name`: The clicked user's display name. +* `prefill`: The default prefill string Etherpad would otherwise use. + +Example: + +```javascript +exports.chatPrefillFromUser = (hookName, {authorId, name}, cb) => { + if (authorId === window.clientVars.ep_my_bot.authorId) return cb('@bot '); + return cb(); +}; +``` + ## collectContentPre Called from: `src/static/js/contentcollector.js` diff --git a/src/static/js/pad_userlist.ts b/src/static/js/pad_userlist.ts index 85bb32a98..43adc4a38 100644 --- a/src/static/js/pad_userlist.ts +++ b/src/static/js/pad_userlist.ts @@ -19,6 +19,7 @@ import padutils from './pad_utils' const hooks = require('./pluginfw/hooks'); +const chat = require('./chat').chat; import html10n from './vendors/html10n'; let myUserInfo = {}; @@ -369,6 +370,51 @@ const paduserlist = (() => { }, 0); }); + // Click any other user's row to open chat with @ prefilled. + // Helps newcomers discover the chat panel and the @-mention convention + // without having to be told. Plugins can transform the prefilled text + // — for example ep_ai_chat replaces "@AI Assistant" with the + // configured trigger ("@ai") — via the chatPrefillFromUser client + // hook (see doc/api/hooks_client-side.md). + $('#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; + // Skip clicks on form controls inside the row. The most important + // case is the rename rendered for unnamed users — without + // this guard, clicking the input would steal focus into #chatinput + // and make it impossible to name an unnamed user from the list. + if ($(event.target).closest('input, textarea, select, button, a, [contenteditable=true]').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); + }); + // color picker $('#myswatchbox').on('click', showColorPicker); $('#mycolorpicker .pickerswatchouter').on('click', function () { diff --git a/src/static/skins/colibris/src/components/users.css b/src/static/skins/colibris/src/components/users.css index c04453c57..c15f984e6 100644 --- a/src/static/skins/colibris/src/components/users.css +++ b/src/static/skins/colibris/src/components/users.css @@ -2,6 +2,19 @@ table#otheruserstable { margin-top: 20px; } +/* + * Clicking a row in the user list opens chat and prefills "@ ". + * The pointer cursor on the name cell makes the affordance visible — + * the swatch keeps its own (color-picker) click semantics, so leave + * its cursor alone. + */ +#otheruserstable tr[data-authorId] .usertdname { + cursor: pointer; +} +#otheruserstable tr[data-authorId]:hover .usertdname { + text-decoration: underline; +} + .popup#users.chatAndUsers > .popup-content { padding: 20px 10px; height: 250px; diff --git a/src/tests/frontend-new/specs/userlist_click_to_chat.spec.ts b/src/tests/frontend-new/specs/userlist_click_to_chat.spec.ts new file mode 100644 index 000000000..7edeadd02 --- /dev/null +++ b/src/tests/frontend-new/specs/userlist_click_to_chat.spec.ts @@ -0,0 +1,175 @@ +import {expect, test} from '@playwright/test'; +import { + goToNewPad, + goToPad, + isChatBoxShown, + setUserName, + toggleUserList, +} from '../helper/padHelper'; + +/** + * Coverage for the click-a-user-to-prefill-@-mention UX added in #7660. + * + * Why a multi-context suite: the row click handler only runs against + * #otheruserstable rows, so we always need a second client connected to + * the same pad to populate that table. Each test opens the pad twice + * with a fresh context, names the second user, then drives the click + * from the first. + */ + +const setSecondUserName = async (page2: any, name: string) => { + await toggleUserList(page2); + await setUserName(page2, name); + await page2.keyboard.press('Enter'); +}; + +test.describe('userlist click → chat prefill', {tag: '@feature:chat'}, () => { + test('clicking another user opens chat and prefills @', + async ({browser}) => { + const padId = await goToNewPad(await browser.newPage()); + // Hack: the line above used a throwaway page just to mint a padId. + // Real users come below. + + const ctx1 = await browser.newContext(); + const page1 = await ctx1.newPage(); + await goToPad(page1, padId); + + const ctx2 = await browser.newContext(); + const page2 = await ctx2.newPage(); + await goToPad(page2, padId); + + await setSecondUserName(page2, 'Alice'); + + // Wait for page1's user list to learn about Alice. + await toggleUserList(page1); + const aliceRow = page1.locator( + '#otheruserstable tr[data-authorId] .usertdname:has-text("Alice")'); + await expect(aliceRow).toBeVisible({timeout: 10_000}); + + // Sanity: chat is hidden before the click. + expect(await isChatBoxShown(page1)).toBe(false); + + await aliceRow.click(); + + // Chat should be open, input prefilled. + await page1.waitForFunction( + "document.querySelector('#chatbox')?.classList.contains('visible')", + null, {timeout: 5_000}); + await page1.waitForFunction( + "document.querySelector('#chatinput')?.value?.startsWith('@Alice ')", + null, {timeout: 5_000}); + + await ctx1.close(); + await ctx2.close(); + }); + + test('clicking the swatch opens the color picker, not chat', + async ({browser}) => { + const padId = await goToNewPad(await browser.newPage()); + + const ctx1 = await browser.newContext(); + const page1 = await ctx1.newPage(); + await goToPad(page1, padId); + + const ctx2 = await browser.newContext(); + const page2 = await ctx2.newPage(); + await goToPad(page2, padId); + await setSecondUserName(page2, 'Bob'); + + await toggleUserList(page1); + const bobRow = page1.locator( + '#otheruserstable tr[data-authorId] .usertdname:has-text("Bob")'); + await expect(bobRow).toBeVisible({timeout: 10_000}); + + const swatch = page1.locator( + '#otheruserstable tr[data-authorId] .usertdswatch').first(); + await swatch.click(); + + // Chat should NOT be opened by a swatch click. + // (We only check the box-state; we don't assert anything about + // any color-picker popup since this PR doesn't change that flow.) + await page1.waitForTimeout(300); + expect(await isChatBoxShown(page1)).toBe(false); + + await ctx1.close(); + await ctx2.close(); + }); + + test('clicking the rename input on an unnamed user does not steal focus', + async ({browser}) => { + const padId = await goToNewPad(await browser.newPage()); + + const ctx1 = await browser.newContext(); + const page1 = await ctx1.newPage(); + await goToPad(page1, padId); + + // Second user joins but never sets a name → row renders an + // . + const ctx2 = await browser.newContext(); + const page2 = await ctx2.newPage(); + await goToPad(page2, padId); + + await toggleUserList(page1); + const unnamedInput = page1.locator( + '#otheruserstable input[data-l10n-id="pad.userlist.unnamed"]') + .first(); + await expect(unnamedInput).toBeVisible({timeout: 10_000}); + + // The act of clicking the input must NOT trigger the row handler. + // Pre-fix, this opened chat and stole focus from the rename input. + await unnamedInput.click(); + await page1.waitForTimeout(300); + + expect(await isChatBoxShown(page1)).toBe(false); + // Focus is still on the unnamed-user input — typing reaches it, + // not #chatinput. + await page1.keyboard.type('Carol'); + const value = await unnamedInput.inputValue(); + expect(value).toBe('Carol'); + + await ctx1.close(); + await ctx2.close(); + }); + + test('partial message in chat input is preserved when prefilling', + async ({browser}) => { + const padId = await goToNewPad(await browser.newPage()); + + const ctx1 = await browser.newContext(); + const page1 = await ctx1.newPage(); + await goToPad(page1, padId); + + const ctx2 = await browser.newContext(); + const page2 = await ctx2.newPage(); + await goToPad(page2, padId); + await setSecondUserName(page2, 'Dave'); + + // Open chat and seed a partial message *before* opening the user + // list. fill() is deterministic — it sets the value without + // racing the chaticon click handler's own focus timer that + // earlier versions of this test were tripping over. + await page1.locator('#chaticon').click(); + await page1.waitForFunction( + "document.querySelector('#chatbox')?.classList.contains('visible')", + null, {timeout: 5_000}); + await page1.locator('#chatinput').fill('hi there'); + + await toggleUserList(page1); + const daveRow = page1.locator( + '#otheruserstable tr[data-authorId] .usertdname:has-text("Dave")'); + await expect(daveRow).toBeVisible({timeout: 10_000}); + await daveRow.click(); + + // Wait for both pieces to land in the input — the prefill fires + // from a 50ms setTimeout in the click handler, so a single wait + // covering the full final state is more reliable than two + // sequential checks. + await page1.waitForFunction(() => { + const v = (document.querySelector('#chatinput') as HTMLTextAreaElement)?.value || ''; + return v.includes('hi there') && v.includes('@Dave'); + }, null, {timeout: 5_000}); + + await ctx1.close(); + await ctx2.close(); + }); +});