Skip to content

feat(sidepanel): neutral #161618 surface, near-white buttons, exit animations - #178

Open
dinndev2 wants to merge 1 commit into
mainfrom
feat/monochrome-surface-and-motion
Open

dinndev2 wants to merge 1 commit into
mainfrom
feat/monochrome-surface-and-motion

Conversation

@dinndev2

@dinndev2 dinndev2 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Design-pass round 3. Supersedes the accent-tinted buttons from #177 — the reference is a light-filled primary next to a dark outlined secondary, which takes the accent out of button fills entirely.

Surface

#0f1115#161618. The old base carried a cool cast that read as "someone's dark theme" rather than as dark.

The elevation ramp is re-derived off it with the same +8/+8 deltas but a neutral hue — the old #171a21/#1f232c sat only +2 above the new base, so card-against-page would have collapsed into a step nobody could see. Borders, muted text and the foreground ramp are neutralised to match; a blue-grey hairline on a neutral base is exactly the tint being removed.

Buttons

button-primary → near-white slab, dark text (16.5:1). button-ghost → its secondary half, same height/radius/padding, only the value inverts, so a pair reads as two of the same kind of object. The send disc takes the same treatment: one language for "press this", labelled or not.

Not pure #fff#f2f2f4 is already far past AA and pure white blooms on dark. The primary keeps a transparent 1px border so it shares a box with the ghost beside it instead of sitting 2px shorter.

The accent is now in no button at all. It survives where it is small and load-bearing: streaming caret, reference indices and their on-page rectangles, the unshipped-work count, on/off switches.

The agent mark

40px ring, 24px glyph. The glyph runs corner-to-corner in its viewBox, so inside a circle the diagonal sets the clearance: at 24-in-32 that was ~0.7px and the wand looked welded to the ring. 24-in-40 gives ~4.7px.

Verified by rendering the candidates side by side rather than by arithmetic — which also showed the glyph itself does not need redrawing.

Exit animations

Every overlay was a <Show>, so closing removed the node on the same tick and there was no frame for a transition to run in — they opened with a fade and vanished with a cut.

  • NavMenu is a native popover, so it gets enter and exit from CSS alone: transition-behavior: allow-discrete + @starting-style. Zero JS.
  • The rest cannot: presence.ts holds them mounted for the exit window and exposes a leaving flag. Used by ModelPicker, ShipBar's send menu, the mention menu and the onboarding guide.

Two things in presence.ts that had to be right, both caught by tests rather than by eye:

  1. The close is a createComputed, not a createEffect. A render effect runs after the pure computations, so <Show when={mounted()}> re-evaluated on close with leaving still false, unmounted, and only then flipped it — the menu blinked out and back before finally going.
  2. mounted derives straight from open, so opening is synchronous. Routing it through a signal cost a tick and ModelPicker's open-focus effect ran against a popover that did not exist yet, dropping focus on <body>. Its existing unit test caught that.

model-picker.test.tsx had encoded "closing removes it synchronously"; those two cases now assert the real contract — interactivity stops immediately, focus returns immediately, the node goes after the animation.

Also

  • Sending a message: the user's turn animates in at 10px/240ms instead of the shared 4px/180ms. It is the one moment the panel answers "did that send?", and 4px alongside a scroll was invisible.
  • Ship bar: vertical breathing room; the row was tight against the composer beneath it.

Gate

lint · typecheck · 1964 unit + integration · 56 e2e.

Note: a first e2e run failed 3 vision/scroll specs (complex-sites ×2, control-vision ×1). All six pass in isolation and the full suite is clean on re-run — flake under full-suite parallelism, unrelated to this diff.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Added smooth opening and closing animations for navigation, mention, model selection, onboarding, and sharing menus.
    • Menus now remain usable and visually consistent during dismissal, with focus returning promptly where applicable.
    • Updated button styling, spacing, colors, and overall side-panel visual design.
  • Bug Fixes

    • Improved overlay dismissal behavior to prevent abrupt disappearance and accidental interaction during transitions.
    • Refined message entrance animations and control sizing for a more polished interface.

…imations

Five UI reports from a design pass. The colour work supersedes the accent-tinted buttons from
#177 — the reference sent was a light-filled primary next to a dark outlined secondary, which
takes the accent out of button fills entirely.

## Surface

