Skip to content

fix(ui): restore the send-message button icon (#4478) - #4480

Merged
Yeraze merged 1 commit into
mainfrom
fix/send-btn-icon-4478
Aug 1, 2026
Merged

fix(ui): restore the send-message button icon (#4478)#4480
Yeraze merged 1 commit into
mainfrom
fix/send-btn-icon-4478

Conversation

@Yeraze

@Yeraze Yeraze commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Fixes #4478

Root cause

Not the icon mapping — UiIcon's send → lucide Send entry is correct, as the issue suspected.

src/App.css declared .send-btn twice at equal specificity. The later block (old L1986) re-declared padding: 0.75rem 1.5rem and won the cascade, while width: 40px still came from the earlier block (old L1274). Under the global box-sizing: border-box (src/index.css), 48px of horizontal padding inside a 40px box leaves zero content width, so the flex child — <UiIcon name="send" size={16} /> — was squeezed to 0px wide.

The .send-btn.channel-action-btn siblings (bell, position) escaped only because their padding: 0.5rem !important outranked the duplicate — which is exactly why the report singles out the send button.

Verified in a browser, not just by reading

Worth doing: flex min-width: auto on a replaced SVG could plausibly have resisted the squeeze, which would have made the above analysis wrong. It doesn't. Measured against the real stylesheet in Chrome:

send icon sibling action icon send button
before 0 × 16 16 × 16 48 × 40
after 16 × 16 16 × 16 40 × 40

Two corrections to the issue report

  • Not theme-specific. Reproduces on the default Catppuccin theme; High Contrast Dark was incidental. (--ctp-accent-text is #000000 on --ctp-blue — fine contrast. The icon wasn't the wrong color; it had no width.)
  • Mobile was broken the same way. The duplicate sits after the @media (max-width: 768px) .send-btn override, silently shadowing it — the same cascade-ordering trap as Map features menu blocks sidebar connect button on mobile #3532. Confirmed broken before (icon 0 × 16 at 400px wide) and fixed after (44px button, 16 × 16 icon).

The fix

Collapse the two blocks into one, keeping the values the cascade actually produced (blue background, --radius-lg, 1rem) so the button's appearance is unchanged — the only computed-style change is horizontal padding, 1.5rem0.5rem. Pin .send-btn > svg { flex-shrink: 0 } so a future padding/width regression can't collapse the icon again.

Test plan

  • New src/styles/sendButtonIcon.test.ts — jsdom implements neither cross-stylesheet cascade nor layout, so this asserts on stylesheet source: one unconditional .send-btn rule, and a box model that leaves room for the 16px icon.
  • Confirmed it's a real regression test: all 3 assertions fail against the unfixed App.css and pass after. (My first draft passed on broken code because it read only the first rule — rewritten to resolve declarations the way the cascade does, last-wins.)
  • Full Vitest suite — success: true, 3947 files, 12875 passed, 0 failed.
  • npx tsc --noEmit — clean.
  • npm run lint:ci — no FAIL lines.

Follow-up (not fixed here)

Four other top-level selectors are still declared twice in App.css.dm-selector (699, 1878), .node-select (703, 1883), .device-info (814, 1060), .message-input-container (1245, 1908). Same fragility class; out of scope for this fix. Also noting the .send-btn:hover box-shadow is a green-tinted glow (rgba(166, 227, 161, 0.4)) on a now-blue button — preserved as-is since it's pre-existing rendered behavior, but it looks like a leftover.


Generated by Claude Code

App.css declared `.send-btn` twice at equal specificity. The later block
re-declared `padding: 0.75rem 1.5rem` and won the cascade, while
`width: 40px` still came from the earlier one. Under the global
`box-sizing: border-box`, 48px of horizontal padding inside a 40px box
leaves zero content width, so the flex child — `<UiIcon name="send"
size={16} />` — was squeezed to 0px wide and the button rendered as a
solid rectangle.

Verified in Chrome against the real stylesheet: before, the send icon
lays out at 0x16 while the `.send-btn.channel-action-btn` siblings are a
healthy 16x16 — those escaped only because their `padding: 0.5rem
!important` outranked the duplicate. After, the button is 40x40 with a
16x16 icon.

The duplicate also shadowed the `@media (max-width: 768px)` override for
`.send-btn`, which is declared between the two blocks, so mobile was
broken the same way (and is fixed here too).

Collapse the two blocks into one, keeping the values the cascade actually
produced so the button looks unchanged; only the padding conflict is
resolved. Pin `.send-btn > svg { flex-shrink: 0 }` so a future padding or
width regression cannot collapse the icon again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L9NzRtqE8eSMS8tvAeodUB
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: fix(ui): restore the send-message button icon (#4478)

Tasks:

  • Gather context and analyze PR
  • Review src/App.css changes
  • Review src/styles/sendButtonIcon.test.ts
  • Post final review

Overall Assessment

This is an excellent, well-reasoned bug fix. The root cause analysis is thorough, the fix is minimal and correct, and the test suite is unusually thoughtful for a CSS regression. A few notes below, none of which are blockers.


src/App.css

Fix is correct. The core change — collapsing two .send-btn blocks at equal specificity into one — directly resolves the cascade-ordering bug. The consolidated block at L1292 correctly preserves padding: 0.75rem 0.5rem (vertical × horizontal), which is geometrically sound: 2 × 0.5rem (8px) = 16px of total horizontal padding inside a 40px box leaves exactly 24px of content width, comfortably fitting the 16px icon.

min-width: 40px addition (L1294) is a good defensive addition over the original which only had width: 40px. This prevents flex layout from shrinking the button below its declared width.

The flex-shrink: 0 on the button itself (L1307) and .send-btn > svg (L1311–1313) provides two layers of regression protection. Redundant but reasonable given the history.

Minor observation: The :hover box-shadow (rgba(166, 227, 161, 0.4) — a green tint) on a blue button is visually inconsistent. The PR correctly notes this as pre-existing and out of scope — just flagging that it's a candidate for a follow-up cleanup ticket.

Tombstone comment at L2009 (/* .send-btn is defined once, above … */) is a good practice — it signals to future authors that the absence of a second rule is intentional. No objection.


src/styles/sendButtonIcon.test.ts

The test strategy is sound. jsdom doesn't implement multi-stylesheet cascade or layout, so testing the CSS source text directly is the right approach here. The PR description's note that a naive first-rule-only test would have passed against the broken CSS is exactly the kind of failure mode that makes tests misleading — and the effectiveDeclaration() helper correctly addresses it.

unconditionalSendBtnRules (L40–42) uses the multiline ^ anchor to match only top-level rules, which correctly excludes the @media-nested mobile override. The comment-stripping step (L30–33) is also necessary — the new tombstone comment at L2009 mentions .send-btn and would otherwise produce a false "rule body" match.

toPx (L61–67) only handles rem and px. If the unit is ever changed to em, %, or a CSS variable, the test will throw rather than silently pass — which is the right behavior for a regression guard. No concern here.

effectiveDeclaration (L51–58) simulates last-wins cascade across equal-specificity rules. The regex ((?:^|;)\s*prop\s*:\s*([^;]+)) is correct for the simple property formats used. One edge case: it would not correctly handle shorthand values containing semicolons (which CSS doesn't allow), so this is fine in practice.

Test naming: The test file lives in src/styles/ but tests src/App.css. Functionally fine — the readFileSync path is resolved relative to the test file's own import.meta.url, so it will always find the right file.


No Issues Found

  • No security concerns.
  • No performance concerns (source-text tests are fast; no rendering involved).
  • The fix does not introduce new CSS specificity issues.
  • min-width addition is additive and safe.

Follow-up Candidates (not blockers for this PR)

The PR description already documents these well:

  1. Four other duplicate top-level selectors in App.css (.dm-selector, .node-select, .device-info, .message-input-container) — same fragility class, worth addressing in a follow-up.
  2. The green-tinted box-shadow on a blue button in .send-btn:hover.

Verdict: Approved. The fix is correct, well-documented, and the regression test is appropriately designed for the CSS cascade constraint.

@Yeraze
Yeraze merged commit 14fa005 into main Aug 1, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Send message button renders with no icon (Lucide style, High Contrast Dark theme)

1 participant