Skip to content

Commit 12f0b23

Browse files
[chat]Expand chat entry point to all pages (#10878)
* Fix primary right position Signed-off-by: Lin Wang <[email protected]> * Make chatbot available on all pages Signed-off-by: Lin Wang <[email protected]> * Changeset file for PR #10878 created/updated * Changeset file for PR #10878 created/updated * Changeset file for PR #10878 created/updated * Fix unit tests Signed-off-by: Lin Wang <[email protected]> * Remove comment code Signed-off-by: Lin Wang <[email protected]> --------- Signed-off-by: Lin Wang <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
1 parent ff9d150 commit 12f0b23

File tree

6 files changed

+83
-188
lines changed

6 files changed

+83
-188
lines changed

changelogs/fragments/10878.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
fix:
2+
- PrimaryHeaderRight not in correct position ([#10878](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10878))
3+
4+
feat:
5+
- [chat]Expand chat entry point to all pages ([#10878](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/10878))

src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/public/chrome/ui/header/header.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ export function Header({
597597
<EuiHeaderSection grow={false}>{renderRecentItems()}</EuiHeaderSection>
598598

599599
{renderBreadcrumbs(false, false)}
600-
600+
<EuiHeaderSection grow={true} />
601601
{renderPrimaryHeaderRight()}
602602
</EuiHeader>
603603

src/plugins/chat/public/components/chat_header_button.tsx

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@
55

66
import React, { useCallback, useRef, useState, useEffect, useImperativeHandle } from 'react';
77
import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
8-
import { CoreStart, MountPoint, SIDECAR_DOCKED_MODE } from '../../../../core/public';
8+
import { useUnmount } from 'react-use';
9+
import { CoreStart, SIDECAR_DOCKED_MODE } from '../../../../core/public';
910
import { ChatWindow, ChatWindowInstance } from './chat_window';
1011
import { ChatProvider } from '../contexts/chat_context';
1112
import { ChatService } from '../services/chat_service';
1213
import { GlobalAssistantProvider } from '../../../context_provider/public';
13-
import { OpenSearchDashboardsContextProvider } from '../../../opensearch_dashboards_react/public';
14+
import {
15+
MountPointPortal,
16+
OpenSearchDashboardsContextProvider,
17+
} from '../../../opensearch_dashboards_react/public';
1418
import { ContextProviderStart, TextSelectionMonitor } from '../../../context_provider/public';
1519
import './chat_header_button.scss';
1620
import { SuggestedActionsService } from '../services/suggested_action';
@@ -38,8 +42,12 @@ export const ChatHeaderButton = React.forwardRef<ChatHeaderButtonInstance, ChatH
3842
const [isOpen, setIsOpen] = useState<boolean>(chatService.isWindowOpen());
3943
const [layoutMode, setLayoutMode] = useState<ChatLayoutMode>(chatService.getWindowMode());
4044
const sideCarRef = useRef<{ close: () => void }>();
41-
const mountPointRef = useRef<HTMLDivElement>(null);
4245
const chatWindowRef = useRef<ChatWindowInstance>(null);
46+
const flyoutMountPoint = useRef(null);
47+
48+
const setMountPoint = useCallback((mountPoint) => {
49+
flyoutMountPoint.current = mountPoint;
50+
}, []);
4351

4452
// Register ChatWindow ref with ChatService for external access
4553
useEffect(() => {
@@ -50,18 +58,7 @@ export const ChatHeaderButton = React.forwardRef<ChatHeaderButtonInstance, ChatH
5058
}, [chatService]);
5159

5260
const openSidecar = useCallback(() => {
53-
if (!mountPointRef.current) return;
54-
55-
const mountPoint: MountPoint = (element) => {
56-
if (mountPointRef.current) {
57-
element.appendChild(mountPointRef.current);
58-
}
59-
return () => {
60-
if (mountPointRef.current && element.contains(mountPointRef.current)) {
61-
element.removeChild(mountPointRef.current);
62-
}
63-
};
64-
};
61+
if (!flyoutMountPoint.current) return;
6562

6663
const sidecarConfig =
6764
layoutMode === ChatLayoutMode.FULLSCREEN
@@ -76,7 +73,7 @@ export const ChatHeaderButton = React.forwardRef<ChatHeaderButtonInstance, ChatH
7673
isHidden: false,
7774
};
7875

79-
sideCarRef.current = core.overlays.sidecar.open(mountPoint, {
76+
sideCarRef.current = core.overlays.sidecar.open(flyoutMountPoint.current, {
8077
className: `chat-sidecar chat-sidecar--${layoutMode}`,
8178
config: sidecarConfig,
8279
});
@@ -172,13 +169,12 @@ export const ChatHeaderButton = React.forwardRef<ChatHeaderButtonInstance, ChatH
172169
}, [chatService, isOpen, openSidecar, closeSidecar]);
173170

174171
// Cleanup on unmount
175-
useEffect(() => {
176-
return () => {
177-
if (sideCarRef.current) {
178-
sideCarRef.current.close();
179-
}
180-
};
181-
}, []);
172+
useUnmount(() => {
173+
if (sideCarRef.current) {
174+
chatService.setWindowState(false);
175+
sideCarRef.current.close();
176+
}
177+
});
182178

183179
return (
184180
<>
@@ -197,35 +193,36 @@ export const ChatHeaderButton = React.forwardRef<ChatHeaderButtonInstance, ChatH
197193
</EuiToolTip>
198194

199195
{/* Mount point for sidecar content */}
200-
<div
201-
ref={mountPointRef}
202-
className={`chatHeaderButton__mountPoint ${
203-
isOpen
204-
? 'chatHeaderButton__mountPoint--visible'
205-
: 'chatHeaderButton__mountPoint--hidden'
206-
}`}
207-
>
208-
<div className="chatHeaderButton__content">
209-
<OpenSearchDashboardsContextProvider services={{ core, contextProvider, charts }}>
210-
<GlobalAssistantProvider
211-
onToolsUpdated={(tools) => {
212-
// Tools updated in chat
213-
}}
214-
>
215-
<ChatProvider
216-
chatService={chatService}
217-
suggestedActionsService={suggestedActionsService}
196+
<MountPointPortal setMountPoint={setMountPoint}>
197+
<div
198+
className={`chatHeaderButton__mountPoint ${
199+
isOpen
200+
? 'chatHeaderButton__mountPoint--visible'
201+
: 'chatHeaderButton__mountPoint--hidden'
202+
}`}
203+
>
204+
<div className="chatHeaderButton__content">
205+
<OpenSearchDashboardsContextProvider services={{ core, contextProvider, charts }}>
206+
<GlobalAssistantProvider
207+
onToolsUpdated={(tools) => {
208+
// Tools updated in chat
209+
}}
218210
>
219-
<ChatWindow
220-
layoutMode={layoutMode}
221-
onToggleLayout={toggleLayoutMode}
222-
ref={chatWindowRef}
223-
/>
224-
</ChatProvider>
225-
</GlobalAssistantProvider>
226-
</OpenSearchDashboardsContextProvider>
211+
<ChatProvider
212+
chatService={chatService}
213+
suggestedActionsService={suggestedActionsService}
214+
>
215+
<ChatWindow
216+
layoutMode={layoutMode}
217+
onToggleLayout={toggleLayoutMode}
218+
ref={chatWindowRef}
219+
/>
220+
</ChatProvider>
221+
</GlobalAssistantProvider>
222+
</OpenSearchDashboardsContextProvider>
223+
</div>
227224
</div>
228-
</div>
225+
</MountPointPortal>
229226
</>
230227
);
231228
}

src/plugins/chat/public/plugin.test.ts

Lines changed: 10 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('ChatPlugin', () => {
4242
},
4343
chrome: {
4444
navControls: {
45-
registerRight: jest.fn(),
45+
registerPrimaryHeaderRight: jest.fn(),
4646
},
4747
globalSearch: {
4848
registerSearchCommand: jest.fn(),
@@ -88,7 +88,7 @@ describe('ChatPlugin', () => {
8888
it('should register chat button in header nav controls', () => {
8989
plugin.start(mockCoreStart, mockDeps);
9090

91-
expect(mockCoreStart.chrome.navControls.registerRight).toHaveBeenCalledWith({
91+
expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalledWith({
9292
order: 1000,
9393
mount: expect.any(Function),
9494
});
@@ -111,7 +111,7 @@ describe('ChatPlugin', () => {
111111
// ChatService should still be created (uses proxy endpoint)
112112
expect(ChatService).toHaveBeenCalledWith();
113113
expect(startContract.chatService).toBeInstanceOf(ChatService);
114-
expect(mockCoreStart.chrome.navControls.registerRight).toHaveBeenCalled();
114+
expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).toHaveBeenCalled();
115115
});
116116

117117
it('should not initialize when plugin is disabled', () => {
@@ -124,7 +124,7 @@ describe('ChatPlugin', () => {
124124

125125
expect(ChatService).not.toHaveBeenCalled();
126126
expect(startContract.chatService).toBeUndefined();
127-
expect(mockCoreStart.chrome.navControls.registerRight).not.toHaveBeenCalled();
127+
expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).not.toHaveBeenCalled();
128128
});
129129

130130
it('should not initialize when enabled is missing (defaults to false)', () => {
@@ -136,7 +136,7 @@ describe('ChatPlugin', () => {
136136

137137
expect(ChatService).not.toHaveBeenCalled();
138138
expect(startContract.chatService).toBeUndefined();
139-
expect(mockCoreStart.chrome.navControls.registerRight).not.toHaveBeenCalled();
139+
expect(mockCoreStart.chrome.navControls.registerPrimaryHeaderRight).not.toHaveBeenCalled();
140140
});
141141
});
142142

@@ -147,12 +147,12 @@ describe('ChatPlugin', () => {
147147
plugin.start(mockCoreStart, mockDeps);
148148

149149
// Get the mount function that was registered
150-
const registerCall = (mockCoreStart.chrome.navControls.registerRight as jest.Mock).mock
151-
.calls[0];
150+
const registerCall = (mockCoreStart.chrome.navControls
151+
.registerPrimaryHeaderRight as jest.Mock).mock.calls[0];
152152
mountFunction = registerCall[0].mount;
153153
});
154154

155-
it('should show chat button when app starts with "explore"', () => {
155+
it('should show chat button', () => {
156156
const mockElement = document.createElement('div');
157157
const mockUnmount = jest.fn();
158158
(toMountPoint as jest.Mock).mockReturnValue(jest.fn().mockReturnValue(mockUnmount));
@@ -167,101 +167,6 @@ describe('ChatPlugin', () => {
167167
// Cleanup
168168
cleanup();
169169
});
170-
171-
it('should hide chat button when app does not start with "explore"', () => {
172-
const mockElement = document.createElement('div');
173-
const mockUnmount = jest.fn();
174-
(toMountPoint as jest.Mock).mockReturnValue(jest.fn().mockReturnValue(mockUnmount));
175-
176-
// Reset the mock to clear previous calls
177-
(toMountPoint as jest.Mock).mockClear();
178-
179-
// Start with a fresh BehaviorSubject for non-explore app
180-
const nonExploreAppId$ = new BehaviorSubject<string | undefined>('dashboard');
181-
mockCoreStart.application.currentAppId$ = nonExploreAppId$;
182-
183-
// Re-register the plugin with the new app ID
184-
plugin.start(mockCoreStart, mockDeps);
185-
const registerCall = (mockCoreStart.chrome.navControls.registerRight as jest.Mock).mock
186-
.calls[1];
187-
const newMountFunction = registerCall[0].mount;
188-
189-
const cleanup = newMountFunction(mockElement);
190-
191-
// Should not mount the component for non-explore apps
192-
expect(toMountPoint).not.toHaveBeenCalled();
193-
194-
// Cleanup
195-
cleanup();
196-
});
197-
198-
it('should handle app changes from explore to non-explore', () => {
199-
const mockElement = document.createElement('div');
200-
const mockUnmount = jest.fn();
201-
(toMountPoint as jest.Mock).mockReturnValue(jest.fn().mockReturnValue(mockUnmount));
202-
203-
const cleanup = mountFunction(mockElement);
204-
205-
// Start with explore app
206-
mockCurrentAppId$.next('explore-metrics');
207-
expect(toMountPoint).toHaveBeenCalled();
208-
209-
// Change to non-explore app
210-
mockCurrentAppId$.next('discover');
211-
expect(mockUnmount).toHaveBeenCalled();
212-
213-
// Cleanup
214-
cleanup();
215-
});
216-
217-
it('should handle undefined app id', () => {
218-
const mockElement = document.createElement('div');
219-
220-
// Reset the mock to clear previous calls
221-
(toMountPoint as jest.Mock).mockClear();
222-
223-
// Create a new plugin instance for this test
224-
const testPlugin = new ChatPlugin(mockInitializerContext);
225-
226-
// Start with a fresh BehaviorSubject for undefined app
227-
const undefinedAppId$ = new BehaviorSubject<string | undefined>(undefined);
228-
const testCoreStart = {
229-
...mockCoreStart,
230-
application: {
231-
currentAppId$: undefinedAppId$,
232-
},
233-
};
234-
235-
// Start the plugin with undefined app ID
236-
testPlugin.start(testCoreStart, mockDeps);
237-
const registerCall = (testCoreStart.chrome.navControls.registerRight as jest.Mock).mock
238-
.calls[0];
239-
const newMountFunction = registerCall[0].mount;
240-
241-
const cleanup = newMountFunction(mockElement);
242-
243-
expect(toMountPoint).not.toHaveBeenCalled();
244-
245-
// Cleanup
246-
cleanup();
247-
});
248-
249-
it('should cleanup subscription on unmount', () => {
250-
const mockElement = document.createElement('div');
251-
const mockSubscription = {
252-
unsubscribe: jest.fn(),
253-
};
254-
255-
// Mock the subscription
256-
jest.spyOn(mockCurrentAppId$, 'subscribe').mockReturnValue(mockSubscription as any);
257-
258-
const cleanup = mountFunction(mockElement);
259-
260-
// Call cleanup
261-
cleanup();
262-
263-
expect(mockSubscription.unsubscribe).toHaveBeenCalled();
264-
});
265170
});
266171

267172
describe('stop', () => {
@@ -301,8 +206,8 @@ describe('ChatPlugin', () => {
301206
it('should pass correct props to ChatHeaderButton', () => {
302207
plugin.start(mockCoreStart, mockDeps);
303208

304-
const registerCall = (mockCoreStart.chrome.navControls.registerRight as jest.Mock).mock
305-
.calls[0];
209+
const registerCall = (mockCoreStart.chrome.navControls
210+
.registerPrimaryHeaderRight as jest.Mock).mock.calls[0];
306211
const mountFunction = registerCall[0].mount;
307212
const mockElement = document.createElement('div');
308213

0 commit comments

Comments
 (0)