`#0f1115` -> `#161618`. The old base carried a cool cast that read as "someone's dark theme"
rather than as dark. The elevation ramp is re-derived off it with the same +8/+8 deltas but a
neutral hue — the old `#171a21`/`#1f232c` sat only +2 above the new base, so card-against-page
would have collapsed into a step nobody could see. Borders, muted text and the foreground ramp
are neutralised to match; a blue-grey hairline on a neutral base is exactly the tint being
removed.

## Buttons

`button-primary` is a near-white slab with dark text (16.5:1). `button-ghost` becomes its
secondary half — same height, radius and padding rhythm, only the value inverts, so a pair reads
as two of the same kind of object. The send disc takes the same treatment: one language for
"press this", whether it is labelled or a 32px puck.

Not pure `#fff` — already far past AA at `#f2f2f4`, and pure white on a dark panel blooms. The
primary keeps a transparent 1px border so it shares a box with the ghost beside it instead of
sitting 2px shorter.

The accent is now in no button at all. It survives where it is small and load-bearing: the
streaming caret, reference indices and their on-page rectangles, the unshipped-work count, on/off
switches.

## The agent mark

40px ring, 24px glyph. The glyph runs corner-to-corner in its viewBox, so inside a CIRCLE its
diagonal is what sets the clearance: at 24-in-32 that was ~0.7px and the wand looked welded to
the ring. 24-in-40 gives ~4.7px. Verified by rendering the three candidates side by side rather
than by arithmetic alone — which also showed the glyph itself does not need redrawing.

## Exit animations

Every overlay was a `<Show>`, so closing removed the node on the same tick and there was no frame
for a transition to run in — they opened with a fade and vanished with a cut.

- `NavMenu` is a native popover, so it gets enter AND exit from CSS alone:
  `transition-behavior: allow-discrete` + `@starting-style`. Zero JS.
- The rest cannot: `presence.ts` holds them mounted for the exit window and exposes a `leaving`
  flag. Used by ModelPicker, ShipBar's send menu, the mention menu and the onboarding guide.

Two things that had to be right in `presence.ts`, both caught by tests rather than by eye:
- The close is a `createComputed`, not a `createEffect`. A render effect runs after the pure
  computations, so `<Show when={mounted()}>` re-evaluated on close with `leaving` still false,
  unmounted, and only then flipped it — the menu blinked out and back before finally going.
- `mounted` derives straight from `open`, so opening is synchronous. Routing it through a signal
  cost a tick and ModelPicker's open-focus effect ran against a popover that did not exist yet,
  dropping focus on `<body>`. Its existing unit test caught that.

`model-picker.test.tsx` had encoded "closing removes it synchronously"; those two cases now
assert the real contract — interactivity stops immediately, focus returns immediately, the node
goes after the animation.

## Sending a message

The user's own turn animates in at 10px/240ms instead of the shared 4px/180ms. It is the one
moment the panel has to answer "did that send?", and 4px alongside a scroll was invisible.

## Ship bar

Vertical breathing room: the row was tight against the composer beneath it.

Gate: lint, typecheck, 1964 unit + integration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Too big: expected string to have <=250 characters at "tone_instructions"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

The side panel now keeps overlays mounted during exit animations. It adds shared presence handling, new popover and message animations, revised neutral styling tokens, updated button styles, and related tests.

Changes

Sidepanel presence and visual updates

Layer / File(s) Summary
Presence lifecycle primitive
src/entrypoints/sidepanel/components/presence.ts, test/unit/presence.test.ts
Adds createPresence with mounted and leaving states, exit timing, reopening cancellation, cleanup, and unit tests.
Overlay presence integration
src/entrypoints/sidepanel/App.tsx, src/entrypoints/sidepanel/components/{Onboarding,ShipBar}.tsx, src/entrypoints/sidepanel/components/chat/{Composer,MentionMenu,ModelPicker}.tsx, test/unit/model-picker.test.tsx
Updates onboarding, send, mention, and model-picker overlays to remain mounted during dismissal and apply leaving state.
Overlay and message animations
src/entrypoints/sidepanel/main.scss, src/entrypoints/sidepanel/components/{NavMenu,Onboarding,ShipBar}.scss, src/entrypoints/sidepanel/components/chat/{MentionMenu,ModelPicker,Message}.scss
Adds entry and exit transitions, discrete popover display transitions, pointer-event disabling, and message-entry animation rules.
Shared visual system and layout updates
src/styles/_tokens.scss, src/styles/_mixins.scss, src/entrypoints/sidepanel/components/Onboarding.scss, src/entrypoints/sidepanel/components/{PreStart,ShipBar}.scss, src/entrypoints/sidepanel/components/chat/EmptyState.scss
Revises neutral colors, elevation, button styling, CTA colors, spacing, and control-size token usage.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Trigger
  participant createPresence
  participant Overlay
  participant CSS
  Trigger->>createPresence: close overlay
  createPresence-->>Overlay: keep mounted and set leaving
  Overlay->>CSS: apply exit animation
  CSS-->>createPresence: complete exit window
  createPresence-->>Overlay: unmount overlay
