Skip to content

Commit 0e3ad7c

Browse files
ashwin-pcLeo7Deng
ashwin-pc
authored and
Leo7Deng
committed
[Discover] A bunch of navigation fixes (opensearch-project#5168)
* Discover: Fixes state persistence after nav * Fixed breadcrumbs and navigation * fixes mobile view --------- Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com> Signed-off-by: Leo Deng <leo7deng@gmail.com>
1 parent 49de29d commit 0e3ad7c

File tree

16 files changed

+206
-175
lines changed

16 files changed

+206
-175
lines changed

‎CHANGELOG.md

+6-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1010

1111
### 🛡 Security
1212

13+
- [CVE-2022-25869] Remove AngularJS 1.8 ([#5086](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5086))
1314
- [CVE-2022-37599] Bump loader-utils from `2.0.3` to `2.0.4` ([#3031](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3031)). Backwards-compatible fixes included in v2.6.0 and v1.3.7 releases.
1415
- [CVE-2022-37603] Bump loader-utils from `2.0.3` to `2.0.4` ([#3031](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3031)). Backwards-compatible fixes included in v2.6.0 and v1.3.7 releases.
1516
- [WS-2021-0638] Bump mocha from `7.2.0` to `10.1.0` ([#2711](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2711))
@@ -36,8 +37,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
3637
- [Decouple] Allow plugin manifest config to define semver compatible OpenSearch plugin and verify if it is installed on the cluster([#4612](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4612))
3738
- [Advanced Settings] Consolidate settings into new "Appearance" category and add category IDs ([#4845](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4845))
3839
- Adds Data explorer framework and implements Discover using it ([#4806](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4806))
39-
- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936/))
40-
- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854/))
40+
- [Theme] Use themes' definitions to render the initial view ([#4936](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4936))
41+
- [Theme] Make `next` theme the default ([#4854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4854))
42+
- [Discover] Update embeddable for saved searches ([#5081](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5081))
4143

4244
### 🐛 Bug Fixes
4345

@@ -55,6 +57,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
5557
- [BUG] Fix buildPointSeriesData unit test fails due to local timezone ([#4992](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4992))
5658
- [BUG][Data Explorer][Discover] Fix total hits issue for no time based data ([#5087](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5087))
5759
- [BUG][Data Explorer][Discover] Add onQuerySubmit to top nav and allow force update to embeddable ([#5160](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5160))
60+
- [BUG][Discover] Fix misc navigation issues ([#5168](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5168))
61+
- [BUG][Discover] Fix mobile view ([#5168](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5168))
5862

5963
### 🚞 Infrastructure
6064

‎src/plugins/data_explorer/public/utils/state_management/store.ts

+31-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ import { reducer as metadataReducer } from './metadata_slice';
99
import { loadReduxState, persistReduxState } from './redux_persistence';
1010
import { DataExplorerServices } from '../../types';
1111

12+
const HYDRATE = 'HYDRATE';
13+
14+
export const hydrate = (newState: RootState) => ({
15+
type: HYDRATE,
16+
payload: newState,
17+
});
18+
1219
const commonReducers = {
1320
metadata: metadataReducer,
1421
};
@@ -22,9 +29,20 @@ let dynamicReducers: {
2229

2330
const rootReducer = combineReducers(dynamicReducers);
2431

32+
const createRootReducer = (): Reducer<RootState> => {
33+
const combinedReducer = combineReducers(dynamicReducers);
34+
35+
return (state: RootState | undefined, action: any): RootState => {
36+
if (action.type === HYDRATE) {
37+
return action.payload;
38+
}
39+
return combinedReducer(state, action);
40+
};
41+
};
42+
2543
export const configurePreloadedStore = (preloadedState: PreloadedState<RootState>) => {
2644
// After registering the slices the root reducer needs to be updated
27-
const updatedRootReducer = combineReducers(dynamicReducers);
45+
const updatedRootReducer = createRootReducer();
2846

2947
return configureStore({
3048
reducer: updatedRootReducer,
@@ -62,6 +80,18 @@ export const getPreloadedStore = async (services: DataExplorerServices) => {
6280
// the store subscriber will automatically detect changes and call handleChange function
6381
const unsubscribe = store.subscribe(handleChange);
6482

83+
// This is necessary because browser navigation updates URL state that isnt reflected in the redux state
84+
services.scopedHistory.listen(async (location, action) => {
85+
const urlState = await loadReduxState(services);
86+
const currentState = store.getState();
87+
88+
// If the url state is different from the current state, then we need to update the store
89+
// the state should have a view property if it was loaded from the url
90+
if (action === 'POP' && urlState.metadata?.view && !isEqual(urlState, currentState)) {
91+
store.dispatch(hydrate(urlState as RootState));
92+
}
93+
});
94+
6595
const onUnsubscribe = () => {
6696
dynamicReducers = {
6797
...commonReducers,

‎src/plugins/discover/public/application/components/doc_views/surrounding_docs_app.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export function SurroundingDocsApp() {
4747

4848
useEffect(() => {
4949
chrome.setBreadcrumbs([
50-
...getRootBreadcrumbs(baseUrl),
50+
...getRootBreadcrumbs(),
5151
{
5252
text: i18n.translate('discover.context.breadcrumb', {
5353
defaultMessage: `Context of #{docId}`,

‎src/plugins/discover/public/application/components/doc_views/surrounding_docs_view.tsx

+20-18
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export const SurroundingDocsView = ({ id, indexPattern }: SurroundingDocsViewPar
9393
field,
9494
values,
9595
operation,
96-
indexPattern.id
96+
indexPattern.id || ''
9797
);
9898
return filterManager.addFilters(newFilters);
9999
},
@@ -115,23 +115,25 @@ export const SurroundingDocsView = ({ id, indexPattern }: SurroundingDocsViewPar
115115
[onAddFilter, rows, indexPattern, setContextAppState, contextQueryState, contextAppState]
116116
);
117117

118+
if (isLoading) {
119+
return null;
120+
}
121+
118122
return (
119-
!isLoading && (
120-
<Fragment>
121-
<TopNavMenu
122-
appName={'discover.context.surroundingDocs.topNavMenu'}
123-
showSearchBar={true}
124-
showQueryBar={false}
125-
showDatePicker={false}
126-
indexPatterns={[indexPattern]}
127-
useDefaultBehaviors={true}
128-
/>
129-
<EuiPage className="discover.context.appPage">
130-
<EuiPageContent paddingSize="s" className="dscDocsContent">
131-
{contextAppMemoized}
132-
</EuiPageContent>
133-
</EuiPage>
134-
</Fragment>
135-
)
123+
<Fragment>
124+
<TopNavMenu
125+
appName={'discover.context.surroundingDocs.topNavMenu'}
126+
showSearchBar={true}
127+
showQueryBar={false}
128+
showDatePicker={false}
129+
indexPatterns={[indexPattern]}
130+
useDefaultBehaviors={true}
131+
/>
132+
<EuiPage className="discover.context.appPage">
133+
<EuiPageContent paddingSize="s" className="dscDocsContent">
134+
{contextAppMemoized}
135+
</EuiPageContent>
136+
</EuiPage>
137+
</Fragment>
136138
);
137139
};

‎src/plugins/discover/public/application/components/sidebar/discover_field.scss

+4
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,8 @@
1111
&:focus {
1212
opacity: 1;
1313
}
14+
15+
@include ouiBreakpoint("xs", "s", "m") {
16+
opacity: 1;
17+
}
1418
}

‎src/plugins/discover/public/application/components/top_nav/get_top_nav_links.tsx

+5-14
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import { DiscoverState, setSavedSearchId } from '../../utils/state_management';
2020
import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../../common';
2121
import { getSortForSearchSource } from '../../view_components/utils/get_sort_for_search_source';
22+
import { getRootBreadcrumbs } from '../../helpers/breadcrumbs';
2223

2324
export const getTopNavLinks = (
2425
services: DiscoverViewServices,
@@ -45,11 +46,9 @@ export const getTopNavLinks = (
4546
defaultMessage: 'New Search',
4647
}),
4748
run() {
48-
setTimeout(() => {
49-
history().push('/');
50-
// TODO: figure out why a history push doesn't update the app state. The page reload is a hack around it
51-
window.location.reload();
52-
}, 0);
49+
core.application.navigateToApp('discover', {
50+
path: '#/',
51+
});
5352
},
5453
testId: 'discoverNewButton',
5554
};
@@ -103,15 +102,7 @@ export const getTopNavLinks = (
103102
history().push(`/view/${encodeURIComponent(id)}`);
104103
} else {
105104
chrome.docTitle.change(savedSearch.lastSavedTitle);
106-
chrome.setBreadcrumbs([
107-
{
108-
text: i18n.translate('discover.discoverBreadcrumbTitle', {
109-
defaultMessage: 'Discover',
110-
}),
111-
href: '#/',
112-
},
113-
{ text: savedSearch.title },
114-
]);
105+
chrome.setBreadcrumbs([...getRootBreadcrumbs(), { text: savedSearch.title }]);
115106
}
116107

117108
// set App state to clean

‎src/plugins/discover/public/application/components/top_nav/open_search_panel.tsx

+3-7
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ interface Props {
5555
export function OpenSearchPanel({ onClose, makeUrl }: Props) {
5656
const {
5757
services: {
58-
core: { uiSettings, savedObjects },
58+
core: { uiSettings, savedObjects, application },
5959
addBasePath,
6060
},
6161
} = useOpenSearchDashboards<DiscoverViewServices>();
@@ -90,12 +90,8 @@ export function OpenSearchPanel({ onClose, makeUrl }: Props) {
9090
},
9191
]}
9292
onChoose={(id) => {
93-
setTimeout(() => {
94-
window.location.assign(makeUrl(id));
95-
// TODO: figure out why a history push doesn't update the app state. The page reload is a hack around it
96-
window.location.reload();
97-
onClose();
98-
}, 0);
93+
application.navigateToApp('discover', { path: `#/view/${id}` });
94+
onClose();
9995
}}
10096
uiSettings={uiSettings}
10197
savedObjects={savedObjects}

‎src/plugins/discover/public/application/helpers/breadcrumbs.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,17 @@
2929
*/
3030

3131
import { i18n } from '@osd/i18n';
32+
import { EuiBreadcrumb } from '@elastic/eui';
33+
import { getServices } from '../../opensearch_dashboards_services';
3234

33-
export function getRootBreadcrumbs(baseUrl: string) {
35+
export function getRootBreadcrumbs(): EuiBreadcrumb[] {
36+
const { core } = getServices();
3437
return [
3538
{
3639
text: i18n.translate('discover.rootBreadcrumb', {
3740
defaultMessage: 'Discover',
3841
}),
39-
href: baseUrl,
42+
onClick: () => core.application.navigateToApp('discover'),
4043
},
4144
];
4245
}

‎src/plugins/discover/public/application/utils/state_management/discover_slice.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { RootState, DefaultViewState } from '../../../../../data_explorer/public
1111
import { buildColumns } from '../columns';
1212
import * as utils from './common';
1313
import { SortOrder } from '../../../saved_searches/types';
14+
import { PLUGIN_ID } from '../../../../common';
1415

1516
export interface DiscoverState {
1617
/**
@@ -79,6 +80,7 @@ export const getPreloadedState = async ({
7980
preloadedState.root = {
8081
metadata: {
8182
indexPattern: indexPatternId,
83+
view: PLUGIN_ID,
8284
},
8385
};
8486

‎src/plugins/discover/public/application/view_components/canvas/discover_chart_container.tsx

+2-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { DiscoverChart } from '../../components/chart/chart';
1313

1414
export const DiscoverChartContainer = ({ hits, bucketInterval, chartData }: SearchData) => {
1515
const { services } = useOpenSearchDashboards<DiscoverViewServices>();
16-
const { uiSettings, data } = services;
16+
const { uiSettings, data, core } = services;
1717
const { indexPattern, savedSearch } = useDiscoverContext();
1818

1919
const isTimeBased = useMemo(() => (indexPattern ? indexPattern.isTimeBased() : false), [
@@ -30,8 +30,7 @@ export const DiscoverChartContainer = ({ hits, bucketInterval, chartData }: Sear
3030
data={data}
3131
hits={hits}
3232
resetQuery={() => {
33-
window.location.href = `#/view/${savedSearch?.id}`;
34-
window.location.reload();
33+
core.application.navigateToApp('discover', { path: `#/view/${savedSearch?.id}` });
3534
}}
3635
services={services}
3736
showResetButton={!!savedSearch && !!savedSearch.id}

‎src/plugins/discover/public/application/view_components/canvas/index.tsx

+22-23
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
import React, { useEffect, useState, useRef, useCallback } from 'react';
7-
import { EuiFlexGroup, EuiFlexItem, EuiPanel } from '@elastic/eui';
7+
import { EuiPanel } from '@elastic/eui';
88
import { TopNav } from './top_nav';
99
import { ViewProps } from '../../../../../data_explorer/public';
1010
import { DiscoverTable } from './discover_table';
@@ -20,6 +20,7 @@ import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_re
2020
import { filterColumns } from '../utils/filter_columns';
2121
import { DEFAULT_COLUMNS_SETTING } from '../../../../common';
2222
import './discover_canvas.scss';
23+
2324
// eslint-disable-next-line import/no-default-export
2425
export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewProps) {
2526
const { data$, refetch$, indexPattern } = useDiscoverContext();
@@ -77,38 +78,36 @@ export default function DiscoverCanvas({ setHeaderActionMenu, history }: ViewPro
7778
const timeField = indexPattern?.timeFieldName ? indexPattern.timeFieldName : undefined;
7879

7980
return (
80-
<EuiFlexGroup direction="column" gutterSize="none" className="dscCanvas">
81-
<EuiFlexItem grow={false}>
82-
<TopNav
83-
opts={{
84-
setHeaderActionMenu,
85-
onQuerySubmit,
86-
}}
87-
/>
88-
</EuiFlexItem>
81+
<EuiPanel
82+
hasBorder={false}
83+
hasShadow={false}
84+
color="transparent"
85+
paddingSize="none"
86+
className="dscCanvas"
87+
>
88+
<TopNav
89+
opts={{
90+
setHeaderActionMenu,
91+
onQuerySubmit,
92+
}}
93+
/>
8994
{status === ResultStatus.NO_RESULTS && (
90-
<EuiFlexItem>
91-
<DiscoverNoResults timeFieldName={timeField} queryLanguage={''} />
92-
</EuiFlexItem>
95+
<DiscoverNoResults timeFieldName={timeField} queryLanguage={''} />
9396
)}
9497
{status === ResultStatus.UNINITIALIZED && (
9598
<DiscoverUninitialized onRefresh={() => refetch$.next()} />
9699
)}
97100
{status === ResultStatus.LOADING && <LoadingSpinner />}
98101
{status === ResultStatus.READY && (
99102
<>
100-
<EuiFlexItem grow={false}>
101-
<EuiPanel hasBorder={false} hasShadow={false} color="transparent" paddingSize="s">
102-
<EuiPanel>
103-
<DiscoverChartContainer {...fetchState} />
104-
</EuiPanel>
103+
<EuiPanel hasBorder={false} hasShadow={false} color="transparent" paddingSize="s">
104+
<EuiPanel>
105+
<DiscoverChartContainer {...fetchState} />
105106
</EuiPanel>
106-
</EuiFlexItem>
107-
<EuiFlexItem>
108-
<DiscoverTable history={history} />
109-
</EuiFlexItem>
107+
</EuiPanel>
108+
<DiscoverTable history={history} />
110109
</>
111110
)}
112-
</EuiFlexGroup>
111+
</EuiPanel>
113112
);
114113
}

‎src/plugins/discover/public/application/view_components/canvas/top_nav.tsx

+2-5
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,9 @@ export const TopNav = ({ opts }: TopNavProps) => {
6262
chrome.docTitle.change(`Discover${pageTitleSuffix}`);
6363

6464
if (savedSearch?.id) {
65-
chrome.setBreadcrumbs([
66-
...getRootBreadcrumbs(getUrlForApp(PLUGIN_ID)),
67-
{ text: savedSearch.title },
68-
]);
65+
chrome.setBreadcrumbs([...getRootBreadcrumbs(), { text: savedSearch.title }]);
6966
} else {
70-
chrome.setBreadcrumbs([...getRootBreadcrumbs(getUrlForApp(PLUGIN_ID))]);
67+
chrome.setBreadcrumbs([...getRootBreadcrumbs()]);
7168
}
7269
}, [chrome, getUrlForApp, savedSearch?.id, savedSearch?.title]);
7370

‎src/plugins/discover/public/application/view_components/context/index.tsx

+6-4
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ const SearchContext = React.createContext<SearchContextValue>({} as SearchContex
1717

1818
// eslint-disable-next-line import/no-default-export
1919
export default function DiscoverContext({ children }: React.PropsWithChildren<ViewProps>) {
20+
const { services: deServices } = useOpenSearchDashboards<DataExplorerServices>();
2021
const services = getServices();
21-
const searchParams = useSearch(services);
22+
const searchParams = useSearch({
23+
...deServices,
24+
...services,
25+
});
2226

23-
const {
24-
services: { osdUrlStateStorage },
25-
} = useOpenSearchDashboards<DataExplorerServices>();
27+
const { osdUrlStateStorage } = deServices;
2628

2729
// Connect the query service to the url state
2830
useEffect(() => {

0 commit comments

Comments
 (0)