Uh oh!
There was an error while loading. Please reload this page.
fix(collaboration): 给 CommentThread 的 "+" 表情选择器补上可访问名,而不只是 tooltip (#3478) - #3503
Merged
Conversation
…t its tooltip (#3478) The fourth glyph-only control in the 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. Different failure from #3441's three buttons, which is why it survived that fix — those carried 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 recorded the defect without naming it, asserting getByTitle('Add thumbs up') and queryAllByRole('button', { name: 'Add thumbs up' }) toHaveLength(0) in one green case — two assertions that together say "the title is set and it is not the name". - `aria-label` (accname §2C, outranks content) added alongside the existing `title`, on the same key. Zero new keys — the copy was always there. - `title` KEPT, not replaced: `+` says nothing to a sighted mouse user either, so the hover hint does real work. Same key for both, so they cannot drift. - Name stays `addThumbsUp` per the issue ruling: it describes what the button does today (`onReaction(id, '👍')`), not what `styles.reactionPicker` hints it might become. Picker-ification is a separate feature. - Adjacent reaction chips untouched — `${emoji} ${count}` already describes them, so name-from-content is correct there. Directions, predicted before the run and confirmed: with the tests applied and the source untouched, `{ name: 'Add thumbs up' }` found 0 buttons and the RTL accname dump listed the picker as `Name "+"` in both en and zh — direct proof of the content-over-title precedence. Both flip green with the `aria-label`. The getByTitle assertions are green on both sides on purpose (the tooltip must survive), as is the `React with thumbs up` count (#3441's separation holds). #3441's pin was rewritten in place to pin the new both-named state rather than the old separation, since that is the statement this change falsifies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt
The latest updates on your projects. Learn more about Vercel for GitHub. |
yinlianghui
commented
Aug 7, 2026
CollaboratorAuthor
|
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…me pins - "the only one of the three" was ambiguous once the describe covered four controls; it always meant the three naming mechanisms, now says so. - "+" is not an emoji — call the group glyph-only consistently. Comments only; no assertion or source change (72 passed, unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTRjn8xBqp75dk7kFupVRt
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 7, 2026 02:11
Uh oh!
There was an error while loading. Please reload this page.
akarma-synetal pushed a commit
to akarma-synetal/objectui
that referenced
this pull request
Aug 10, 2026
…t path filtering into the jobs (objectstack-ai#3523 steps 1-2) (objectstack-ai#3722) * ci(step 1): subscribe ci/lint/control-bytes/docs-links to merge_group (objectstack-ai#3523) Step 1 of objectui#3523, on its own commit as the issue's ruling requires: this is a pure addition and changes nothing about which pull requests or pushes run CI. The merge queue is enforced on this repository by a ruleset (objectstack-ai#3243 measured a direct push to `main` returning 405 `Changes must be made through the merge queue`), but not one of the 22 workflows subscribed `merge_group` — repo-wide `event=merge_group` runs stood at total_count = 0. A queue nothing subscribes to can only carry an empty required-check set, so it rebuilt each PR on the current `main` and let it through without validating anything. On 2026-08-07 that was cashed in: objectstack-ai#3503 / objectstack-ai#3510 / objectstack-ai#3516 merged between 02:11Z and 02:15Z with `Type Check` at conclusion=failure, on a `main` poisoned by objectstack-ai#3498, and objectstack-ai#3505 had to hot-fix it. The four workflows named by the issue now carry the trigger. It is spelled `merge_group: types: [checks_requested]`; `checks_requested` is the only activity type GitHub defines for this event today, so this is equivalent to objectstack's bare `merge_group:` and merely refuses to inherit a second type added later. Two `ci.yml` jobs additionally needed to be told the third event exists — both changes are no-ops for `pull_request` and `push`: - `test` moves from `if: github.event_name == 'pull_request'` to `!= 'push'`, so a queue build actually runs the suite instead of skipping every shard. - `docs`'s `should_run` treated anything that is not `push` as a pull request and diffed `github.event.pull_request.*`, which is null on a queue build — an empty revision range, i.e. the site build silently skipped on the last check before `main`. Verified rather than assumed: `concurrency` needs no merge-queue case. On `merge_group` the group expression falls back to `github.ref`, which is the queue's own generation — measured on objectstack, whose 3552 queue runs report head_branch `gh-readonly-queue/main/pr-6594-251e888a…`. That can collide with neither a PR group (a bare number) nor a push group (`refs/heads/main`). Refs objectstack-ai#3523 * ci(step 2): move ci/lint path filtering out of the PR trigger into the jobs (objectstack-ai#3523) Step 2 of objectui#3523, deliberately a separate commit from step 1: step 1 adds runs that did not exist, this one changes which pull requests start CI, and mixing them would make that impossible to review apart. `on.pull_request.paths-ignore` skips the WHOLE workflow when every changed file matches, and GitHub has no per-job path filter. A docs-only or changeset-only PR therefore started neither `ci.yml` nor `lint.yml` — objectstack-ai#3509 measured zero check runs from them. A check that is never *created* does not fail a required-status-check rule, it leaves the pull request pending; inside the merge queue it fails on the ruleset's 60-minute status-check timeout. So none of `Lint`, `Type Check`, `Test (shard N/4)`, `Build & E2E` or `Changeset Fixed Group Check` could be made required while the filter lived on the trigger — which is why the queue's required set was empty to begin with. The filter is not deleted, it moves. `type-check`, `test`, `e2e` (ci.yml) and `lint` (lint.yml) each open with a `Decide whether this change needs a full run` step, and every step after it carries `if: steps.relevant.outputs.should_run == 'true'`. The job always runs and always reports; the paths decide only whether it does any work. That is the shape `ci.yml`'s `docs` job has used since objectstack-ai#3450 — not a new mechanism — and the exclusion lists are byte-for-byte the `paths-ignore` they replace, so which PRs pay for a full run is exactly as before. Two deliberate narrowings, both stated so they can be argued with: - `paths-ignore` stays on the `push` trigger. Branch protection and the merge queue judge pull requests and queue builds, never pushes to `main`, so the push lane gains nothing from losing it and would cost a full CI run on every docs merge. It also gives the ignore list one authored home, which `merge-queue-reporting.test.ts` then pins the in-job copies against. - `changeset-check` is not gated. It is a checkout and one `node` call, so short-circuiting it would cost more in complexity than it saves. The gate fails OPEN: an unresolvable diff runs everything rather than reporting green having built nothing (objectstack#4928's filter contract). Measured both ways against a fixture repository — as shipped an unreachable base sha yields should_run=true; rewritten with the `|| echo ""` spelling `ci.yml`'s older `docs` gate still uses, the same input yields should_run=false, i.e. a silent full skip. That pre-existing `docs` gate is reported separately rather than changed here. `scripts/__tests__/merge-queue-reporting.test.ts` pins both steps; against the pre-change workflows 9 of its 10 assertions go red, each naming its own regression. `lint-workflow.test.ts`'s TypeScript-exclusion tripwire now reads the in-job list as well as the trigger, which is where that list now lives. Refs objectstack-ai#3523 --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#3478
同一个组件里第四个「只有字形」的控件,也是 objectui#3441 漏掉的那个。
缺陷
+按钮的内容就是字面量'+',所以 objectstack#5506 给它的title(collaboration.addThumbsUp)永远不可能成为它的可访问名 —— 对button而言,accname 算法先从内容取名(§2F),根本轮不到 §2I 的title。读屏念出来的是 "plus button"。文案是有的、十个语言包都译好了,却只送达了本来就看得见这个按钮的人。这和 #3441 修的那三个按钮是不同的失败形态,也正是它能从那次修复里活下来的原因:那三个在任何地方都没有作者写的文案,所以任何一次「这个 key 接上了吗」的检查都能发现缺口。这里 key 接上了,英文也在
COLLAB_DEFAULT_TRANSLATIONS里 —— 错的只是它的落点。#3441 自己的 pin 测试其实已经把这个缺陷记录在案了,只是没被读出来 —— 同一个绿色用例里同时断言:
两条同时为绿,合起来说的正是「title 设了,而且它不是名字」。当时的 docblock 把它们读成「把 picker 和快捷 👍 钉开」;它们同时也是一份没人读的 bug report。
修法
在既有
title旁边补aria-label(accname §2C,优先级高于内容),用同一个 key。零新增 key —— 文案一直都在。title保留而非替换。 这是本组件里唯一一个「两个都留才对、而不是冗余」的控件:+对用鼠标的明眼人同样什么都没说,悬停提示自有其价值。([finding] CommentThread 遗留两处:纯 emoji 按钮没有无障碍名;7 天以上时间戳不跟随会话语言 #3441 修的 👍/❤️ 本来就没有title可留,所以那边只有aria-label。)两个属性读同一个 key,名字和 tooltip 不可能漂移。addThumbsUp(按 issue 上的裁决):它描述这个按钮今天的行为(无条件onReaction(id, '👍')),而不是styles.reactionPicker暗示它将来可能变成什么。把它做成真正的表情选择器是独立的 feature,文案届时跟着行为走。它也继续与 [finding] CommentThread 遗留两处:纯 emoji 按钮没有无障碍名;7 天以上时间戳不跟随会话语言 #3441 的reactThumbsUp保持区分 —— 在已有 reaction 的评论上两个控件同屏,而现在两个都有真正的可访问名了,共用一个 key 会比 [finding] CommentThread 遗留两处:纯 emoji 按钮没有无障碍名;7 天以上时间戳不跟随会话语言 #3441 当初拒绝共用时更糟:两个外观不同的按钮会自报同一个名字。${emoji} ${count},本身就是描述性的,name-from-content 在那里是正确答案。方向(先预测,后运行)
只应用测试、不动源码时:
{ name: 'Add thumbs up' }找到 0 个按钮(RED),RTL 的 accname dump 把 picker 列为Name "+"—— en 与 zh 皆然,这就是「内容压过 title」的直接物证;aria-label后两者同时转绿({ name: '+' }归零、{ name: 'Add thumbs up' }为 1)。刻意两侧都绿的:所有
getByTitle断言(tooltip 必须存活)、以及React with thumbs up计 2(#3441 的区分没有被推翻)。#3441 的那条 pin 就地重写为钉住新的「两者都命名」状态,而不是旧的「分离」状态 —— 因为后者正是本次改动所证伪的那句话。
验证
改动范围:
CommentThread.tsx的那一个createElementprops 对象、两个测试文件、一个 changeset(patch)。packages/i18n未触碰。Generated by Claude Code