Loading

Possibly related PRs

Suggested reviewers: sebyx07, ivndev001

Poem

I hop through menus, smooth and bright,
They stay mounted through the night.
Fade and pop, then softly go,
Solid buttons steal the show.
Tests keep watch as timers flee—
A tidy trail for bunny me.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the pull request's main visual design and exit animation changes.
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
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/monochrome-surface-and-motion

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
src/styles/_mixins.scss (1)

157-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Update the button-primary documentation.

The comment above this mixin still describes an accented, unfilled control, a 10% wash, and accent-colored text. Lines 158-162 now use var(--dz-solid) and var(--dz-solid-fg). Rewrite the rationale to document the current solid treatment.

🤖 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 `@src/styles/_mixins.scss` around lines 157 - 162, Update the documentation
comment for the button-primary mixin to describe its current solid treatment
using var(--dz-solid) and var(--dz-solid-fg), replacing the outdated accented
unfilled, 10% wash, and accent-text rationale. Preserve the existing
transparent-border alignment explanation.
src/entrypoints/sidepanel/components/PreStart.scss (1)

25-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align the agent-mark documentation with the 40px token.

Both changed wrappers now use tokens.$dz-control-lg, which is 40px. The nearby comments still describe a 32px mark and an obsolete rejection of the 40px size.

  • src/entrypoints/sidepanel/components/PreStart.scss#L25-L26: change the “same 32px” description to “same 40px”.
  • src/entrypoints/sidepanel/components/chat/EmptyState.scss#L34-L35: rewrite the sizing rationale to describe the current 40px ring and 24px glyph.
🤖 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 `@src/entrypoints/sidepanel/components/PreStart.scss` around lines 25 - 26,
Update the comments at src/entrypoints/sidepanel/components/PreStart.scss:25-26
and src/entrypoints/sidepanel/components/chat/EmptyState.scss:34-35 to match the
current sizing: describe the PreStart mark as 40px, and document the EmptyState
sizing as a 40px ring containing a 24px glyph. No style changes are needed.
🤖 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 `@src/entrypoints/sidepanel/components/chat/MentionMenu.scss`:
- Around line 19-24: Update MentionMenu’s rendering logic around the visible-row
calculation and the <Show> condition so the .dz-mention listbox remains mounted
whenever props.leaving is true, even when shown() is empty. Preserve the last
rendered mention rows during this exit window so the .is-leaving animation can
run, while retaining the existing empty-state behavior once leaving completes.

In `@src/entrypoints/sidepanel/components/chat/Message.scss`:
- Around line 12-13: Insert a blank line immediately before the double-slash
comment preceding the dz-rise animation declaration, preserving the existing
animation rule and comment content.

In `@src/entrypoints/sidepanel/components/chat/ModelPicker.tsx`:
- Around line 194-201: Make the leaving popovers inert before delayed unmount:
in src/entrypoints/sidepanel/components/chat/ModelPicker.tsx lines 194-201,
apply the inert attribute to the popover when presence.leaving() is true;
likewise, in src/entrypoints/sidepanel/components/ShipBar.tsx lines 101-102,
apply inert to the send menu while sendPresence.leaving() is true. Add coverage
confirming Tab cannot focus either still-mounted menu during its leaving
transition.

In `@src/entrypoints/sidepanel/components/NavMenu.scss`:
- Around line 84-111: Update both transition declarations in the native popover
styles, including the base state and &:popover-open, to add overlay with
allow-discrete alongside display. Preserve the existing opacity, transform,
timing, and starting-style behavior so the popover remains in the top layer
through its exit transition.

In `@src/entrypoints/sidepanel/components/Onboarding.tsx`:
- Around line 81-82: Update the Onboarding component to read props.leaving,
apply the is-leaving class to its root dialog, and mark the leaving subtree
inert while the delayed unmount is pending. Preserve the existing active-modal
behavior when leaving is false.

