Skip to content

Line up a notification's button with its text when it has no icon - #28042

Closed
brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/toast-action-align-without-icon
Closed

brennanb2025 wants to merge 1 commit into
mainfrom
brennanb2025/toast-action-align-without-icon

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 0 0 0 0
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​7 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5

ELI5

Some notifications in the bottom-right corner have a button but no icon. On those, the button sat indented under nothing, about the width of a missing icon away from the text above it. After a restart, the "Resumed 14 chats" notification looked broken: its Show button floated out of line under the title.

Now the button lines up with the text whenever the notification has no icon. Notifications that do have an icon look exactly as before.

What Changed

  • Before: every notification button was pushed 26px to the right. That only lines it up with the text when an icon sits to the left of the text.
  • After: the button starts flush with the text. The 26px indent applies only when an icon comes before the button, using the order Sonner (the notification library) always renders: close button, icon, text, buttons. A second button still sits right next to the first.

This touches only the shared notification stylesheet (main.css). It fixes every icon-less notification that has a button, not just the resume one (dictation, activity clear, external file change, and others).

Why

The indent was written for icon notifications and applied to all of them. Making the indent depend on the icon fixes the cause, so it's better than giving the resume notification an icon just to fill the gap: that would leave the other icon-less notifications broken and change several test mocks for nothing. The selector uses a plain sibling match ([data-icon] ~ [data-button]) instead of :has(), which keeps clear of the style recalculation cost an earlier :has() rule in this file caused.

Linked Issue

N/A (maintainer report from a screenshot of the resume notification)

Visual Proof

The real "Resumed 3 chats" toast, captured the moment it appeared. Both runs used a hidden Orca dev build on a throwaway profile, driven only through the developer tools connection. Three real Claude chats were left running a 5-minute command, Orca was quit, and on relaunch the chats were resumed from the dialog. Before = current main (c62c5a4). After = this PR plus #27580, merged locally.

Before (main) After
before after

Measured in the app: before, the button's left edge sat 26px right of the title (x 1331 vs 1305). After, both are at x 1305.

The first check was a headless Chromium render of the three toast shapes (no icon, with an icon, no icon with a description), using the toast library's stylesheet plus Orca's:

Before After
before.png after.png

Testing

  • orca-ci-checks: 22 of 24 passed. The two failures are unrelated to this change. audit:code-quality:native also fails on current main (a config script's imports from ci: stop new main-process code from freezing Orca with synchronous launches #27756), and typecheck:ci fails locally with Bun's Script not found "check". The plain pnpm run typecheck passed.

  • No automated test: this is a CSS-only change, and the before/after render above covers it.

  • I manually tested these changes locally (real resume in a hidden dev build, macOS, light theme; see Visual Proof)

  • Automated tests added/updated, or explained why not below

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

CSS only, in the renderer. It has no effect on platform, SSH, mobile or compatibility, and right-to-left layout is unchanged: the indent was already a left margin.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 01d7f965-e491-4322-b4b0-ff3a7eaf50b0

📥 Commits

Reviewing files that changed from the base of the PR and between c62c5a4 and 1abdadc.


📒 Files selected for processing (1)
  • src/renderer/src/assets/main.css

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.



📝 Walkthrough

Walkthrough

Toast action buttons no longer have a default left margin. Buttons that follow an icon retain a 26px left offset.


Priority: ⬇️ Low

Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 1abda

The stylesheet aligns iconless toast actions with their text while preserving spacing after icons; no material merge-blocking risk remains.

Pre-merge checks · Passed 5
Passed Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: aligning notification buttons with text when no icon is present.
Description check Passed The description includes all required sections, explains the user impact and implementation, provides issue context, visual proof, testing details, known unrelated CI failures, and completed checklist…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

After restart, the “Resumed chats” notification put Show 26px to the right of its text because the shared stylesheet reserved icon space even when no icon rendered. This PR removes that empty indent from native notification buttons without icons and preserves the existing spacing when an icon is present.

Reviewed 1abdadc4af22537d83cbed126e86df7b24fe7597. Independent architecture and correctness reviews, a dedicated performance source scan, and the final readiness checklist found no proven PR-caused issues. Shared CSS addresses the common cause across iconless actions and can land independently of the bottom-bar change. No review fixes, deferred code findings, commits, or pushes were needed.

Fresh hidden-app QA ran on M4Air using OpenCode opencode-go/deepseek-v4.1-flash. I inspected clean captures showing the iconless Show aligned with its text in dark and light themes, the narrow 420px layout, and action/cancel buttons on notifications with icons retaining their spacing. The attached before image recreates the old 26px margin with a CSS override on the reviewed head; it is a constructed comparison, not a parent-commit build. Both images use constructed notification results, not a real chat restart.

QA remains incomplete. The driver exited with ClientError: Transport: The operation timed out before completing a real Codex restart/resume and its Show action. Custom icons, promise/loading transitions, formatted descriptions, custom toasts, and colored notifications received source review but lack accepted fresh visual proof. Direction changes were exercised, but full right-to-left layout acceptance and Windows/Linux rendering remain unverified. Earlier captures with onboarding or instrumentation errors were rejected. No runtime performance measurements or unit/typecheck runs were made for this CSS-only review.

CI is red from the existing main-branch failure: static analysis imports diffCounts, formatBaseline, parseBaseline, and parseBaselineNotes from a module that does not export them; verify fails downstream. Both affected scripts are byte-identical on this PR and inspected origin/main (a94092908c2). Recorded CI logs. No unrelated repair or CI rerun was made.

The temporary M4Air worktree qa-pr-28042 was removed after the rig exited and the checkout was confirmed clean. Evidence remains outside the worktree in the PR-specific QA directory.

No merge, rebase, auto-merge, or draft-state change was performed. Source review is clean; this is not an unconditional QA pass.

Constructed previous spacing:

Constructed previous 26px spacing

Reviewed spacing:

Reviewed head with aligned Show button

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing: the resume toast this started from was redesigned in #28068 (icon + inline action), and the small misalignment left on other toasts with a button isn't worth a change on its own.

@brennanb2025
brennanb2025 deleted the brennanb2025/toast-action-align-without-icon branch October 13, 2026 07:07
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.

1 participant