Polish profile popover, pricing modal spacing, and dropdown hover styling - #5752
Polish profile popover, pricing modal spacing, and dropdown hover styling#5752cmgardella wants to merge 2 commits into
Conversation
- Fix skill-selector footer buttons touching with no gap when stacked;
size them to content and lay out via flex-wrap
- Remove leftover scroll-shadow ("chin") artifact on the LLM select list
and restyle its footer actions to match the skill-menu pill buttons
- Resize the Past Sessions popover to inset 30px from both panel edges,
match its header font to the panel title, and restyle background,
border, dividers, and hover colors
- Fix a nested double-stroke on the session "more options" dropdown by
neutralizing the wormholed BoxelDropdown content wrapper's own border/shadow
- Auto-close the "more options" dropdown on mouse-out (hover-intent debounce)
instead of requiring a click elsewhere, and clear lingering focus styling
- Give the currently-active session row a hover state even while its
options menu is open
- Restyle the workspace chooser's "View All" sort dropdown (default state
matches the chooser background; open state uses a dark platter with a
white 25%-opacity stroke, white text, and a teal active-item checkmark)
…ling - Restyled the profile popover: lighter divider, teal Upgrade Plan CTA matching Sign Out's style/font, custom hover colors for black/teal buttons, removed the credit icon next to "Not available on Free plan", flat #EFEDED daily-grant note with no border, and precise 20px spacing around the Buy More Credits CTA - Added 50px of breathing room above the pricing modal's headline while keeping scroll-snap stable via matching scroll-padding-top - Added a 100%-opacity white stroke on hover for the search, AI assistant, and profile avatar icon buttons, matching the rest of the app's icon button hover treatment - Made the workspace chooser's "View All" sort dropdown and the hosted-site-url dropdown's hover state span edge-to-edge like the standard boxel dropdown/menu pattern, instead of being inset from the platter
a240bde to
f464a63
Compare
12fefc3 to
15e5032
Compare
richardhjtan
left a comment
richardhjtan
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Went after this as a styling change: the colors and units against the repo's .gts CSS conventions, the :global() reach into other packages' internals, the new hover-close behavior's keyboard and teardown paths — plus where this PR can actually land.
Blocking, and mechanical: the base branch has already merged. This targets ai-assistant-panel-design-polish, and the PR that owned that branch merged two weeks ago. Retarget to main first — main has since taken 13, 12, 10 and 7 commits on workspace-chooser/index.gts, matrix/room.gts, workspace-chooser/workspace.gts and submode-layout.gts, and a merge into main conflicts in four files: ai-assistant/past-session-item.gts, matrix/room.gts, workspace-chooser/index.gts, pill-menu/index.gts. Everything below is worth doing on the rebased diff, not this one.
Recommendations:
- Every hardcoded color in the diff is either an existing named token or a hairline variant of one — and four of them replace a token that was already there. See the thread on
.menuinpast-session-item.gts; it carries the full mapping and applies topast-sessions.gts,workspace-chooser/index.gts,profile-info-popover.gtsandsubmode-layout.gtstoo. - The sort dropdown is styled through third-party class names nothing in this repo pins. See the thread on
workspace-chooser/index.gts. - The new hover-close blurs the trigger and leaves a timer running past teardown. See the thread on
scheduleCloseDropdown. - Raw
pxwhere the conventions ask forremor a token, including onecalc()that mixes a token with a magic offset. See the threads onpast-session-item.gtsand the pricing modal.
The red check is stale, not a signal. Host Memory Baseline failed on a run started three weeks ago, before the current head. It needs a re-run on the rebased branch before it means anything.
Adjacent, out of scope. llm-select.gts's new :deep(.menu-content::after) { display: none } describes, in its own comment, a PillMenu bug: LLMSelect builds a footer row inside .llm-list instead of passing a :footer block, so PillMenu never sets has-footer and leaves its bottom scroll-shadow floating over the list. Suppressing the shadow from the consumer is the right size of fix for a polish PR; the footer block belongs in PillMenu's own API and not here.
| background: #3b394b; | ||
| border: 1px solid rgba(255, 255, 255, 0.25); |
There was a problem hiding this comment.
[Claude Code 🤖] These are named tokens, and swapping a token out for its literal costs the theming that token exists to provide. packages/boxel-ui/src/styles/variables.css already defines every value the diff hardcodes:
| literal in the diff | token | note |
|---|---|---|
#3b394b |
--boxel-650 |
also in past-sessions.gts, workspace-chooser/index.gts |
#272330 |
--boxel-700 |
replaces var(--ai-assistant-menu-hover-background) here |
#4f4b57 |
--boxel-575 |
replaces var(--past-sessions-divider-color) |
#afafb7 |
--boxel-400 |
profile-info-popover.gts, replaces var(--boxel-dark) |
#00ffba |
--boxel-teal / --boxel-highlight |
workspace-chooser/index.gts checkmark |
rgba(255, 255, 255, 1) |
— | this is #fff; submode-layout.gts and ai-assistant/button.gts too |
Four more have no token and are a hairline off one that exists, which is the tell that they were sampled from a mockup rather than looked up: #00d99e against --boxel-dark-teal: #00da9f (which is what --boxel-highlight-hover resolves to), #efeded against --boxel-50: #f5f5f5, plus #00c38e, #404040, #595959.
.claude/skills/gts-component-conventions/SKILL.md §3 asks for two things here, and the second matters more than the first: no raw literal in a component, and reach for the semantic role token rather than the numbered palette, because --boxel-400…--boxel-700 are primitives that do not flip between light and dark while --muted, --muted-foreground, --foreground and --boxel-highlight-hover do. So --boxel-highlight-hover for the teal hover, not a new #00d99e; and where a hover surface recurs across these four components, one component-scoped custom property rather than the same literal in four files.
Blocking, and a regression — the tokens being replaced were correct before this diff.
| } | ||
| :global( | ||
| .workspace-chooser-sort-dropdown | ||
| .boxel-select-option-item.ember-power-select-option--selected.ember-power-select-option--highlighted | ||
| ) { | ||
| background-color: #3b394b; | ||
| } | ||
| :global( | ||
| .workspace-chooser-sort-dropdown | ||
| .boxel-select-option-item.ember-power-select-option--selected:hover | ||
| ) { |
There was a problem hiding this comment.
[Claude Code 🤖] ember-power-select-option--selected and --highlighted are ember-power-select's own class names, and nothing in this repo pins them — a version bump that renames them takes the selected-row hover with it, silently and only in this one dropdown.
@dropdownClass plus the --boxel-dropdown-* custom properties above is the supported seam, and it is already doing most of the work here. What is left reaching past it is only the selected-and-highlighted state. Ask: can BoxelSelect expose that state as an override knob (a --boxel-dropdown-selected-hover-color, say) so the rule lives next to the other five and the third-party name stays inside the component that owns the dependency? If it can't, a comment naming ember-power-select as the source of those two classes at least tells the next reader why the selector looks like this.
Non-blocking; a decision about where the seam goes.
| private scheduleCloseDropdown() { | ||
| this.cancelCloseDropdown(); | ||
| this.closeDropdownTimer = setTimeout(() => { | ||
| this.closeDropdownAction?.(); | ||
| this.triggerElement?.blur(); | ||
| }, 200); | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖] Two things on this path.
The blur is unconditional. mouseleave on the row schedules a close that also calls blur() on the trigger, so a keyboard user who opened the menu with Enter and then moves the mouse — anywhere, for any reason — loses focus to <body> and has to tab back in. Gate the blur on the interaction that needs it (the pointer path) or drop it; the close itself is fine.
The timer outlives the component. Nothing clears closeDropdownTimer on destroy. The callback is harmless once the modifiers have nulled closeDropdownAction and triggerElement, so this is not a user-visible bug — but a pending timer per row past teardown is exactly the shape the host suite's memory work chases, and the fix is three lines in the constructor:
registerDestructor(this, () => this.cancelCloseDropdown());Non-blocking. The blur is the half worth a decision.
| } | ||
| .date { | ||
| margin-top: var(--boxel-sp-xxs); | ||
| margin-top: calc(var(--boxel-sp-xxs) - 5px); |
There was a problem hiding this comment.
[Claude Code 🤖] calc(var(--boxel-sp-xxs) - 5px) subtracts a magic offset from a token, so the result moves whenever the token is retuned — in a direction nobody chose. If the intended margin is a fixed value, write that value; if it's a smaller step on the spacing scale, use that token.
Same ask for font-size: 12px on the next line and the other raw pixels the diff adds — left/right: 30px in past-sessions.gts, gap: 5px in matrix/room.gts and pill-menu/index.gts, margin-bottom: 4px in profile-info-popover.gts. §2 of the .gts conventions asks for rem or an existing --boxel-sp-* / --boxel-font-size-* token; the root is 16px, so 12px is 0.75rem and there is a --boxel-font-size-xs.
Non-blocking, except that the calc() is the one that will actually surprise someone.
| <style scoped> | ||
| :global(.choose-subscription-plan-modal .dialog-box__content) { | ||
| scroll-snap-type: y mandatory; | ||
| scroll-padding-top: 50px; |
There was a problem hiding this comment.
[Claude Code 🤖] This 50px and the padding-top: 50px on .boxel-pricing-container have to stay equal or scroll-snap lands the headline under the absolutely-positioned header — but nothing here says so, and they are in different rules 20 lines apart. One custom property used by both makes the constraint the code rather than a coincidence:
.boxel-pricing-container { --pricing-header-offset: 3.125rem; padding-top: var(--pricing-header-offset); }
:global(.choose-subscription-plan-modal .dialog-box__content) { scroll-padding-top: var(--pricing-header-offset); }Also in this block: @title='' on ModalContainer empties the header rather than removing it, which is why the header then needs position: absolute + pointer-events: none and the close button needs pointer-events: auto handed back. If ModalContainer can take no title at all, that whole four-rule workaround goes away. Worth checking before this lands, since an empty-string title is the kind of thing a later change to ModalContainer breaks without knowing anyone depended on it.
Non-blocking.
4 participants
Summary
#EFEDEDdaily-grant note with no border, and precise 20px spacing around the "Buy more credits" CTAscroll-padding-topStacked on #5728 so this diff only shows the new changes.
Test plan
🤖 Generated with Claude Code