fix(chat): move clear-animation lifecycle to the view layer#7092
Merged
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes.
More templates
algoliasearch-helper
instantsearch-ui-components
instantsearch.css
instantsearch.js
react-instantsearch
react-instantsearch-core
react-instantsearch-nextjs
react-instantsearch-router-nextjs
vue-instantsearch
commit: |
Clicking "Clear" relied on the opacity `transitionend` event to actually remove the messages. Under `prefers-reduced-motion: reduce` the global stylesheet sets `transition: none !important`, so the transition — and therefore `transitionend` — never fires: the messages looked cleared (opacity jumped to 0) but stayed in state and in sessionStorage, and reappeared on reload. `isClearing` was also left stuck at `true`. The root cause is a layering issue: the headless connector owned a view-only concern. `clearMessages` only set `isClearing` (a CSS-driven fade), and the real clear (`setMessages([])`, conversation reset, error and feedback reset) ran from `onClearTransitionEnd` — a DOM-event-shaped method on the connector's public render state. The connector should not know about CSS transitions. Move the fade-out choreography into the shared `Chat` UI component, which is the common owner of the header (the Clear button) and the messages (the fading content). It owns `isClearing` via an injected `useState` hook — supplied through the existing `Hooks` seam, the same way `memo` already is, so each flavor passes its own runtime's hook (React → react, JS → preact/hooks); Vue has no chat. When the user prefers reduced motion (or no state hook is supplied), the component commits the clear immediately instead of waiting for a transition that never fires. The connector now exposes a single synchronous `clearMessages` commit and no longer surfaces `isClearing` / `onClearTransitionEnd` on its render state. The React and JS widgets are updated to supply `useState` and to drop the removed fields. Tests: - connectChat: `clearMessages` clears messages, resets the conversation id, and is a no-op on empty — synchronously, with no transition step. - Chat component: fades out then commits on `transitionend`; commits immediately under reduced motion; commits immediately with no state hook (graceful degradation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
932df51 to
3c3cd59
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Moves the “clear conversation” lifecycle out of the headless connectChat connector and into the shared Chat UI component, fixing the reduced-motion case where the clear transition never ends (so the clear never commits).
Changes:
- Connector:
clearMessages()now synchronously commits the clear and no longer exposesisClearing/onClearTransitionEndon render state. - UI component:
Chatowns the fade-out choreography via an injecteduseStatehook, committing immediately under reduced motion (or without a hook). - Wrappers/tests: React + InstantSearch.js widgets inject
useState; tests updated/added for the new clearing behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/react-instantsearch/src/widgets/Chat.tsx | Injects useState into createChatComponent; removes connector-driven clearing fields and updates canClear. |
| packages/instantsearch.js/src/widgets/chat/chat.tsx | Injects Preact useState into createChatComponent; removes connector-driven clearing fields and updates canClear. |
| packages/instantsearch.js/src/connectors/chat/connectChat.ts | Makes clearMessages() a synchronous commit; removes isClearing and onClearTransitionEnd from render state. |
| packages/instantsearch.js/src/connectors/chat/tests/connectChat-test.ts | Updates connector tests to match the synchronous clear behavior and removed render-state fields. |
| packages/instantsearch-ui-components/src/components/chat/Chat.tsx | Implements view-layer clearing state/transition orchestration with reduced-motion immediate commit fallback. |
| packages/instantsearch-ui-components/src/components/chat/tests/Chat.test.tsx | Adds tests for fade-out + transitionend commit, reduced-motion immediate commit, and “no hook” immediate commit. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback on the clear flow: - Drop the empty-messages early return in the connector's `clearMessages`. It also stops an in-flight stream and resets the error status and conversation id, which can be set even with no messages (e.g. a failed resume/reconnect) — guarding on message count alone could leave the chat stuck in an error state whose "New conversation" button (rendered on `status === 'error'`, independent of message count) no-ops. - Stop any in-flight stream eagerly when clear is clicked with the animation path: the view component now calls `stop()` immediately and defers only the message removal to the transition end, so the assistant stops responding right away rather than after the fade-out. Previously the connector stopped the stream synchronously on click; moving the lifecycle to the view layer had deferred that too. Tests: - connectChat: replace the now-obsolete "no-op on empty messages" test with one asserting clear exits the error state and rotates the conversation id even with no messages. - Chat component: add a test asserting clear stops streaming immediately and commits the removal on transition end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`clearMessages` mutated `messages`/`status` (which synchronously re-render via ChatState callbacks) before resetting `feedbackState` and the conversation id (which emit nothing). The re-render therefore captured a stale id and stale feedback, and nothing re-rendered afterward. Reorder so the non-reactive resets run first. Also trim narrative comments introduced earlier in this PR down to the non-obvious "why". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The clearing animation genuinely needs a state hook, so make `useState` a required injection rather than an optional one tolerated by a `noopUseState` fallback and a `useClearingState`/`hasStateHook` dance. Both flavor wrappers (React, instantsearch.js) already pass it, and we own the contract — the optionality bought nothing. This matches the existing precedent in createDisplayResultsToolComponent, which already requires `useMemo`. `memo` stays optional here; tightening it (and ChatMessages' optional memo plus its fallback) is a separate, pre-existing cleanup. Co-Authored-By: Claude Opus 4.8 (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.
Summary
Fixes a bug where clicking Clear in
<Chat>doesn't actually clear the conversation when the user hasprefers-reduced-motion: reduceenabled — the messages visually disappear but reappear on reload — and fixes the underlying layering issue that caused it.Root cause
The clear flow was transition-driven, and the headless connector owned a view-only concern:
clearMessages()inconnectChat.tsonly setisClearing, which adds a CSS class that fades the content out via an opacitytransition.setMessages([])(which also persists[]tosessionStorage), conversation-id reset, error/feedback reset — ran fromonClearTransitionEnd, a DOM-event-shaped method exposed on the connector's public render state, wired only to the messages'onTransitionEnd(gated onpropertyName === 'opacity').Under reduced motion the global stylesheet disables the transition:
With
transition: none, the opacity change is instant and notransitionendevent fires, soonClearTransitionEndnever runs: messages are never cleared from state orsessionStorage,isClearingstays stuck attrue, and the conversation comes back on reload.Fix — move the animation lifecycle into the view layer
Rather than special-casing reduced motion in the connector, this removes the layering violation:
clearMessagescommit and no longer surfacesisClearing/onClearTransitionEndon its render state. (Breaking change to the chat connector's render state — chat is a brand-new, unreleased feature.)ChatUI component owns the fade-out choreography. It's the common owner of the header (the Clear button) and the messages (the fading content), soisClearinglives there as local state via an injecteduseState— supplied through the existingHooksseam, exactly likememoalready is. Each flavor passes its own runtime's hook (React →react, JS →preact/hooks; Vue has no chat). Hooks can't be imported directly into the shared component because React renders it with React's reconciler.matchMedia) — or when no state hook is supplied — the component commits immediately instead of waiting for a transition that never fires. This also makes the headless connector trivially SSR/Node-safe again (nowindow).The React and JS widgets are updated to inject
useStateand drop the removed render-state fields.Tests
connectChat:clearMessagesclears messages, resets the conversation id, and is a no-op on empty — synchronously, with no transition step. Removed the now-obsoleteisClearing/onClearTransitionEndassertions.Chatcomponent (newclearingblock): fades out then commits ontransitionend; commits immediately under reduced motion; commits immediately with no state hook (graceful degradation).All chat suites pass (272 tests across
instantsearch-ui-components,instantsearch.js,react-instantsearch). Typecheck and lint are clean (the only remaining monorepo type errors are pre-existinginstantsearch-cli/commander issues onmaster).Test plan
🤖 Generated with Claude Code