Repository navigation
fix(webui): keep message actions visible - #6955
Merged
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-6955 environment in ironclaw-ci-preview
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-6955
July 31, 2026 09:34
Destroyed
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe message metadata and action row is now always visible. The component removes hover/focus opacity utilities, and its tests verify the updated visibility and control-sizing expectations. ChangesMessage metadata visibility
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size: XS
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts (1)
551-570: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale assertion message.
The message at Line 560 still says “hover meta row”. Change it to “always-visible meta row” so test failures describe the current contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts` around lines 551 - 570, Update the assertion message in the test named “message timestamp and actions share an always-visible meta row” from “hover meta row” to “always-visible meta row”; leave the assertion and implementation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts`:
- Around line 567-570: Update the visibility assertion in the message-bubble
test to first extract the metadata-row class expression from
messageBubbleSource, then assert that this extracted expression contains none of
the prohibited opacity utilities regardless of class order. Replace the current
text-iron-400-anchored regular expression while preserving the existing failure
message and visibility requirements.
---
Outside diff comments:
In
`@crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts`:
- Around line 551-570: Update the assertion message in the test named “message
timestamp and actions share an always-visible meta row” from “hover meta row” to
“always-visible meta row”; leave the assertion and implementation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1cb43dfb-a215-4a77-a96f-e5f096d72ff3
📒 Files selected for processing (2)
crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.tscrates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.tsx
Comment on lines
+567
to
+570
| assert.doesNotMatch( | ||
| messageBubbleSource, | ||
| /text-iron-400[^"\n]*(?:opacity-0|group-hover:opacity-100|focus-within:opacity-100)/, | ||
| "message actions should not depend on hover or keyboard focus for visibility", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the visibility assertion independent of class order.
The regular expression at Line 569 only detects opacity utilities after text-iron-400. If a future edit places opacity-0 before that token, the test passes while the row is hidden. Extract the metadata-row class expression first, then assert that it contains none of the prohibited opacity utilities.
Proposed test adjustment
+ const metaRowStart = messageBubbleSource.indexOf('"mt-1 flex min-h-7');
+ const metaRowEnd = messageBubbleSource.indexOf('].join(" ")', metaRowStart);
+ const metaRowSource = messageBubbleSource.slice(metaRowStart, metaRowEnd);
assert.doesNotMatch(
- messageBubbleSource,
- /text-iron-400[^"\n]*(?:opacity-0|group-hover:opacity-100|focus-within:opacity-100)/,
+ metaRowSource,
+ /opacity-0|group-hover:opacity-100|focus-within:opacity-100/,
"message actions should not depend on hover or keyboard focus for visibility",
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| assert.doesNotMatch( | |
| messageBubbleSource, | |
| /text-iron-400[^"\n]*(?:opacity-0|group-hover:opacity-100|focus-within:opacity-100)/, | |
| "message actions should not depend on hover or keyboard focus for visibility", | |
| const metaRowStart = messageBubbleSource.indexOf('"mt-1 flex min-h-7'); | |
| const metaRowEnd = messageBubbleSource.indexOf('].join(" ")', metaRowStart); | |
| const metaRowSource = messageBubbleSource.slice(metaRowStart, metaRowEnd); | |
| assert.doesNotMatch( | |
| metaRowSource, | |
| /opacity-0|group-hover:opacity-100|focus-within:opacity-100/, | |
| "message actions should not depend on hover or keyboard focus for visibility", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@crates/ironclaw_webui/frontend/src/pages/chat/components/message-bubble.test.ts`
around lines 567 - 570, Update the visibility assertion in the message-bubble
test to first extract the metadata-row class expression from
messageBubbleSource, then assert that this extracted expression contains none of
the prohibited opacity utilities regardless of class order. Replace the current
text-iron-400-anchored regular expression while preserving the existing failure
message and visibility requirements.
Contributor
🔎 Review · PR #6955
Submitted review →Reviewed the complete trusted base-to-head comparison. The focused WebUI change correctly removes hover/focus opacity gating from the message metadata row while preserving action availability, alignment, disabled states, and icon controls. The regression test covers the intended class contract. No actionable findings identified. Automatic · PR opened + CI failed · attempt 1 of 3 · completed in 1m 9s Run details
|
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🔍 Review complete · PR #6955
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison. The focused WebUI change correctly removes hover/focus opacity gating from the message metadata row while preserving action availability, alignment, disabled states, and icon controls. The regression test covers the intended class contract. No actionable findings identified.
Validation and technical details
- Verified trusted refs: base 945d926 and head 60ec71e.
- Inspected the complete two-file diff and surrounding MessageBubble rendering/action-gating logic.
- Confirmed the reviewed files in the checkout match refs/ironloop/head.
git diff --check refs/ironloop/base refs/ironloop/headpassed.- Focused Vitest run passed: 1 file, 18 tests. The environment reported Node 24.18.0 despite the package requesting Node >=22 <23.
- Base:
main - Head:
codex/always-show-message-actionsat60ec71e - Run:
9065a074-9dee-4364-9bdd-d1bfbb73e346
serrrfirat
added
the
skip-regression-check
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 86.6% — 322406 / 372279 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (18 entry/entries excluded from the accounting above)
|
This branch was successfully deployed
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
1 participant
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
Change Type
Linked Issue
None — requested as a focused WebUI visibility correction.
Validation
cargo fmt --all -- --check— Not applicable: no Rust changed.cargo clippy --all --benches --tests --examples --all-features -- -D warnings— Not applicable: no Rust changed.pnpm lint(lint:conventions+tsc --noEmit)pnpm test— 117 files / 986 testspnpm build— includes enforced bundle budgetscargo test --features integration— Not applicable: no database or integration behavior changed.review-pr/pr-shepherd --fix— Not run for this two-file presentation fix.Test Strategy
User behavior: Given a rendered user or final assistant message, when the pointer is not hovering the message and no action has focus, then its timestamp and permitted message action icons remain visible.
Risk areas:
Tests added or updated:
message-bubble.test.tsto require the always-visible meta-row classes and rejectopacity-0,group-hover:opacity-100, orfocus-within:opacity-100visibility gating.What the tests prove: message timestamps and permitted action icons remain in the rendered meta row without hover/focus opacity gating, while the existing action layout and icon-only controls remain intact.
Commands run:
pnpm exec vitest run src/pages/chat/components/message-bubble.test.tspnpm lintpnpm testpnpm buildSecurity Impact
None. Existing action availability and export feature gates are unchanged.
Reborn Trust-Boundary Checklist
N/A — presentation-only frontend class change; no trust, ingress, runtime, persistence, or policy boundary changed.
Database Impact
None.
Blast Radius
Limited to the timestamp/action meta row for user and final assistant message bubbles. A regression could affect visual density, but not whether an action is authorized or executable.
Rollback Plan
Revert the single commit to restore hover/focus-only visibility.
Review Follow-Through
No known follow-up. Reviewer judgment is limited to the always-visible visual density.
Review track: A (presentational WebUI bug fix with focused regression coverage)