fix(collaboration): 给 CommentThread 的 "+" 表情选择器补上可访问名,而不只是 tooltip (#3478)

.changeset/collaboration-reaction-picker-accessible-name-3478.md

@@ -0,0 +1,52 @@
+---
+'@object-ui/collaboration': patch
+---
+
+Give `CommentThread`'s `+` reaction picker a real accessible name (objectui#3478)
+
+The fourth glyph-only control in the same component, and the one objectui#3441
+walked past. Its content is the literal `'+'`, so the `title` objectstack#5506
+gave it (`collaboration.addThumbsUp`) could never become its accessible name: a
+`button`'s name is computed from CONTENT (accname §2F) before the `title`
+tooltip is consulted at all (§2I). A screen reader announced "plus button". The
+copy existed, was localized into all ten packs, and reached only the people who
+could already see the button.
+
+That is a different failure from #3441's three buttons, which is why it survived
+that fix — those had no authored copy anywhere, so every "is the key wired up?"
+check found the gap. Here the key WAS wired up and the English WAS in
+`COLLAB_DEFAULT_TRANSLATIONS`; only its DESTINATION was wrong. #3441's own pin
+test recorded the defect without naming it, asserting `getByTitle('Add thumbs
+up')` and `queryAllByRole('button', { name: 'Add thumbs up' })).toHaveLength(0)`
+in the same green case — two assertions that together say "the title is set and
+it is not the name". The docblock read them as pinning the picker apart from the
+quick 👍; they were also, unread, the bug report.
+
+The fix adds `aria-label` (accname §2C, which outranks content) alongside the
+existing `title`, on the same key. Zero new keys — the copy was always there.
+
+The `title` is KEPT rather than replaced, and this is the one control in the
+component where carrying both is right instead of redundant: `+` says nothing to
+a sighted mouse user either, so the hover hint is doing real work of its own.
+(The 👍/❤️ buttons #3441 fixed had no `title` to keep.) Both attributes read the
+same key, so the tooltip and the name cannot drift apart.
+
+The name stays `addThumbsUp` — it describes what the button does today
+(`onReaction(id, '👍')`, unconditionally), not what `styles.reactionPicker`
+hints it might become. Turning it into an actual emoji picker is a feature
+change, and the copy follows the behaviour when that lands. It also stays
+distinct from #3441's `reactThumbsUp`: on a comment that already has reactions
+both controls are on screen at once, and now that both carry a real accessible
+name, sharing one key would be worse than when #3441 declined to — two visibly
+different buttons announcing themselves identically.
+
+The adjacent reaction chips are deliberately untouched. Their content is
+`${emoji} ${count}` — already a descriptive name — so name-from-content is the
+right answer there, and their `title` adds the count in words.
+
+Tests assert the computed accessible name via `getByRole('button', { name })`
+rather than the presence of an attribute, in English, Chinese and with no
+`I18nProvider` mounted; the mirror assertion (`{ name: '+' }` finds nothing) is
+what fails if the `aria-label` is ever dropped. #3441's pin was rewritten in
+place to pin the new both-named state instead of the old separation, since the
+statement it used to make is exactly the one this change falsifies.

packages/collaboration/src/CommentThread.tsx

@@ -591,9 +591,28 @@ export function CommentThread({
               ),
             }, `${emoji} ${userIds.length}`),
           ),
+          // The reaction-bar picker (objectui#3478). Its content is the literal
+          // `'+'`, so the `title` objectstack#5506 gave it could never become
+          // its accessible name — name-from-content (accname §2F) is resolved
+          // before the `title` tooltip (§2I) gets a turn, and a screen reader
+          // announced "plus button". The copy was localized and correct and
+          // reached nobody who could not see the glyph. `aria-label` (§2C)
+          // outranks content, so it is what actually names the control.
+          //
+          // The `title` STAYS: `+` tells a sighted mouse user nothing either,
+          // so the hover hint does real work of its own. Both attributes read
+          // the SAME key, so name and tooltip cannot drift apart. (The 👍/❤️
+          // buttons below had no `title` to keep — hence `aria-label` alone.)
+          //
+          // Still `addThumbsUp`: the button dispatches `onReaction(id, '👍')`
+          // unconditionally today, and the name describes that, not what
+          // `styles.reactionPicker` hints it might one day become. The chips
+          // above stay content-named — `${emoji} ${count}` already describes
+          // them.
           onReaction && React.createElement('button', {
             style: styles.reactionPicker,
             onClick: () => onReaction(comment.id, '👍'),
+            'aria-label': t('collaboration.addThumbsUp'),
             title: t('collaboration.addThumbsUp'),
           }, '+'),
         ),
