Conversation
Adds a separate chat trigger widget, and give a warning if you don't use it, ai mode or inline layout
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 29 |
| Duplication | -1 |
TIP This summary will be updated as you push new changes. Give us feedback
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: |
There was a problem hiding this comment.
Pull request overview
This PR splits the “open chat” entry point out of the Chat UI into a dedicated chatTrigger widget/component, and adds runtime validation to ensure the Chat widget has a configured way to open (via chatTrigger, AI-mode inputs, or opting out).
Changes:
- Introduces
chatTriggeracross InstantSearch.js + React InstantSearch (+ auseChatTriggerconnector) and updates exports. - Adds entry-point validation in
connectChatand propagatesopensChatfrom AI-mode input widgets. - Refactors tests and shared UI types to accommodate removal of the built-in toggle button.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/common/widgets/chat/utils.ts | Switches test “open chat” helper from DOM click to a global setter hook |
| tests/common/widgets/chat/index.ts | Resets the global setter hook between tests |
| packages/react-instantsearch/src/widgets/SearchBox.tsx | Marks AI-mode search box as an entry point (opensChat) |
| packages/react-instantsearch/src/widgets/index.ts | Exports new ChatTrigger component |
| packages/react-instantsearch/src/widgets/ChatTrigger.tsx | Adds React ChatTrigger component and registers presence via useChatTrigger |
| packages/react-instantsearch/src/widgets/Chat.tsx | Removes toggle-button wiring; adds disableTriggerValidation plumbing + getOpen handle |
| packages/react-instantsearch/src/widgets/Autocomplete.tsx | Marks AI-mode autocomplete as an entry point (opensChat) |
| packages/react-instantsearch/src/widgets/tests/utils/all-widgets.tsx | Disables trigger validation for widget catalog tests |
| packages/react-instantsearch/src/tests/common-widgets.test.tsx | Wires chat ref for tests via global setter hook |
| packages/react-instantsearch-core/src/index.ts | Exports useChatTrigger |
| packages/react-instantsearch-core/src/connectors/useChatTrigger.ts | Adds useChatTrigger connector wrapper |
| packages/instantsearch.js/src/widgets/search-box/search-box.tsx | Marks AI-mode SearchBox widget as an entry point (opensChat) |
| packages/instantsearch.js/src/widgets/index.ts | Exports new chatTrigger widget |
| packages/instantsearch.js/src/widgets/chat/chat.tsx | Adds setOpen/getOpen widget API + disableTriggerValidation param |
| packages/instantsearch.js/src/widgets/chat/tests/chat.test.tsx | Adds a negative test for entry-point validation |
| packages/instantsearch.js/src/widgets/chat-trigger/chat-trigger.tsx | Adds the new JS chatTrigger widget implementation |
| packages/instantsearch.js/src/widgets/autocomplete/autocomplete.tsx | Marks AI-mode Autocomplete widget as an entry point (opensChat) |
| packages/instantsearch.js/src/connectors/index.ts | Exports connectChatTrigger |
| packages/instantsearch.js/src/connectors/chat/connectChatTrigger.ts | Adds presence-marker connector that sets opensChat |
| packages/instantsearch.js/src/connectors/chat/connectChat.ts | Adds entry-point validation logic (+ opt-out flag) |
| packages/instantsearch.js/src/connectors/chat/tests/connectChat-test.ts | Adjusts tests to bypass validation where needed |
| packages/instantsearch.js/src/tests/common-widgets.test.tsx | Updates Chat test setup to bypass validation + expose setOpen |
| packages/instantsearch-ui-components/src/components/chat/types.ts | Makes toggleButtonComponent optional in layout props |
| packages/instantsearch-ui-components/src/components/chat/Chat.tsx | Removes built-in toggle button rendering |
| packages/instantsearch-ui-components/src/components/chat/tests/Chat.test.tsx | Snapshot updated after toggle button removal (currently shows leaked prop) |
| .claude/skills/port-widget/SKILL.md | Adds migration checklist guidance for cross-package API changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| $$widgetType: 'ais.chatTrigger', | ||
| dispose: () => { | ||
| render(null, containerNode); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
chatTrigger doesn’t set opensChat: true, but connectChat’s entry-point validation only checks mainIndex.getWidgets().some(w => w.opensChat === true). As a result, adding this widget won’t satisfy the validation and chat will still throw. Add opensChat: true on the returned widget (or implement this widget via connectChatTrigger, which already sets opensChat).
| const button = document.createElement('button'); | ||
| button.className = `ais-ChatTrigger ${ | ||
| userCssClasses.button ? String(userCssClasses.button) : '' | ||
| }`; | ||
| button.setAttribute('aria-label', 'Open chat'); | ||
| button.setAttribute('aria-pressed', String(isOpen)); | ||
|
|
||
| button.addEventListener('click', () => { | ||
| chat.setOpen(!chat.getOpen()); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }); | ||
|
|
||
| if (LayoutComponent) { | ||
| render( | ||
| <LayoutComponent | ||
| open={isOpen} | ||
| onClick={() => { | ||
| chat.setOpen(!isOpen); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }} | ||
| toggleIconComponent={iconComponent} | ||
| />, | ||
| button | ||
| ); |
There was a problem hiding this comment.
The template rendering path creates a DOM <button> and then renders a template whose rootTagName is also button into it. This produces nested buttons and (with the extra button.addEventListener('click', ...)) can double-toggle due to event bubbling, effectively cancelling the open/close action. Consider rendering a single button element (no nested button), and ensure there is only one click handler responsible for toggling.
| const button = document.createElement('button'); | ||
| button.className = `ais-ChatTrigger ${ | ||
| userCssClasses.button ? String(userCssClasses.button) : '' | ||
| }`; | ||
| button.setAttribute('aria-label', 'Open chat'); | ||
| button.setAttribute('aria-pressed', String(isOpen)); | ||
|
|
||
| button.addEventListener('click', () => { | ||
| chat.setOpen(!chat.getOpen()); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }); | ||
|
|
||
| if (LayoutComponent) { | ||
| render( | ||
| <LayoutComponent | ||
| open={isOpen} | ||
| onClick={() => { | ||
| chat.setOpen(!isOpen); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }} | ||
| toggleIconComponent={iconComponent} | ||
| />, | ||
| button | ||
| ); | ||
| } else { | ||
| button.textContent = isOpen ? 'Close' : 'Chat'; | ||
| } | ||
|
|
||
| render(button, containerNode); |
There was a problem hiding this comment.
render(button, containerNode) passes an HTMLButtonElement as the vnode argument, but preact.render expects a VNode/component child (string/number/VNode), not a DOM node. This is likely to throw or behave unexpectedly at runtime. Render a Preact element into containerNode instead of creating a DOM node and passing it to render.
| const button = document.createElement('button'); | |
| button.className = `ais-ChatTrigger ${ | |
| userCssClasses.button ? String(userCssClasses.button) : '' | |
| }`; | |
| button.setAttribute('aria-label', 'Open chat'); | |
| button.setAttribute('aria-pressed', String(isOpen)); | |
| button.addEventListener('click', () => { | |
| chat.setOpen(!chat.getOpen()); | |
| button.setAttribute('aria-pressed', String(chat.getOpen())); | |
| }); | |
| if (LayoutComponent) { | |
| render( | |
| <LayoutComponent | |
| open={isOpen} | |
| onClick={() => { | |
| chat.setOpen(!isOpen); | |
| button.setAttribute('aria-pressed', String(chat.getOpen())); | |
| }} | |
| toggleIconComponent={iconComponent} | |
| />, | |
| button | |
| ); | |
| } else { | |
| button.textContent = isOpen ? 'Close' : 'Chat'; | |
| } | |
| render(button, containerNode); | |
| const toggleChat = () => { | |
| chat.setOpen(!chat.getOpen()); | |
| renderTrigger(); | |
| }; | |
| if (LayoutComponent) { | |
| render( | |
| <LayoutComponent | |
| open={isOpen} | |
| onClick={toggleChat} | |
| toggleIconComponent={iconComponent} | |
| />, | |
| containerNode | |
| ); | |
| } else { | |
| render( | |
| <button | |
| className={`ais-ChatTrigger ${ | |
| userCssClasses.button ? String(userCssClasses.button) : '' | |
| }`} | |
| aria-label="Open chat" | |
| aria-pressed={String(isOpen)} | |
| onClick={toggleChat} | |
| > | |
| {isOpen ? 'Close' : 'Chat'} | |
| </button>, | |
| containerNode | |
| ); | |
| } |
| const externalRefs = { | ||
| setOpen: null as unknown as (open: boolean) => void, | ||
| getOpen: null as unknown as () => boolean, | ||
| }; | ||
|
|
||
| const specializedRenderer = createRenderer({ | ||
| containerNode, | ||
| cssClasses, | ||
| renderState: {}, | ||
| templates, | ||
| tools, | ||
| externalRefs, | ||
| }); | ||
|
|
||
| const makeWidget = connectChat(specializedRenderer, () => | ||
| render(null, containerNode) | ||
| ); | ||
|
|
||
| return { | ||
| const widget = { | ||
| ...makeWidget({ | ||
| resume, | ||
| tools, | ||
| disableTriggerValidation, | ||
| ...options, | ||
| }), | ||
| $$widgetType: 'ais.chat', | ||
| $$widgetType: 'ais.chat' as const, | ||
| /** | ||
| * Opens or closes the chat | ||
| */ | ||
| setOpen(open: boolean) { | ||
| externalRefs.setOpen(open); | ||
| }, | ||
| /** | ||
| * Returns whether the chat is currently open | ||
| */ | ||
| getOpen(): boolean { | ||
| return externalRefs.getOpen(); | ||
| }, |
There was a problem hiding this comment.
externalRefs.setOpen / externalRefs.getOpen are initialized as null as unknown as ... and are called unconditionally in widget.setOpen() / widget.getOpen(). If a consumer (or chatTrigger) calls these methods before the chat has rendered at least once, this will crash at runtime. Add guards (e.g., throw a clear error until initialized) or initialize to safe functions that don’t rely on the first render order of widgets.
| const [isOpen, setIsOpen] = useState(false); | ||
| useChatTrigger({}, { $$widgetType: 'ais.chatTrigger' }); | ||
|
|
||
| useEffect(() => { | ||
| if (chatRef.current) { | ||
| setIsOpen(chatRef.current.getOpen?.() ?? false); | ||
| } | ||
| }, [chatRef]); | ||
|
|
||
| const handleClick = () => { | ||
| if (chatRef.current) { | ||
| const currentOpen = chatRef.current.getOpen?.() ?? isOpen; | ||
| const nextOpen = !currentOpen; | ||
|
|
||
| chatRef.current.setOpen(nextOpen); | ||
| setIsOpen(nextOpen); | ||
| } | ||
| onClick?.(); | ||
| }; |
There was a problem hiding this comment.
ChatTrigger keeps its own isOpen state and only updates it on mount and when the trigger itself is clicked. If the chat is opened/closed via another entry point (e.g. Chat header close button, SearchBox/Autocomplete AI mode, programmatic setOpen), aria-pressed and the label can become stale. Consider deriving isOpen from the InstantSearch chat render state (or adding an onOpenChange/subscription mechanism on ChatHandle) instead of duplicating state here.
| globalThis.__chatTestSetOpen?.(true); | ||
|
|
There was a problem hiding this comment.
openChat() uses globalThis.__chatTestSetOpen?.(true), which becomes a silent no-op if the test setup forgets to assign __chatTestSetOpen. That can make failures harder to diagnose (chat never opens, later assertions fail). Consider throwing an explicit error when __chatTestSetOpen is null/undefined so the test fails fast with a clear message.
| globalThis.__chatTestSetOpen?.(true); | |
| const setOpen = globalThis.__chatTestSetOpen; | |
| if (setOpen == null) { | |
| throw new Error( | |
| 'openChat() requires globalThis.__chatTestSetOpen to be set by the test setup.' | |
| ); | |
| } | |
| setOpen(true); |
| const { query, refine, isSearchStalled } = useSearchBox({ queryHook }, { | ||
| $$widgetType: 'ais.searchBox', | ||
| ...(aiMode ? { opensChat: true } : {}), | ||
| } as Parameters<typeof useSearchBox>[1]); |
There was a problem hiding this comment.
The additionalWidgetProperties argument is being type-cast to Parameters<typeof useSearchBox>[1] to allow opensChat. This hides type drift and makes future refactors riskier. Prefer extending the relevant widget/additional-properties typings to include opensChat (or a generic index signature) so this can be passed without casting.
| const { refine } = useSearchBox({}, { | ||
| $$type: 'ais.autocomplete', | ||
| $$widgetType: 'ais.autocomplete', | ||
| ...(aiMode ? { opensChat: true } : {}), | ||
| } as Parameters<typeof useSearchBox>[1]); |
There was a problem hiding this comment.
Same as SearchBox: the cast on the useSearchBox additional widget properties is only needed because opensChat isn’t represented in the type. It would be safer to update the shared typing so opensChat can be provided without as Parameters<...> casts.
|
|
||
| See documentation: https://www.algolia.com/doc/api-reference/widgets/chat/js/#connector" | ||
| `); | ||
| }); |
There was a problem hiding this comment.
Entry-point validation is now tested only for the negative case (throws when no entry point). Add at least one positive test asserting search.start() does not throw when a chatTrigger widget is present and/or when an AI-mode input widget sets opensChat: true, to prevent regressions in the intended supported setups.
| }); | |
| }); | |
| test('does not throw when a `chatTrigger` widget is present', () => { | |
| const container = document.createElement('div'); | |
| document.body.appendChild(container); | |
| const search = instantsearch({ | |
| indexName: 'indexName', | |
| searchClient: createSearchClient(), | |
| }); | |
| const chatTriggerWidget = { | |
| $$type: 'ais.chatTrigger', | |
| init() {}, | |
| }; | |
| search.addWidgets([ | |
| chat({ | |
| container, | |
| agentId: 'test-agent-id', | |
| }), | |
| chatTriggerWidget, | |
| ]); | |
| expect(() => { | |
| search.start(); | |
| }).not.toThrow(); | |
| }); |
| function renderTrigger() { | ||
| const isOpen = chat.getOpen(); | ||
| const button = document.createElement('button'); | ||
| button.className = `ais-ChatTrigger ${ | ||
| userCssClasses.button ? String(userCssClasses.button) : '' | ||
| }`; | ||
| button.setAttribute('aria-label', 'Open chat'); | ||
| button.setAttribute('aria-pressed', String(isOpen)); | ||
|
|
||
| button.addEventListener('click', () => { | ||
| chat.setOpen(!chat.getOpen()); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }); | ||
|
|
||
| if (LayoutComponent) { | ||
| render( | ||
| <LayoutComponent | ||
| open={isOpen} | ||
| onClick={() => { | ||
| chat.setOpen(!isOpen); | ||
| button.setAttribute('aria-pressed', String(chat.getOpen())); | ||
| }} | ||
| toggleIconComponent={iconComponent} | ||
| />, | ||
| button | ||
| ); | ||
| } else { | ||
| button.textContent = isOpen ? 'Close' : 'Chat'; | ||
| } | ||
|
|
||
| render(button, containerNode); | ||
| } | ||
|
|
||
| renderTrigger(); | ||
|
|
||
| return { | ||
| $$widgetType: 'ais.chatTrigger', | ||
| dispose: () => { | ||
| render(null, containerNode); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
This widget renders immediately during widget creation (renderTrigger(); runs before the widget is added/started). InstantSearch widgets should render in init/render lifecycle methods; rendering during factory execution can create DOM side effects even if the widget is never mounted, and it also prevents re-rendering on subsequent renders. Move the initial render into init (and update in render as needed) and ensure the widget exposes $$type like other widgets/connectors do.
| const widgets = instantSearchInstance.mainIndex.getWidgets() as Array<{ | ||
| opensChat?: boolean; | ||
| }>; | ||
|
|
||
| const hasEntryPoint = widgets.some((w) => w.opensChat === true); |
There was a problem hiding this comment.
should use walkIndex
Adds a separate chat trigger widget, and give a warning if you don't use it, ai mode or inline layout
still to do: validate this is the right approach, ensure there's no leftover code from previous iterations.