In `@src/styles/_mixins.scss`:
- Around line 157-180: Add the required blank lines in the button styles around
the declarations and double-slash comment identified by Stylelint, specifically
within the primary button rules and the button-ghost mixin near _button-base.
Preserve all styling values and behavior while satisfying
declaration-empty-line-before and double-slash-comment-empty-line-before.

---

Nitpick comments:
In `@src/entrypoints/sidepanel/components/PreStart.scss`:
- Around line 25-26: Update the comments at
src/entrypoints/sidepanel/components/PreStart.scss:25-26 and
src/entrypoints/sidepanel/components/chat/EmptyState.scss:34-35 to match the
current sizing: describe the PreStart mark as 40px, and document the EmptyState
sizing as a 40px ring containing a 24px glyph. No style changes are needed.

In `@src/styles/_mixins.scss`:
- Around line 157-162: Update the documentation comment for the button-primary
mixin to describe its current solid treatment using var(--dz-solid) and
var(--dz-solid-fg), replacing the outdated accented unfilled, 10% wash, and
accent-text rationale. Preserve the existing transparent-border alignment
explanation.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 13c4ac7d-f84d-478b-b497-17bc1a2d5081

📥 Commits

Reviewing files that changed from the base of the PR and between e92a190 and 38eba87.

📒 Files selected for processing (20)
  • src/entrypoints/sidepanel/App.tsx
  • src/entrypoints/sidepanel/components/NavMenu.scss
  • src/entrypoints/sidepanel/components/Onboarding.scss
  • src/entrypoints/sidepanel/components/Onboarding.tsx
  • src/entrypoints/sidepanel/components/PreStart.scss
  • src/entrypoints/sidepanel/components/ShipBar.scss
  • src/entrypoints/sidepanel/components/ShipBar.tsx
  • src/entrypoints/sidepanel/components/chat/Composer.tsx
  • src/entrypoints/sidepanel/components/chat/EmptyState.scss
  • src/entrypoints/sidepanel/components/chat/MentionMenu.scss
  • src/entrypoints/sidepanel/components/chat/MentionMenu.tsx
  • src/entrypoints/sidepanel/components/chat/Message.scss
  • src/entrypoints/sidepanel/components/chat/ModelPicker.scss
  • src/entrypoints/sidepanel/components/chat/ModelPicker.tsx
  • src/entrypoints/sidepanel/components/presence.ts
  • src/entrypoints/sidepanel/main.scss
  • src/styles/_mixins.scss
  • src/styles/_tokens.scss
  • test/unit/model-picker.test.tsx
  • test/unit/presence.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Test (e2e)
  • GitHub Check: Build extension
🧰 Additional context used
📓 Path-based instructions (5)
src/**/*.scss

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.scss: Co-locate component styles with their component and scope styles to a root class using BEM-like child names.
Do not hardcode hex colors or pixel values when a token exists; use the shared SCSS tokens.

Files:

  • src/entrypoints/sidepanel/components/chat/EmptyState.scss
  • src/entrypoints/sidepanel/components/NavMenu.scss
  • src/entrypoints/sidepanel/components/PreStart.scss
  • src/entrypoints/sidepanel/components/Onboarding.scss
  • src/entrypoints/sidepanel/components/chat/MentionMenu.scss
  • src/entrypoints/sidepanel/components/ShipBar.scss
  • src/entrypoints/sidepanel/components/chat/ModelPicker.scss
  • src/styles/_tokens.scss
  • src/entrypoints/sidepanel/components/chat/Message.scss
  • src/entrypoints/sidepanel/main.scss
  • src/styles/_mixins.scss
src/entrypoints/sidepanel/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Keep UI exclusively in the side panel, which must remain CSP-clean.

Files:

  • src/entrypoints/sidepanel/components/Onboarding.tsx
  • src/entrypoints/sidepanel/components/ShipBar.tsx
  • src/entrypoints/sidepanel/components/presence.ts
  • src/entrypoints/sidepanel/components/chat/ModelPicker.tsx
  • src/entrypoints/sidepanel/components/chat/MentionMenu.tsx
  • src/entrypoints/sidepanel/components/chat/Composer.tsx
  • src/entrypoints/sidepanel/App.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not use remote code or eval; SolidJS must be prebuilt to static JavaScript.

