♿ fix: Restore WCAG AA Contrast for Text Tokens & Hide Edit Action While Streaming - #14677
Merged
Merged
Conversation
…ile Streaming Fixes the unreadable composer placeholder and the edit pencil that appears on hover mid-generation, plus the sibling token failures found while tracing the root cause. Placeholder: #13879 moved the composer from `dark:placeholder-white/60` to the semantic `placeholder:text-text-tertiary`, but `--text-tertiary` was `var(--gray-500)` in *both* themes, and #595959 is a dark gray. Dark mode fell from 5.90:1 to 1.91:1. Fixed at the token (dark -> gray-400, 4.56:1) rather than the call site: the token has 99 usages and was failing at 1.91-2.77:1 on every dark surface. The .gizmo dark theme already uses a light gray (#999999) for the same token, so only the default dark theme carried the inverted value. Two more instances of the same "token never tuned per theme" bug: - `--text-warning` was amber-500 in both themes: 2.15:1 in light across 13 real warning strings. Now amber-700 (5.02:1). - Light `status-{success,warning,error}` on their own `-subtle` fill measured 3.58 / 3.07 / 4.41 -- the exact pairing Alert, Badge, Tag and Chip use for every status variant. Bumped to the 700 ramp (5.21 / 4.84 / 5.91). Solid `bg-status-*` is only used for dots, so nothing renders text on it. Edit action: `hideEditButton` already covers `isSubmitting` and the button got `isVisible={false}` -> `opacity-0`, but `group-hover:opacity-100` (0,2,0) outranks bare `opacity-0` (0,1,0), so hovering the row revealed a disabled pencil. The reveal classes are now gated on `isVisible`, with `pointer-events-none` so the hidden button is inert. Both token sources of truth (style.css and themes/*.ts) were updated and verified in sync across all 67 tokens. Tests: new HoverButtons spec covers both hover states; semanticTokens.spec.ts gains a contrast guardrail over text tokens x surfaces and each status hue against its subtle fill, verified to fail on the original values. applyTheme.spec.ts now derives its expectation from the theme object instead of pinning a hex, so retuning a hue no longer breaks an unrelated plumbing test.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two UX/accessibility regressions in the chat UI by retuning semantic color tokens per theme to restore WCAG AA contrast (light + dark), and by ensuring the message “Edit” hover action cannot appear/interact while a message is streaming. It also adds/adjusts tests to prevent these regressions from recurring.
Changes:
- Retune semantic text/status tokens (e.g.,
text-tertiary,text-warning,status-*) to meet WCAG AA contrast in affected themes/surfaces. - Fix hover-action visibility behavior so the Edit pencil cannot be revealed by
group-hoverwhile streaming (and is fully inert when hidden). - Add/adjust Jest tests to guard contrast and hover-action behavior; make
applyThemetoken expectation derive from the theme object.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/client/src/theme/utils/applyTheme.spec.ts | Makes the --status-error assertion derive from defaultTheme to avoid brittle literal expectations. |
| packages/client/src/theme/themes/default.ts | Retunes light theme warning/status RGB ramps to improve contrast. |
| packages/client/src/theme/themes/dark.ts | Retunes dark theme tertiary text token to avoid low-contrast dark-on-dark usage. |
| packages/client/src/theme/semanticTokens.spec.ts | Adds a WCAG AA contrast guardrail across key text tokens/surfaces and status-on-subtle pairings. |
| client/src/style.css | Updates CSS semantic token defaults to match the retuned theme values (light + dark). |
| client/src/components/Chat/Messages/HoverButtons.tsx | Gates hover-reveal opacity classes on isVisible and makes hidden buttons inert via pointer-events-none. |
| client/src/components/Chat/Messages/tests/HoverButtons.spec.tsx | Adds regression tests ensuring the Edit button stays hidden/inert while streaming and reveals when settled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+65
| function toRgb(theme: IThemeRGB, token: keyof IThemeRGB): Rgb { | ||
| const parts = theme[token]?.trim().split(/\s+/).map(Number); | ||
| if (parts?.length !== 3 || parts.some(Number.isNaN)) { | ||
| throw new Error(`theme token "${token}" is not an "R G B" triplet`); | ||
| } | ||
| return [parts[0], parts[1], parts[2]]; | ||
| } |
Collaborator
Author
|
@codex review |
CI's changed-files import-order check flagged the new spec; the previous commit bypassed the lint-staged hook that would have caught it.
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
danny-avila
added a commit
that referenced
this pull request
Aug 7, 2026
The Jest spec can only assert class names — jsdom applies no stylesheet, so it could not see `disabled:opacity-50` (0,2,0) outranking `opacity-0` (0,1,0) and repainting the hidden pencil at half opacity. That is exactly how the ghost survived #14677 with a green suite. Asserts computed opacity in a real browser mid-stream, and asserts the sibling Copy action is at opacity 1 in the same breath so a hover that silently failed to register cannot make the check pass for the wrong reason. Verified to fail on the pre-fix build with `Received: "0.5"`, and to pass 3/3 after.
danny-avila
added a commit
that referenced
this pull request
Aug 7, 2026
* 🩹 fix: Keep Edit Action Fully Hidden While Streaming #14677 stopped the row-hover reveal from un-hiding the edit button, but the pencil still shows as a dimmed ghost mid-generation. The shared Button primitive sets `disabled:opacity-50`, which compiles to `.disabled\:opacity-50:disabled` — specificity (0,2,0). The hidden state used a plain `opacity-0` at (0,1,0), so the disabled style won and painted the icon at half opacity. Verified in Chromium against a running instance: only two opacity rules match the button, and the computed value was 0.5. Switching the hidden state to `!opacity-0` (Tailwind emits `opacity: 0 !important`) drops it to 0 while the sibling actions still reveal at 1 on hover. The existing unit test could not catch this: jsdom applies no stylesheet, so asserting class names never exercised the cascade. It now asserts the important modifier specifically, with a comment explaining why a bare `opacity-0` is insufficient. * 🧪 test: Browser guard for the hidden edit action The Jest spec can only assert class names — jsdom applies no stylesheet, so it could not see `disabled:opacity-50` (0,2,0) outranking `opacity-0` (0,1,0) and repainting the hidden pencil at half opacity. That is exactly how the ghost survived #14677 with a green suite. Asserts computed opacity in a real browser mid-stream, and asserts the sibling Copy action is at opacity 1 in the same breath so a hover that silently failed to register cannot make the check pass for the wrong reason. Verified to fail on the pre-fix build with `Received: "0.5"`, and to pass 3/3 after.
LogicalAbsurd
pushed a commit
to LogicalAbsurd/LibreChat
that referenced
this pull request
Aug 27, 2026
…ile Streaming (LibreChat-AI#14677) * ♿ fix: Restore WCAG AA Contrast for Text Tokens & Hide Edit Action While Streaming Fixes the unreadable composer placeholder and the edit pencil that appears on hover mid-generation, plus the sibling token failures found while tracing the root cause. Placeholder: LibreChat-AI#13879 moved the composer from `dark:placeholder-white/60` to the semantic `placeholder:text-text-tertiary`, but `--text-tertiary` was `var(--gray-500)` in *both* themes, and #595959 is a dark gray. Dark mode fell from 5.90:1 to 1.91:1. Fixed at the token (dark -> gray-400, 4.56:1) rather than the call site: the token has 99 usages and was failing at 1.91-2.77:1 on every dark surface. The .gizmo dark theme already uses a light gray (#999999) for the same token, so only the default dark theme carried the inverted value. Two more instances of the same "token never tuned per theme" bug: - `--text-warning` was amber-500 in both themes: 2.15:1 in light across 13 real warning strings. Now amber-700 (5.02:1). - Light `status-{success,warning,error}` on their own `-subtle` fill measured 3.58 / 3.07 / 4.41 -- the exact pairing Alert, Badge, Tag and Chip use for every status variant. Bumped to the 700 ramp (5.21 / 4.84 / 5.91). Solid `bg-status-*` is only used for dots, so nothing renders text on it. Edit action: `hideEditButton` already covers `isSubmitting` and the button got `isVisible={false}` -> `opacity-0`, but `group-hover:opacity-100` (0,2,0) outranks bare `opacity-0` (0,1,0), so hovering the row revealed a disabled pencil. The reveal classes are now gated on `isVisible`, with `pointer-events-none` so the hidden button is inert. Both token sources of truth (style.css and themes/*.ts) were updated and verified in sync across all 67 tokens. Tests: new HoverButtons spec covers both hover states; semanticTokens.spec.ts gains a contrast guardrail over text tokens x surfaces and each status hue against its subtle fill, verified to fail on the original values. applyTheme.spec.ts now derives its expectation from the theme object instead of pinning a hex, so retuning a hue no longer breaks an unrelated plumbing test. * 🔤 style: Sort imports in HoverButtons spec CI's changed-files import-order check flagged the new spec; the previous commit bypassed the lint-staged hook that would have caught it.
LogicalAbsurd
pushed a commit
to LogicalAbsurd/LibreChat
that referenced
this pull request
Aug 27, 2026
) * 🩹 fix: Keep Edit Action Fully Hidden While Streaming LibreChat-AI#14677 stopped the row-hover reveal from un-hiding the edit button, but the pencil still shows as a dimmed ghost mid-generation. The shared Button primitive sets `disabled:opacity-50`, which compiles to `.disabled\:opacity-50:disabled` — specificity (0,2,0). The hidden state used a plain `opacity-0` at (0,1,0), so the disabled style won and painted the icon at half opacity. Verified in Chromium against a running instance: only two opacity rules match the button, and the computed value was 0.5. Switching the hidden state to `!opacity-0` (Tailwind emits `opacity: 0 !important`) drops it to 0 while the sibling actions still reveal at 1 on hover. The existing unit test could not catch this: jsdom applies no stylesheet, so asserting class names never exercised the cascade. It now asserts the important modifier specifically, with a comment explaining why a bare `opacity-0` is insufficient. * 🧪 test: Browser guard for the hidden edit action The Jest spec can only assert class names — jsdom applies no stylesheet, so it could not see `disabled:opacity-50` (0,2,0) outranking `opacity-0` (0,1,0) and repainting the hidden pencil at half opacity. That is exactly how the ghost survived LibreChat-AI#14677 with a green suite. Asserts computed opacity in a real browser mid-stream, and asserts the sibling Copy action is at opacity 1 in the same breath so a hover that silently failed to register cannot make the check pass for the wrong reason. Verified to fail on the pre-fix build with `Received: "0.5"`, and to pass 3/3 after.
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Two reported regressions, plus the sibling failures found while tracing the root cause. All four are the same underlying bug: a semantic token that was never tuned per theme.
1. Composer placeholder unreadable in dark mode
#13879 moved the composer from
dark:placeholder-white/60to the semanticplaceholder:text-text-tertiary. But--text-tertiarywasvar(--gray-500)in both themes, and#595959is a dark gray — so dark mode inherited a foreground darker than most of its own surfaces.surface-chat(dark)placeholder-white/60)text-tertiary)Fixed at the token, not the call site:
text-text-tertiaryhas 99 usages and was failing at 1.91–2.77:1 on every dark surface. The.gizmodark theme already uses a light gray (#999999) for the same token, so only the default dark theme carried the inverted value.2. Edit pencil appears on hover mid-generation
hideEditButtonalready coversisSubmitting, and the button correctly receivedisVisible={false}→opacity-0. Butgroup-hover:opacity-100has specificity (0,2,0) and outranks bareopacity-0(0,1,0) — so hovering the row revealed a disabled pencil.The hover-reveal classes are now gated on
isVisible, withpointer-events-noneso the hidden button is fully inert. Both render paths (ContentRender,MessageRender) share this component.ForkandFeedbackuse the same class block but have no hidden state, so they were unaffected.3.
--text-warningin light mode — 2.15:1Same shape:
amber-500in both themes, never tuned for light. 13 usages, all real warning copy (ToolApproval,AskUserQuestion,DeleteAccount, API key warnings). Nowamber-700→ 5.02:1.4. Status hues on their own subtle fill (light) — Alert / Badge / Tag / Chip
text-status-xonbg-status-x-subtleis exactly how all four shared primitives paint every status variant:Bumped to the 700 ramp. Solid
bg-status-*is only used for small dots/indicators — nothing renders text on it — so darkening the light foregrounds is safe. Dark mode already measured 8–10:1 and is untouched.Not changed
text-destructive(4.09:1) andtext-warning(4.25:1) onbg-surface-tertiaryin light — no component pairs them today.status-*-strong+text-on-statustokens added in 🧩 refactor: Shared UI Design System Tokens #14670 all pass (4.53–10.05, clearly hand-tuned).Notes
Both token sources of truth (
client/src/style.cssandpackages/client/src/theme/themes/*.ts) were updated and verified in sync across all 67 tokens — nothing had enforced that before.Test plan
HoverButtons.spec.tsxcovers both hover states (hidden while streaming, revealed once settled).semanticTokens.spec.tsgains a contrast guardrail: text tokens × surfaces, and each status hue against its subtle fill. Verified it fails on the original values:applyTheme.spec.tspinned--status-errorto a literal hex; it now derives the expectation from the theme object, so retuning a hue no longer breaks an unrelated plumbing test.packages/client, 2056 acrossclient/src/components/Chat|MCP|utils, eslint clean,tsc --noEmitclean.