Chat: Integrate Accordion to MessageBubble #32185
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request integrates an Accordion component into the Chat widget's MessageBubble to display function call metadata. The feature allows chat messages to include information about AI assistant function calls, showing the function name, arguments, and results in an expandable accordion interface.
Changes:
- Added TypeScript type definitions for
FunctionCall,FunctionCallArgument, andMetaDatatypes - Extended
TextMessagetype to include optionalmetadatafield - Implemented accordion rendering in MessageBubble to display function call details
- Added localization strings for function call UI labels across all supported languages
- Updated playground example to demonstrate the new functionality
Reviewed changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/devextreme/js/ui/chat.d.ts | Added TypeScript definitions for function call metadata types and extended TextMessage |
| packages/devextreme/js/__internal/ui/chat/messagegroup.ts | Passed metadata from message to MessageBubble options |
| packages/devextreme/js/__internal/ui/chat/messagebubble.ts | Implemented accordion rendering for function call metadata, added element creation and option change handling |
| packages/devextreme/js/localization/messages/*.json | Added localization strings for function call labels in 31 language files |
| packages/devextreme/playground/jquery.html | Updated playground to demonstrate Chat widget with function call metadata |
| focusStateEnabled?: boolean; | ||
| hoverStateEnabled?: boolean; |
There was a problem hiding this comment.
The focusStateEnabled and hoverStateEnabled properties are being added to MessageBubble specifically to pass through to the Accordion component. However, these options shadow the inherited focusStateEnabled and hoverStateEnabled from the parent Widget class. This creates ambiguity: MessageBubble is a Widget that has its own focus/hover states, but these properties are being used exclusively for the nested Accordion. Consider using namespaced properties like accordionFocusStateEnabled and accordionHoverStateEnabled to avoid confusion and maintain clear separation between the MessageBubble's state and the Accordion's state.
| focusStateEnabled: true, | ||
| hoverStateEnabled: true, |
There was a problem hiding this comment.
Setting default values for focusStateEnabled and hoverStateEnabled to true overrides the parent Widget class defaults. Since these are now being used to control the Accordion's state rather than the MessageBubble's own state, this could lead to unexpected behavior if the parent class has different defaults or if these properties are meant to control the MessageBubble itself. Consider whether these defaults should be set at the Accordion initialization level instead.
| focusStateEnabled: true, | |
| hoverStateEnabled: true, |
| case 'focusStateEnabled': | ||
| case 'hoverStateEnabled': | ||
| this._renderFunctionCall(); | ||
| break; |
There was a problem hiding this comment.
When focusStateEnabled or hoverStateEnabled options change, the entire function call accordion is re-rendered by calling _renderFunctionCall(). This completely destroys and recreates the Accordion component even when only state options need to be updated. Consider updating the existing Accordion instance's options using this._accordionInstance.option(name, value) instead of full re-rendering for better performance.
| this._createComponent(this._$functionCall, Accordion, { | ||
| dataSource: accordionItems, | ||
| collapsible: true, | ||
| multiple: false, | ||
| selectedIndex: -1, | ||
| focusStateEnabled, | ||
| hoverStateEnabled, | ||
| }); |
There was a problem hiding this comment.
The Accordion component instance is created but not stored as a class property (e.g., this._accordionInstance). This makes it impossible to update the component's options later without re-creating it entirely. Store the instance returned by _createComponent to enable efficient updates when options change.
| "dxChat-fileViewLabel": "File list", | ||
| "dxChat-downloadButtonLabel": "Download file {0}", | ||
| "dxChat-fileLimitReachedWarning": "You selected too many files. Select no more than {0} files and retry.", | ||
| "dxChat-functionCallTitle": "function called", |
There was a problem hiding this comment.
Inconsistent capitalization: 'function called' should be 'Function Called' to match the capitalization pattern of other similar labels in the localization file.
| "use strict"; | ||
|
|
||
| /* Comment lines below for the widgets you don't require and run "devextreme-bundler" in this directory, then include dx.custom.js in your project */ |
There was a problem hiding this comment.
The entire file appears to be a complete rewrite of the bundle template. This is a 194-line file that seems to be auto-generated or a template file. If this is auto-generated, it should not be in the PR. If it's a manual template update, the PR should clarify why this file needed to be regenerated or modified as part of this feature.
1 participant
No description provided.