Files:

  • src/entrypoints/sidepanel/components/Onboarding.tsx
  • test/unit/presence.test.ts
  • src/entrypoints/sidepanel/components/ShipBar.tsx
  • src/entrypoints/sidepanel/components/presence.ts
  • src/entrypoints/sidepanel/components/chat/ModelPicker.tsx
  • src/entrypoints/sidepanel/components/chat/MentionMenu.tsx
  • src/entrypoints/sidepanel/components/chat/Composer.tsx
  • src/entrypoints/sidepanel/App.tsx
  • test/unit/model-picker.test.tsx
src/entrypoints/sidepanel/components/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

src/entrypoints/sidepanel/components/*.tsx: Use one component per .tsx file, with a co-located .scss file sharing the same basename.
Keep business logic out of components; components should render and dispatch only, with logic in the agent, DOM, MCP, or changeset modules.
Use signals or stores for state, never prop-drill more than one level, derive values with createMemo, perform side effects with createEffect, and do not manipulate the DOM manually.

Files:

  • src/entrypoints/sidepanel/components/Onboarding.tsx
  • src/entrypoints/sidepanel/components/ShipBar.tsx
src/styles/_tokens.scss

📄 CodeRabbit inference engine (CLAUDE.md)

Define shared colors, spacing, and radius tokens in src/styles/_tokens.scss.

Files:

  • src/styles/_tokens.scss
🪛 Stylelint (17.14.0)
src/entrypoints/sidepanel/components/chat/Message.scss

[error] 12-12: Expected empty line before comment (scss/double-slash-comment-empty-line-before)

(scss/double-slash-comment-empty-line-before)

src/styles/_mixins.scss

[error] 159-159: Expected empty line before comment (scss/double-slash-comment-empty-line-before)

(scss/double-slash-comment-empty-line-before)


[error] 157-157: Expected empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)


[error] 180-180: Expected empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

🔇 Additional comments (12)
src/entrypoints/sidepanel/components/presence.ts (1)

1-68: LGTM!

test/unit/presence.test.ts (1)

1-92: LGTM!

src/entrypoints/sidepanel/components/chat/Composer.tsx (1)

14-14: LGTM!

Also applies to: 70-70, 162-168

src/entrypoints/sidepanel/components/chat/MentionMenu.tsx (1)

51-52: LGTM!

Also applies to: 69-69

src/styles/_tokens.scss (1)

23-61: LGTM!

Also applies to: 71-71, 197-199, 215-217, 282-282

src/entrypoints/sidepanel/main.scss (1)

82-118: LGTM!

src/entrypoints/sidepanel/components/Onboarding.scss (1)

5-15: LGTM!

Also applies to: 211-220

src/entrypoints/sidepanel/components/ShipBar.scss (1)

13-13: LGTM!

Also applies to: 81-85

src/entrypoints/sidepanel/components/chat/ModelPicker.scss (1)

53-59: LGTM!

src/entrypoints/sidepanel/components/chat/Message.scss (1)

15-21: LGTM!

src/styles/_mixins.scss (2)

181-190: LGTM!


272-280: LGTM!

Comment on lines +19 to +24
animation: dz-pop var(--dz-motion-fast) var(--dz-ease);

&.is-leaving {
animation: dz-pop-out var(--dz-motion-fast) var(--dz-ease-out) forwards;
pointer-events: none;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the mention listbox mounted during a no-match dismissal.

Composer closes presence when the filtered mention list becomes empty. MentionMenu.tsx:56-106 then also uses <Show when={shown().length > 0}>, which can remove .dz-mention before .is-leaving applies. Keep the listbox mounted while props.leaving is true, and retain the last rendered rows during the exit window.

🤖 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 `@src/entrypoints/sidepanel/components/chat/MentionMenu.scss` around lines 19 -
24, Update MentionMenu’s rendering logic around the visible-row calculation and
the <Show> condition so the .dz-mention listbox remains mounted whenever
props.leaving is true, even when shown() is empty. Preserve the last rendered
mention rows during this exit window so the .is-leaving animation can run, while
retaining the existing empty-state behavior once leaving completes.

Comment on lines +12 to 13
// Each turn arrives rather than appearing.
animation: dz-rise var(--dz-motion-base) var(--dz-ease);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the blank line required by Stylelint.

Insert an empty line before the comment at Line 12. This resolves scss/double-slash-comment-empty-line-before.

🧰 Tools
🪛 Stylelint (17.14.0)

[error] 12-12: Expected empty line before comment (scss/double-slash-comment-empty-line-before)

(scss/double-slash-comment-empty-line-before)

🤖 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 `@src/entrypoints/sidepanel/components/chat/Message.scss` around lines 12 - 13,
Insert a blank line immediately before the double-slash comment preceding the
dz-rise animation declaration, preserving the existing animation rule and
comment content.

Source: Linters/SAST tools

Comment on lines +194 to +201
<Show when={presence.mounted()}>
{/* The popover wraps the search field AND the menu. The field is deliberately a SIBLING
of `role="menu"`, not a child: a menu may only own `menuitem*` nodes, so a textbox
inside one is invalid ARIA. The keydown handler is attached to BOTH interactive
children rather than to this wrapper: arrows must drive the list whether the cursor
is in the field or focus has moved onto a row, and a handler on a plain div is a
static-element interaction. */}
<div class="dz-modelpicker__popover">
<div class="dz-modelpicker__popover" classList={{ 'is-leaving': presence.leaving() }}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make leaving menus inert before their delayed unmount.

