feat(human): animated background, responsive layout, collapsible chat panel#2957
feat(human): animated background, responsive layout, collapsible chat panel#2957graycyrus wants to merge 5 commits into
Conversation
…yout, collapsible chat (tinyhumansai#2955) - Add 3 CSS-only animated gradient blobs drifting behind the mascot - Replace absolute positioning with flex layout for responsive behavior - Add collapsible chat sidebar with toggle button and localStorage persistence - Move top controls (Push to Talk, Send to Meeting) into a proper flex bar - Add i18n keys for chat toggle across all 14 locales - Add 7 new tests for collapsible chat panel behavior Closes tinyhumansai#2955
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
Nice direction overall — the flex restructure is a real improvement over the absolute-positioned layout, the localStorage persistence mirrors the existing speakReplies pattern cleanly, and the test coverage on the collapse/toggle behavior is solid. A couple of things need attention before this is ready, plus a few smaller notes.
Change summary
| Area | Change | Notes |
|---|---|---|
HumanPage.tsx |
Flex layout, animated blobs, collapsible chat | Core logic sound; collapse doesn't reclaim space (see inline) |
HumanPage.test.tsx |
7 new tests | Good coverage, but assert on class presence, not computed layout |
tailwind.config.js |
3 blob keyframes | Color tokens (accent-lavender, accent-mint, primary-*) all resolve |
| i18n (en + 14 chunks) | human.openChat, human.collapseChat |
Keys present, but all locales got English values (see below) |
Blocking
Collapsing the chat panel doesn't reclaim the space. The panel slides out via translate-x-full, but the <aside> keeps shrink-0 w-[420px] in the flex row. A CSS transform is purely visual — the layout box still reserves 420px. So when collapsed, the panel slides off-screen but leaves a 420px empty gap, and the mascot stays in its narrower flex track instead of expanding. That directly contradicts the stated goal ("Mascot fills available space, chat panel shrinks gracefully"). Details inline.
Issue alignment (#2955)
Issue #2955 specifies three breakpoints: large (>1024px) side-by-side, medium (768–1024px) slide-over, small (<768px) bottom sheet / full overlay. The implementation has no responsive variants (sm:/md:/lg:) at all — it's a single fixed layout with max-w-[90vw] on the panel. The mascot doesn't shrink on small screens and there's no bottom-sheet behavior. Since the PR is marked Closes #2955, either implement the remaining breakpoints or narrow the issue's scope so it isn't auto-closed with criteria unmet.
Minor
- i18n placeholders: all 14 locale chunks received the literal English strings ("Open chat" / "Collapse chat").
i18n:checkpasses because the keys exist, but ar/bn/de/hi/ko/ru/zh-CN etc. will render English. Fine as a placeholder if intentional — worth queueing real translations so non-English users don't see untranslated UI.
Happy to re-review once the collapse layout reclaims space. The rest is close.
| {/* Chat sidebar — collapsible panel */} | ||
| <aside | ||
| data-testid="human-chat-panel" | ||
| className={`shrink-0 w-[420px] max-w-[90vw] flex flex-col transition-transform duration-300 ease-in-out ${ |
There was a problem hiding this comment.
[major] Collapsing here only applies translate-x-full, but the aside keeps shrink-0 w-[420px] in the flex row. A transform doesn't change layout flow, so the 420px is still reserved — the panel slides off-screen and leaves a dead 420px gap on the right, and the mascot's flex-1 track never grows to fill it. The collapse looks broken rather than reclaiming space.
To actually reclaim the width, collapse the box itself, e.g. animate the width:
className={`shrink-0 overflow-hidden flex flex-col transition-all duration-300 ease-in-out ${
chatOpen ? 'w-[420px] max-w-[90vw]' : 'w-0'
}`}(or unmount the panel / use a negative margin). The current tests pass because jsdom asserts on class presence, not computed layout, so they won't catch this.
There was a problem hiding this comment.
Fixed in d698a22. The aside now animates its own box width to w-0 (with overflow-hidden) on collapse instead of translate-x-full, so the layout box no longer reserves 420px and the mascot's flex-1 track reclaims the freed space. Also took the chance to add the three #2955 breakpoints: small (<md) full-width slide-over overlay, medium (md,<lg) narrower slide-over, large (lg+) side-by-side. Tests updated to assert the width-based collapse and responsive classes.
| <div className="absolute inset-0 bg-stone-100 dark:bg-neutral-950 overflow-hidden flex flex-col"> | ||
| {/* ── Animated background blobs (CSS-only, z-0) ── */} | ||
| <div className="pointer-events-none absolute inset-0 overflow-hidden" aria-hidden="true"> | ||
| <div className="absolute -top-1/4 -left-1/4 w-[60%] h-[60%] rounded-full bg-primary-400/10 dark:bg-primary-500/[0.07] blur-3xl animate-blob-drift-1" /> |
There was a problem hiding this comment.
[minor] These blobs animate infinitely (20–30s). The global prefers-reduced-motion block in app/src/index.css only covers the cmd-palette classes, so motion-sensitive users will still see all three drifting. Consider adding the animate-blob-drift-* classes to that media block, or use Tailwind's motion-reduce:animate-none variant on each blob.
There was a problem hiding this comment.
Fixed in d698a22 — added motion-reduce:animate-none to all three blob divs so prefers-reduced-motion users don't see the infinite drift. New test asserts the class is present on each blob.
Resolve i18n chunk modify/delete conflicts: main retired the chunked locale layout (chunks/<locale>-N.ts) for flat per-locale files. Carry the PR's two new keys (human.openChat, human.collapseChat) into the flat files with real translations for all 13 non-English locales instead of the English placeholders the PR had added to the deleted chunk files.
… reduced-motion Addresses @sanil-23 review on HumanPage.tsx: - [blocking] Collapsing the chat panel only applied translate-x-full, which is purely visual — the <aside> kept shrink-0 w-[420px] so the layout box still reserved 420px and the mascot's flex-1 track never grew. Animate the box width to w-0 (with overflow-hidden) instead so the panel collapses its own width and the mascot reclaims the freed space. - [tinyhumansai#2955 alignment] Add the three responsive breakpoints the issue specifies: small (<md) full-width slide-over overlay, medium (md,<lg) narrower slide-over, large (lg+) side-by-side in the flex row. Below lg the panel is an absolute overlay so it no longer squeezes the mascot. - [minor] Add motion-reduce:animate-none to the three background blobs so prefers-reduced-motion users don't see the infinite 20-30s drift. Tests updated to assert width-based collapse (w-0) and the responsive + reduced-motion classes instead of the removed translate-* classes.
|
Pushed d698a22 addressing the review:
Also resolved the merge conflict against current main: main retired the chunked Local checks: |
PR tinyhumansai#3075 changed the 'app.openhumanLink.notifications.sent' copy in en.ts from a curly apostrophe (didn’t) to a straight one (didn't) without updating OpenhumanLinkModal.notifications.test.tsx, leaving 2 assertions matching the old curly form. Those tests fail on main and were pulled into this branch by the main merge, failing the Frontend Coverage (Vitest) job — which produces no lcov on failure — and thereby blocking the Coverage Gate. Make the assertions apostrophe-agnostic ([' or ’]) so they match the shipped copy and stay robust to future typography changes.
…lock
inference_provider_admin_round22_raw_coverage_e2e.rs runs its #[tokio::test]
cases on parallel libtest threads (default). Three of them mutate the same
process-global env vars (OPENHUMAN_WORKSPACE, OPENHUMAN_OLLAMA_BASE_URL) via
EnvVarGuard, so a sibling test could clobber the workspace/config path while
provider_admin_model_listing_... was between set and read — surfacing the wrong
config and flaking 'assertion failed: object_error.contains("nested provider
failure")'. This intermittently fails Rust Core Coverage (cargo-llvm-cov),
whose instrumented (slower) build widens the race window, which in turn blocks
the Coverage Gate that depends on it.
Adopt the repo's existing env_lock() pattern (channels_round26,
tools_agent_credentials_state): an OnceLock<Mutex<()>> each env-mutating test
holds for its whole body, making the set -> read -> restore cycle atomic
regardless of --test-threads. Pre-existing flake (test added in tinyhumansai#3023, pulled
in by the main merge); not introduced by this PR's frontend changes.
Verified: the file's 5 tests pass across repeated runs locally.
2 participants
Summary
Redesigns the
/humanpage to feel alive and responsive:@keyframesdrift animations (20-30s cycles). Pure CSS, zero JS overhead. Layered behind mascot at z-0.absolute inset-y-0 left-0 right-[436px]with a flex layout. Mascot fills available space, chat panel shrinks gracefully.transition-transform. State persisted tolocalStorage. Panel header with collapse chevron.Changes
app/src/features/human/HumanPage.tsxapp/src/features/human/HumanPage.test.tsxapp/tailwind.config.jsapp/src/lib/i18n/en.ts+ 14 chunk fileshuman.openChat,human.collapseChatkeysTest plan
pnpm typecheck— cleanpnpm lint— 0 errorspnpm format:check— cleanpnpm build— cleanpnpm i18n:check— 0 missing keys across all localesCloses #2955