Repository navigation
feat(app): make the session menu's Delete item red and drop its ellipsis - #184
Merged
Merged
Conversation
Both surfaces the item renders on — the sidebar/home session rows and the menu beside a session title — go through SessionActionsMenu, so this is one edit. The colour rides on the item's own `--menu-v2-fg` hook rather than a text utility, because menu-v2.css colours the label span directly and a class on the item would not reach it. Only the v2 branch is touched: the legacy DropdownMenu fallback is unreachable now that settings.general.newLayoutDesigns() is hardcoded true. Adds an e2e guard, since the colour depends on a shared component's internal variable. It resolves the danger token at runtime instead of pinning a hex, and checks that a sibling item is not also red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Unit: the Recents row budget moves to app-sidebar-rows.ts as `recentsRowCapacity` and gets a table of heights. It was the one real logic seam in this work — off-by-one arithmetic over the row pitch and gap that decides whether "View all" stays reachable — and it was untested. Extraction is behaviour-identical. E2E: the Delete item is now asserted on both surfaces the user sees it on, the sidebar row menu and the in-session header menu, sharing one set of assertions. The header trigger opens on the first click, so only the sidebar helper retries past the known first-click defect. E2E: the section header is checked for keyboard operability, and a height sweep pins that "View all" is never clipped out of the list. That sweep stops at a 400px window. Below it the flexible area drops under one row and View all is drawn with nowhere to go — pre-existing, unrelated to the collapse work, and left for its own fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
quickbeard
added a commit
that referenced
this pull request
Aug 11, 2026
* docs: fold the seven sidebar entries into one The Unreleased section had grown to seven bullets that are all one piece of work: the persistent sidebar, its Recents section, the titlebar buttons, the folding header, the row menus, and two follow-ups to those. Read from a release, that is one feature, so it is now one entry carrying all six PR numbers. Written as the end state rather than as a history. #185 fixed a row menu that ignored its first click and a View all row that could be drawn clipped, and #184 restyled Delete — all of them defects in, or refinements to, work that has not shipped yet, so a reader of the released changelog never saw the behavior being corrected. What survives from them is the behavior itself: View all is left out rather than clipped in a window too short for it, and Delete is red with no ellipsis. No detail was dropped on the way: the Projects/Settings/Help move, Recents sizing and its View all row, the fold and its persistence, the narrow-window fallback, both titlebar buttons and the absence of keybinds for them, View → Toggle Sidebar, and every item in the row menu with its rename/delete/share behavior are all still stated. Released sections are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * minor changes --------- Co-authored-by: Quickbeard <quickbeard@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Two small changes to the Delete item in a session's ⋮ / ⋯ menu: it loses its trailing ellipsis and turns red. Both the sidebar rows and the menu beside a session title render the same
SessionActionsMenu, so this is one edit covering both (and the home "View all" list, which uses it too).Notes on the two lines:
--menu-v2-fg, the item's own colour hook inmenu-v2.css, not atext-*utility. The label lives in a[data-slot="menu-v2-item-content"]span that setscoloritself, so a class on the item would not reach it; setting the variable recolours both. The highlighted state only swaps the background, so it survives hover. This keeps the change inside the fork-onlysession-actions-menu.tsxinstead of adding adangervariant to the sharedMenuV2inpackages/ui— the lowest tier that works, per the divergence policy.DropdownMenufallback in the same component is unreachable:settings.general.newLayoutDesigns()is hardcodedtrue(context/settings.tsx:209). I left the dead branch alone rather than expand scope.Share...keeps its ellipsis, so this is not a blanket strip of the convention. Delete still opens its confirmation dialog.Test
New
session-menu-delete-item.spec.tsguards it, because the colour depends on a shared component's internal variable name. It resolves the danger token at runtime rather than pinning a hex, asserts the label (not the row) carries it, checks a sibling item does not, and re-checks after hover.Pre-existing defect found while writing that test
The session-row ⋮ trigger swallows its first click and opens on the second. This reproduces on a clean
maincheckout with the hover-reveal wrapper fully faded in (pointer-events: auto) and the app settled for 2.5s, so it is not a harness artifact and not caused by this PR. The spec retries past it with a comment saying so rather than asserting the behaviour into place. Worth its own fix.app-sidebar.tsxgains onedata-slot="session-row-actions"hook so the spec can wait on that wrapper's fade instead of a magic timeout — the button is "visible" to Playwright at opacity 0, so it is no signal.Divergences
session-actions-menu.tsxandapp-sidebar.tsxare both fork-only, as is the new spec, so no upstream files change.DIVERGENCES.mdrow 125 said the menu moved "verbatim" frommessage-timeline.tsx; that is now qualified, so a future syncer does not diff the two expecting a match.Verification
bun typecheck(packages/app) — cleanbun test(packages/app) — 619 passbun test:e2e:local— 94 pass, 2 fail inremote-session-settings.spec.ts; both fail identically on cleanmain, pre-existing and unrelatedbun lint— identical to themainbaseline (4740 warnings, 1 pre-existing error)rgb(184, 45, 53), holding through hover)One unrelated line in
session-actions-menu.tsxis reformatted: the file was already prettier-dirty onmainandprettier --writefixed it.🤖 Generated with Claude Code