-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[Discover Tabs] Extend the discover locator to allow opening in a new tab #234912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
efc8993
708f08e
42f403d
ea68b9e
449a634
da0f12f
cfb3ed6
a909cc8
73da781
be766b1
96a2dc8
17f6970
00add5c
3936eb8
4785f72
6bdab7e
bdaf825
6256d4d
1f9be33
1d268cc
33a1964
27766ef
1f6aa3d
38d9c42
0dec342
1808017
db8bafe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,11 @@ import { | |
| selectInitialUnifiedHistogramLayoutPropsMap, | ||
| selectTabRuntimeInternalState, | ||
| } from '../runtime_state'; | ||
| import { APP_STATE_URL_KEY, GLOBAL_STATE_URL_KEY } from '../../../../../../common/constants'; | ||
| import { | ||
| APP_STATE_URL_KEY, | ||
| GLOBAL_STATE_URL_KEY, | ||
| NEW_TAB_ID, | ||
| } from '../../../../../../common/constants'; | ||
| import type { DiscoverAppState } from '../../discover_app_state_container'; | ||
| import { createInternalStateAsyncThunk, createTabItem } from '../utils'; | ||
| import { setBreadcrumbs } from '../../../../../utils/breadcrumbs'; | ||
|
|
@@ -113,6 +117,20 @@ export const updateTabs: InternalStateThunkActionCreator<[TabbedContentState], P | |
| }; | ||
|
|
||
| if (!existingTab) { | ||
| // the following assignments for initialAppState, globalState, and dataRequestParams are for supporting `openInNewTab` action | ||
| tab.initialAppState = | ||
| 'initialAppState' in item | ||
| ? cloneDeep(item.initialAppState as TabState['initialAppState']) | ||
| : tab.initialAppState; | ||
| tab.globalState = | ||
| 'globalState' in item | ||
| ? cloneDeep(item.globalState as TabState['globalState']) | ||
| : tab.globalState; | ||
| tab.dataRequestParams = | ||
| 'dataRequestParams' in item | ||
| ? (item.dataRequestParams as TabState['dataRequestParams']) | ||
| : tab.dataRequestParams; | ||
|
|
||
| if (item.duplicatedFromId) { | ||
| // the new tab was created by duplicating an existing tab | ||
| const existingTabToDuplicateFrom = selectTab(currentState, item.duplicatedFromId); | ||
|
|
@@ -288,7 +306,9 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string | |
| (dispatch, getState) => { | ||
| const currentState = getState(); | ||
|
|
||
jughosta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (restoreTabId === currentState.tabs.unsafeCurrentId) { | ||
| // Restoring the 'new' tab ID is a no-op because it represents a placeholder for creating new tabs, | ||
| // not an actual tab that can be restored. | ||
| if (restoreTabId === currentState.tabs.unsafeCurrentId || restoreTabId === NEW_TAB_ID) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -318,6 +338,46 @@ export const restoreTab: InternalStateThunkActionCreator<[{ restoreTabId: string | |
| ); | ||
| }; | ||
|
|
||
| export const openInNewTab: InternalStateThunkActionCreator< | ||
| [ | ||
| { | ||
| tabLabel?: string; | ||
| appState?: TabState['initialAppState']; | ||
| globalState?: TabState['globalState']; | ||
| searchSessionId?: string; | ||
| } | ||
| ] | ||
| > = | ||
| ({ tabLabel, appState, globalState, searchSessionId }) => | ||
| (dispatch, getState) => { | ||
| const initialAppState = appState ? cloneDeep(appState) : undefined; | ||
| const initialGlobalState = globalState ? cloneDeep(globalState) : {}; | ||
| const currentState = getState(); | ||
| const currentTabs = selectAllTabs(currentState); | ||
|
|
||
| const newDefaultTab: TabState = { | ||
| ...DEFAULT_TAB_STATE, | ||
| ...createTabItem(currentTabs), | ||
| initialAppState, | ||
| globalState: initialGlobalState, | ||
| }; | ||
|
|
||
| if (tabLabel) { | ||
| newDefaultTab.label = tabLabel; | ||
| } | ||
|
|
||
| if (searchSessionId) { | ||
| newDefaultTab.dataRequestParams = { | ||
| ...newDefaultTab.dataRequestParams, | ||
| searchSessionId, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would still need additional handling to sync it to the search session manager. |
||
| }; | ||
| } | ||
|
|
||
| return dispatch( | ||
| updateTabs({ items: [...currentTabs, newDefaultTab], selectedItem: newDefaultTab }) | ||
| ); | ||
| }; | ||
|
|
||
| export const disconnectTab: InternalStateThunkActionCreator<[TabActionPayload]> = | ||
| ({ tabId }) => | ||
| (_, __, { runtimeStateManager }) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public'; | |
| import { createTabsStorageManager } from '../tabs_storage_manager'; | ||
|
|
||
| describe('InternalStateStore', () => { | ||
| it('should set data view', async () => { | ||
| const createTestStore = async () => { | ||
| const services = createDiscoverServicesMock(); | ||
| const urlStateStorage = createKbnUrlStateStorage(); | ||
| const runtimeStateManager = createRuntimeStateManager(); | ||
|
|
@@ -36,6 +36,12 @@ describe('InternalStateStore', () => { | |
| tabsStorageManager, | ||
| }); | ||
| await store.dispatch(internalStateActions.initializeTabs({ discoverSessionId: undefined })); | ||
|
|
||
| return { store, runtimeStateManager }; | ||
| }; | ||
|
|
||
| it('should set data view', async () => { | ||
| const { store, runtimeStateManager } = await createTestStore(); | ||
| const tabId = store.getState().tabs.unsafeCurrentId; | ||
| expect( | ||
| selectTabRuntimeState(runtimeStateManager, tabId).currentDataView$.value | ||
|
|
@@ -45,4 +51,37 @@ describe('InternalStateStore', () => { | |
| dataViewMock | ||
| ); | ||
| }); | ||
|
|
||
| it('should append a new tab to the tabs list', async () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding a unit test here! Would have liked to add more of these initially if I wasn't rushing through so much of it early on 🙂 |
||
| const { store } = await createTestStore(); | ||
| const initialTabId = store.getState().tabs.unsafeCurrentId; | ||
| expect(store.getState().tabs.allIds).toHaveLength(1); | ||
| expect(store.getState().tabs.unsafeCurrentId).toBe(initialTabId); | ||
| const params = { | ||
| tabLabel: 'New tab', | ||
| searchSessionId: 'session_123', | ||
| appState: { | ||
| query: { query: 'test this', language: 'kuery' }, | ||
| }, | ||
| globalState: { | ||
| timeRange: { | ||
| from: '2024-01-01T00:00:00.000Z', | ||
| to: '2024-01-02T00:00:00.000Z', | ||
| }, | ||
| }, | ||
| }; | ||
| await store.dispatch(internalStateActions.openInNewTab(params)); | ||
| const tabsState = store.getState().tabs; | ||
| expect(tabsState.allIds).toHaveLength(2); | ||
| expect(tabsState.unsafeCurrentId).not.toBe(initialTabId); | ||
| expect(tabsState.unsafeCurrentId).toBe(tabsState.allIds[1]); | ||
| expect(tabsState.byId[tabsState.unsafeCurrentId].label).toBe(params.tabLabel); | ||
| expect(tabsState.byId[tabsState.unsafeCurrentId].initialAppState).toEqual(params.appState); | ||
| expect(tabsState.byId[tabsState.unsafeCurrentId].globalState).toEqual(params.globalState); | ||
| expect(tabsState.byId[tabsState.unsafeCurrentId].dataRequestParams).toEqual({ | ||
| searchSessionId: params.searchSessionId, | ||
| timeRangeAbsolute: undefined, | ||
| timeRangeRelative: undefined, | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth creating an issue for this one if we don't do it in this PR, since we should do it for 9.2. Could be a good smaller task for one of the newer team members.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #235320