🩹 fix: Keep Edit Action Fully Hidden While Streaming - #14687
Conversation
#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.
There was a problem hiding this comment.
Pull request overview
This PR fixes a UI regression in the chat message hover actions where the Edit (pencil) button could still appear as a faint “ghost” while a response is streaming, due to Tailwind specificity between disabled:opacity-50 (from the shared Button) and a plain opacity-0.
Changes:
- Strengthen the hidden-state styling for the Edit action by using Tailwind’s important modifier (
!opacity-0) so it reliably overridesdisabled:opacity-50. - Update the unit test to assert the presence of
!opacity-0(and absence of bareopacity-0) to guard against future regressions of this specificity issue.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| client/src/components/Chat/Messages/HoverButtons.tsx | Ensures the hidden Edit button stays fully invisible while disabled/streaming by using !opacity-0. |
| client/src/components/Chat/Messages/tests/HoverButtons.spec.tsx | Adjusts assertions to specifically require !opacity-0, matching the intended cascade behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
|
Added a browser-level regression guard: Why this needed to be an e2e test. The Jest spec I added in #14677 passed while the bug was live in production. jsdom applies no stylesheet, so it can only assert class names — it cannot see that Verified in both directions against a real build (
Guards against false positives. The spec asserts the sibling Copy action is at Uses the existing |
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. 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". |
) * 🩹 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.
Summary
Follow-up to #14677. That PR stopped the row-hover reveal from un-hiding the edit button, but the pencil still shows as a dimmed ghost while a response is streaming.
The shared
Buttonprimitive setsdisabled:opacity-50, which compiles to.disabled\:opacity-50:disabled— specificity (0,2,0). The hidden state used a plainopacity-0at (0,1,0). The button isdisabledwhile streaming, so the disabled style won and painted the icon at half opacity.Only two opacity rules match that button, and this is the whole bug:
Verification
Measured in Chromium against a running instance (login → send → hover the user row mid-stream):
The sibling Copy button still measures 1 on hover, so the reveal behavior is unchanged — only the hidden action is affected.
Confirmed separately that Tailwind 3.4.1 emits
.\!opacity-0 { opacity: 0 !important }for the!prefix, so the fix is a real cascade win rather than a class that silently no-ops.Why the unit test missed it
HoverButtons.spec.tsxasserted class names, and jsdom applies no stylesheet — sotoHaveClass('opacity-0')passed while the rendered pixel was 50% opaque. Class-name assertions cannot see a specificity conflict.The test now asserts the important modifier specifically (
!opacity-0, and not bareopacity-0), with a comment recording why. That still can't evaluate the cascade, so the honest guard for this class of bug is a computed-style assertion in Playwright — worth adding to the e2e mock suite as a follow-up if you want it.Test plan
client/src/components/Chat/Messages— 689 tests pass.--check, andsort-imports --checkall clean on both changed files.🤖 Generated with Claude Code