createPresence keeps these focusable subtrees in the DOM after the open state becomes false. is-leaving does not prevent keyboard focus. In ModelPicker, Tab after closeMenu() can advance into the still-mounted search input. Apply inert while leaving and add coverage for the Tab path.

  • src/entrypoints/sidepanel/components/chat/ModelPicker.tsx#L194-L201: set the popover inert while presence.leaving() is true.
  • src/entrypoints/sidepanel/components/ShipBar.tsx#L101-L102: set the send menu inert while sendPresence.leaving() is true.
📍 Affects 2 files
  • src/entrypoints/sidepanel/components/chat/ModelPicker.tsx#L194-L201 (this comment)
  • src/entrypoints/sidepanel/components/ShipBar.tsx#L101-L102
🤖 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 `@src/entrypoints/sidepanel/components/chat/ModelPicker.tsx` around lines 194 -
201, Make the leaving popovers inert before delayed unmount: in
src/entrypoints/sidepanel/components/chat/ModelPicker.tsx lines 194-201, apply
the inert attribute to the popover when presence.leaving() is true; likewise, in
src/entrypoints/sidepanel/components/ShipBar.tsx lines 101-102, apply inert to
the send menu while sendPresence.leaving() is true. Add coverage confirming Tab
cannot focus either still-mounted menu during its leaving transition.

Comment on lines +84 to +111
// Enter AND exit, with no JS at all. `display` is a discrete property, so a popover normally
// snaps between `none` and `flex` and any transition on it is skipped — which is why this
// menu opened and closed with a hard cut. `transition-behavior: allow-discrete` makes the
// browser hold `display` until the rest of the transition finishes, and `@starting-style`
// supplies the frame-zero values the element has never had. Chrome 117+; this surface is
// Chrome-only, and the `presence.ts` dance exists precisely for the menus that are NOT
// popovers and therefore cannot do this.
opacity: 0;
transform: scale(0.97);
transition:
opacity var(--dz-motion-fast) var(--dz-ease-out),
transform var(--dz-motion-fast) var(--dz-ease-out),
display var(--dz-motion-fast) allow-discrete;