@@ -608,15 +627,18 @@ export function CommentThread({
           // ever reaches `title` (accname §2F outranks §2I) — so these two were
           // announced as the bare glyph: "thumbs up" / "red heart" at best,
           // nothing at all where the SR has no name for the codepoint. Hence
-          // `aria-label`, which overrides content, rather than the `title` the
-          // `+` picker above uses.
+          // `aria-label`, which overrides content. (The `+` picker above had
+          // only a `title` and was announced as "plus" for exactly the same
+          // reason, until objectui#3478 gave it an `aria-label` too.)
           //
           // Their own key pair, NOT a reuse of `collaboration.addThumbsUp`: the
           // `+` above happens to fire the same `onReaction(id, '👍')` today, but
           // it is the reaction PICKER's entry point (`styles.reactionPicker`)
           // whose copy follows the picker if it ever picks. Sharing one key
-          // would also give a comment that already has reactions two visible
-          // controls answering to one name.
+          // would now be worse than when #3441 wrote this down — since #3478
+          // both controls carry a real accessible name, so on a comment that
+          // already has reactions a shared key means two visibly different
+          // controls announcing themselves identically.
           onReaction && React.createElement('button', {
             style: styles.actionBtn,
             onClick: () => onReaction(comment.id, '👍'),

packages/collaboration/src/__tests__/comment-thread-i18n.test.tsx

@@ -293,25 +293,33 @@ describe('CommentThread per-comment actions (objectstack#5506)', () => {
 });
 
 /**
- * objectui#3441 — the three emoji-only controls objectstack#5506 left unnamed.
+ * objectui#3441 / #3478 — the four glyph-only controls objectstack#5506 left
+ * unnamed: the quick 👍, the quick ❤️, the reply-banner ✕ (all #3441) and the
+ * reaction-bar `+` picker (#3478).
  *
  * These assert the computed ACCESSIBLE NAME (`getByRole('button', { name })`,
  * which runs dom-accessibility-api's accname implementation), not the presence
  * of an attribute. That distinction is the whole point of the fix: for a
  * `button`, name-from-content (accname §2F) is consulted BEFORE the `title`
- * tooltip (§2I), so hanging a `title` on `'👍'` the way the `+` picker does
- * would have left the computed name as the glyph. `aria-label` is the only one
- * of the three that outranks content.
+ * tooltip (§2I), so a `title` hung on `'👍'` — or on `'+'`, which is what the
+ * picker had until #3478 — leaves the computed name as the glyph. Of the three
+ * things that can name a `button` here, `aria-label` is the only one that
+ * outranks content.
  *
  * ── Direction ─────────────────────────────────────────────────────────────
  * RED before / GREEN after in EVERY language, `en` included — unlike the
  * copy-pin cases above, these names did not exist in any locale on
  * `origin/main`, so there is no "English was already right" half here. The
- * `queryAllByRole(… { name: '👍' })` assertions are the mirror image: they
- * pass ONLY after the fix, because the glyph was the name until `aria-label`
- * displaced it.
+ * `queryAllByRole(… { name: '👍' })` / `{ name: '+' }` assertions are the
+ * mirror image: they pass ONLY after the fix, because the glyph was the name
+ * until `aria-label` displaced it.
+ *
+ * The one deliberate exception is the picker's `getByTitle` assertion, green on
+ * both sides: `+` keeps its tooltip as well as gaining a name. It is the only
+ * control here that legitimately carries both — a sighted mouse user gets the
+ * hover hint the glyph cannot give them either.
  */
-describe('CommentThread emoji-only control names (objectui#3441)', () => {
+describe('CommentThread glyph-only control names (objectui#3441, #3478)', () => {
   it('names the two quick-reaction buttons in English', () => {
     renderThread('en');
 
@@ -371,19 +379,57 @@ describe('CommentThread emoji-only control names (objectui#3441)', () => {
   });
 
   /**
-   * The `+` picker keeps `collaboration.addThumbsUp` (objectstack#5506) and the
-   * quick 👍 gets its own `reactThumbsUp`, even though both dispatch the same
-   * `onReaction(id, '👍')` today. This case is what pins the two apart: on a
-   * comment that already has reactions both controls are on screen at once, so
-   * sharing one key would put two visibly different controls under one name.
+   * objectui#3478 — the `+` reaction picker, the fourth glyph-only control
+   * (`+` is not an emoji, but accname does not care), and the one objectui#3441
+   * left behind.
+   *
+   * Its content is the literal `'+'`, so accname §2F named it "plus" and the
+   * `title` at §2I never got a turn — `collaboration.addThumbsUp` reached the
+   * tooltip and nothing else. #3441's own pin is what recorded this: it
+   * asserted `getByTitle('Add thumbs up')` AND
+   * `queryAllByRole('button', { name: 'Add thumbs up' })).toHaveLength(0)` in
+   * the same case, two simultaneously-green assertions that say in so many
+   * words "the title is set and it is not the name". The docblock read them as
+   * pinning the picker APART from the quick 👍; they were also, unread, the
+   * bug report. This case now pins the fixed state: `aria-label` names it,
+   * `title` still shows it, and the two keys stay distinct.
+   *
+   * ── Direction ─────────────────────────────────────────────────────────────
+   * RED before / GREEN after, twice over: the `name: 'Add thumbs up'` lookup
+   * finds 0 buttons on `origin/main` (it was `toHaveLength(0)` there), and the
+   * `name: '+'` lookup finds 1 — the glyph IS the name until `aria-label`
+   * displaces it. The `getByTitle` and `React with thumbs up` assertions are
+   * green on both sides on purpose: the tooltip must survive the fix (the
+   * picker is the one control here that legitimately has both), and the two
+   * controls must stay under two names.
    */
-  it('keeps the reaction-bar picker distinct from the quick thumbs-up', () => {
+  it('names the reaction-bar picker and keeps its tooltip', () => {
     renderThread('en');
 
+    // Tooltip survives — green on both sides.
     expect(screen.getByTitle('Add thumbs up')).toBeTruthy();
-    expect(screen.queryAllByRole('button', { name: 'Add thumbs up' })).toHaveLength(0);
+    // …and is now ALSO the accessible name. Exactly one picker: it renders
+    // only on a comment that already has reactions, i.e. `c2`.
+    expect(screen.getAllByRole('button', { name: 'Add thumbs up' }).length).toBe(1);
+    // The bug, mirrored: `'+'` was the computed name on origin/main.
+    expect(screen.queryAllByRole('button', { name: '+' })).toHaveLength(0);
+    // Still two names for two controls — #3441's separation is not undone.
     expect(screen.getAllByRole('button', { name: 'React with thumbs up' }).length).toBe(2);
   });
+
+  /**
+   * The same control in the session language. `addThumbsUp` is already in every
+   * locale pack (objectstack#5506 shipped it for the tooltip), so this needed
+   * no new key — only a call site that lets a screen reader reach it.
+   */
+  it('names the reaction-bar picker in the session language', () => {
+    renderThread('zh');
+
+    expect(screen.getByRole('button', { name: '点赞' })).toBeTruthy();
+    expect(screen.getByTitle('点赞')).toBeTruthy();
+    expect(screen.queryAllByRole('button', { name: '+' })).toHaveLength(0);
+    expect(screen.queryAllByRole('button', { name: 'Add thumbs up' })).toHaveLength(0);
+  });
 });
 
 /**

packages/collaboration/src/__tests__/comment-thread-no-provider-fallback.test.tsx

@@ -183,6 +183,27 @@ describe('CommentThread with no I18nProvider — English fallback (objectstack#5
     expect(screen.queryAllByRole('button', { name: '✕' })).toHaveLength(0);
   });
 
+  /**
+   * objectui#3478 — the fourth glyph-only control, the reaction-bar `+`.
+   *
+   * It already had `title: t('collaboration.addThumbsUp')`, which is why the
+   * defect survived #3441: the key WAS wired up and the English WAS in the
+   * defaults map, so every copy pin in this file passed. What no assertion
+   * covered until now is that `title` on a `button` never becomes the name —
+   * content (accname §2F) wins, and the content is `'+'`.
+   *
+   * RED before / GREEN after on the two `role`-based assertions; the
+   * `getByTitle` above (in the reaction-tooltip case) is green on both sides
+   * and stays there, because the tooltip is kept, not replaced.
+   */
+  it('names the reaction-bar picker in English, tooltip and all', () => {
+    renderBare();
+
+    expect(screen.getAllByRole('button', { name: 'Add thumbs up' }).length).toBe(1);
+    expect(screen.queryAllByRole('button', { name: '+' })).toHaveLength(0);
+    expect(screen.getByTitle('Add thumbs up')).toBeTruthy();
+  });
+
   /**
    * objectui#3441 — with no provider the session language is whatever
    * react-i18next reports (in practice `'en'`), and the >= 7d branch now hands