&:popover-open {
display: flex;
flex-direction: column;
opacity: 1;
transform: none;
transition:
opacity var(--dz-motion-base) var(--dz-ease),
transform var(--dz-motion-base) var(--dz-ease),
display var(--dz-motion-base) allow-discrete;

@starting-style {
opacity: 0;
transform: scale(0.97);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- target file outline ---'
ast-grep outline src/entrypoints/sidepanel/components/NavMenu.scss 2>/dev/null || true
printf '%s\n' '--- target source ---'
sed -n '1,150p' src/entrypoints/sidepanel/components/NavMenu.scss
printf '%s\n' '--- popover references ---'
rg -n -C 4 'popover|transition-behavior|starting-style|overlay' src/entrypoints/sidepanel/components src/entrypoints 2>/dev/null | head -240
printf '%s\n' '--- browser support/config references ---'
rg -n -i -C 2 'chrome|browserslist|browser.?support|sidepanel' package.json .browserslistrc browserslist* vite.config.* webpack.config.* 2>/dev/null | head -200 || true

Repository: developerz-ai/ai-designer

Length of output: 29136


🌐 Web query:

MDN Popover API Using transition overlay allow-discrete top layer exit transition

💡 Result:

To enable smooth entry and exit transitions for elements in the top layer (such as those using the Popover API), you must use a combination of modern CSS features: transition-behavior, the overlay property, and @starting-style [1][2][3]. Because popovers toggle between display: none and a visible state, their exit animation is typically cut off immediately when they are removed from the DOM or hidden [4][2][5]. The following configuration ensures the exit animation completes before the element is hidden or removed from the top layer [6][7][8]: 1. transition-behavior: allow-discrete: This property enables transitions on discrete properties like display and overlay, which are not normally animatable [6][7][4]. 2. display: Including display in your transition list—paired with allow-discrete—ensures the element remains visible (set to display: block) for the full duration of the exit animation [6][4][3]. 3. overlay: Including overlay in your transition list—also paired with allow-discrete—defers the removal of the element from the top layer until the animation finishes [6][7][2]. Without this, the element might be removed from the top layer prematurely, potentially ending the animation abruptly or causing layout issues [7][1][5]. 4. @starting-style: This rule allows you to define the initial state of the popover for entry animations, providing a starting point to animate from as the element enters the top layer [1][8][3]. Example CSS implementation [6][2]: [popover] { /* Final state of the exit animation / opacity: 0; transform: scale(0.9); / Transition for all properties, applying allow-discrete to discrete ones / transition: opacity 0.3s, transform 0.3s, display 0.3s allow-discrete, overlay 0.3s allow-discrete; } [popover]:popover-open { opacity: 1; transform: scale(1); } / Entry animation start state */ @starting-style { [popover]:popover-open { opacity: 0; transform: scale(0.9); } } When using shorthand transition properties, ensure transition-behavior: allow-discrete is declared after the shorthand or explicitly applied to the discrete properties to prevent the shorthand from overriding the behavior [4][2][9].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- NavMenu files ---'
fd -i 'NavMenu' .
printf '%s\n' '--- native popover markup and controls ---'
rg -n -i -C 8 'popover|showPopover|hidePopover|togglePopover' src/entrypoints/sidepanel --glob '*NavMenu*' --glob '*.tsx' --glob '*.ts' | head -240
printf '%s\n' '--- package/browser metadata ---'
sed -n '1,120p' package.json
printf '%s\n' '--- focused transition verifier ---'
python3 - <<'PY'
from pathlib import Path
import re

path = Path("src/entrypoints/sidepanel/components/NavMenu.scss")
text = path.read_text()
blocks = re.findall(r"transition:\s*(.*?);", text, flags=re.S)
print(f"transition declarations: {len(blocks)}")
for index, block in enumerate(blocks, 1):
    normalized = " ".join(block.split())
    print(f"{index}: {normalized}")
    print("  has display allow-discrete:", bool(re.search(r"\bdisplay\b[^,;]*\ballow-discrete\b", normalized)))
    print("  has overlay allow-discrete:", bool(re.search(r"\boverlay\b[^,;]*\ballow-discrete\b", normalized)))
PY

Repository: developerz-ai/ai-designer

Length of output: 28005


🌐 Web query:

CSS Popover API overlay transition both base state and :popover-open state exit animation top layer authoritative

💡 Result:

To animate both the entry and exit states of a CSS popover (which renders in the top layer), you must use a combination of three modern CSS features: display, overlay, and transition-behavior: allow-discrete [1][2][3]. Because popovers toggle between display: none and a visible state, and are promoted to the top layer when open, standard transitions are insufficient on their own [4][3][5]. You must include display and overlay in your transition list to keep the element visible and in the top layer for the duration of the exit animation [1][2][4]. Key implementation requirements: 1. Base State ([popover]): Define your exit (closed) styles here. This is where you set the initial properties for the animation (e.g., opacity: 0) and configure the transitions [6][3][5]. 2. Open State ([popover]:popover-open): Define the final visible styles here (e.g., opacity: 1) [1][6][5]. 3. @starting-style: Use this block to define the starting styles for the entry animation (e.g., opacity: 0). It must be placed after the open state selector or inside it to ensure correct cascading [6][3][5]. 4. Transition Configuration: You must add display and overlay to your transition property and include transition-behavior: allow-discrete [1][4][7]. Example CSS structure: [popover] { /* Exit/Closed state styles / opacity: 0; transition: opacity 0.3s, display 0.3s allow-discrete, overlay 0.3s allow-discrete; } [popover]:popover-open { / Open state styles / opacity: 1; @starting-style { / Starting state for entry */ opacity: 0; } } The overlay property is crucial because it ensures the popover is not removed from the top layer prematurely, allowing the exit animation to complete [2][4][3]. Failure to include display in the transition list will cause the popover to immediately set display: none upon closing, cutting off any exit transition [1][4][7].

Citations:


Keep the native popover in the top layer during exit.

display ... allow-discrete does not delay top-layer removal. Add overlay ... allow-discrete to both transition declarations so the opacity and transform transitions can complete.

🤖 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 `@src/entrypoints/sidepanel/components/NavMenu.scss` around lines 84 - 111,
Update both transition declarations in the native popover styles, including the
base state and &:popover-open, to add overlay with allow-discrete alongside
display. Preserve the existing opacity, transform, timing, and starting-style
behavior so the popover remains in the top layer through its exit transition.

Comment on lines +81 to +82
/** True while the guide is animating out — App keeps it mounted for that window. */
leaving?: boolean;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Apply props.leaving to the onboarding root.

App passes this prop, but Onboarding never reads it. The dialog therefore cannot receive is-leaving for its exit animation. It also remains an active modal during the exit window. Apply the leaving class and make the leaving subtree inert before the delayed unmount.

🤖 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 `@src/entrypoints/sidepanel/components/Onboarding.tsx` around lines 81 - 82,
Update the Onboarding component to read props.leaving, apply the is-leaving
class to its root dialog, and mark the leaving subtree inert while the delayed
unmount is pending. Preserve the existing active-modal behavior when leaving is
false.

Comment thread src/styles/_mixins.scss
Comment on lines +157 to +180
padding: 0 t.$dz-space-5;
background: var(--dz-solid);
// A transparent border, not `none`: it holds the same box as the ghost button beside it, so a
// primary/secondary pair lines up on the baseline instead of the primary sitting 2px shorter.
border: t.$dz-border-width solid transparent;
color: var(--dz-solid-fg);
font-weight: t.$dz-weight-semibold;

// Deepen the wash rather than brightening a fill — `filter: brightness` on a translucent
// surface shifts whatever shows through it too, which is how a hover ends up tinting the card
// behind the button.
&:not(:disabled):hover {
background: var(--dz-accent-fill-strong);
border-color: var(--dz-accent);
background: var(--dz-solid-hover);
}

&:not(:disabled):active {
background: var(--dz-accent-fill-strong);
border-color: var(--dz-accent);
color: var(--dz-fg-strong);
background: var(--dz-solid);
transform: translateY(t.rem(1));
}
}

// Quiet action — transparent until hovered, subtle outline for shape.
// The secondary half of the pair — a recessed slab with a visible outline, not a bare label.
// It sits beside `button-primary` and has to read as the same KIND of object at a glance: same
// height, same radius, same padding rhythm; only the value inverts.
@mixin button-ghost($height: t.$dz-control-md, $radius: t.$dz-radius, $border: true) {
@include _button-base($height, $radius);
padding: 0 t.$dz-space-3;
background: transparent;
padding: 0 t.$dz-space-5;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the reported Stylelint spacing errors.

Stylelint reports declaration-empty-line-before on Lines 157 and 180 and double-slash-comment-empty-line-before on Line 159. Add the required blank lines.

Suggested spacing fix
   `@include` _button-base($height, $radius);
+
   padding: 0 t.$dz-space-5;
   background: var(--dz-solid);
+
   // A transparent border...
...
   `@include` _button-base($height, $radius);
+
   padding: 0 t.$dz-space-5;
🧰 Tools
🪛 Stylelint (17.14.0)

[error] 159-159: Expected empty line before comment (scss/double-slash-comment-empty-line-before)

(scss/double-slash-comment-empty-line-before)


[error] 157-157: Expected empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)


[error] 180-180: Expected empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

🤖 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 `@src/styles/_mixins.scss` around lines 157 - 180, Add the required blank lines
in the button styles around the declarations and double-slash comment identified
by Stylelint, specifically within the primary button rules and the button-ghost
mixin near _button-base. Preserve all styling values and behavior while
satisfying declaration-empty-line-before and
double-slash-comment-empty-line-before.

Source: Linters/SAST tools

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.

2 participants