Skip to content

Commit

Permalink
[Workplace Search] Bypass UnsavedChangesPrompt for tab changes in Dis…
Browse files Browse the repository at this point in the history 
…play Settings (#97062)

* Move redirect logic into logic file

* Add logic to prevent prompt from triggering when changing tabs

The idea here is to set a boolean flag that sends false for unsavedChanges when switching between tabs and then sets it back after a successful tab change

* Keep sidebar nav item active for both tabs

* Add tests
  • Loading branch information
@scottybollinger
scottybollinger authored Apr 14, 2021
1 parent 0965366 commit 3bc2952
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 26 deletions.
8 changes: 4 additions & 4 deletions ...kplace_search/views/content_sources/components/display_settings/display_settings.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import '../../../../../__mocks__/shallow_useeffect.mock';

import { mockKibanaValues } from '../../../../../__mocks__';
import { setMockValues, setMockActions } from '../../../../../__mocks__';
import { exampleResult } from '../../../../__mocks__/content_sources.mock';

Expand All @@ -25,11 +24,11 @@ import { DisplaySettings } from './display_settings';
import { FieldEditorModal } from './field_editor_modal';

describe('DisplaySettings', () => {
const { navigateToUrl } = mockKibanaValues;
const { exampleDocuments, searchResultConfig } = exampleResult;
const initializeDisplaySettings = jest.fn();
const setServerData = jest.fn();
const setColorField = jest.fn();
const handleSelectedTabChanged = jest.fn();

const values = {
isOrganization: true,
Expand All @@ -46,6 +45,7 @@ describe('DisplaySettings', () => {
initializeDisplaySettings,
setServerData,
setColorField,
handleSelectedTabChanged,
});
setMockValues({ ...values });
});
Expand Down Expand Up @@ -83,15 +83,15 @@ describe('DisplaySettings', () => {
const tabsEl = wrapper.find(EuiTabbedContent);
tabsEl.prop('onTabClick')!(tabs[0]);

expect(navigateToUrl).toHaveBeenCalledWith('/sources/123/display_settings/');
expect(handleSelectedTabChanged).toHaveBeenCalledWith('search_results');
});

it('handles second tab click', () => {
const wrapper = shallow(<DisplaySettings tabId={0} />);
const tabsEl = wrapper.find(EuiTabbedContent);
tabsEl.prop('onTabClick')!(tabs[1]);

expect(navigateToUrl).toHaveBeenCalledWith('/sources/123/display_settings/result_detail');
expect(handleSelectedTabChanged).toHaveBeenCalledWith('result_detail');
});
});

Expand Down
28 changes: 8 additions & 20 deletions ...s/workplace_search/views/content_sources/components/display_settings/display_settings.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,11 @@ import {
} from '@elastic/eui';

import { clearFlashMessages } from '../../../../../shared/flash_messages';
import { KibanaLogic } from '../../../../../shared/kibana';
import { Loading } from '../../../../../shared/loading';
import { UnsavedChangesPrompt } from '../../../../../shared/unsaved_changes_prompt';
import { AppLogic } from '../../../../app_logic';
import { ViewContentHeader } from '../../../../components/shared/view_content_header';
import { SAVE_BUTTON } from '../../../../constants';

import {
DISPLAY_SETTINGS_RESULT_DETAIL_PATH,
DISPLAY_SETTINGS_SEARCH_RESULT_PATH,
getContentSourcePath,
} from '../../../../routes';

import {
UNSAVED_MESSAGE,
DISPLAY_SETTINGS_TITLE,
Expand All @@ -42,7 +34,7 @@ import {
SEARCH_RESULTS_LABEL,
RESULT_DETAIL_LABEL,
} from './constants';
import { DisplaySettingsLogic } from './display_settings_logic';
import { DisplaySettingsLogic, TabId } from './display_settings_logic';
import { FieldEditorModal } from './field_editor_modal';
import { ResultDetail } from './result_detail';
import { SearchResults } from './search_results';
Expand All @@ -52,19 +44,20 @@ interface DisplaySettingsProps {
}

export const DisplaySettings: React.FC<DisplaySettingsProps> = ({ tabId }) => {
const { initializeDisplaySettings, setServerData } = useActions(DisplaySettingsLogic);
const { initializeDisplaySettings, setServerData, handleSelectedTabChanged } = useActions(
DisplaySettingsLogic
);

const {
dataLoading,
sourceId,
addFieldModalVisible,
unsavedChanges,
exampleDocuments,
navigatingBetweenTabs,
} = useValues(DisplaySettingsLogic);

const { isOrganization } = useValues(AppLogic);

const hasDocuments = exampleDocuments.length > 0;
const hasUnsavedChanges = hasDocuments && unsavedChanges;

useEffect(() => {
initializeDisplaySettings();
Expand All @@ -87,12 +80,7 @@ export const DisplaySettings: React.FC<DisplaySettingsProps> = ({ tabId }) => {
] as EuiTabbedContentTab[];

const onSelectedTabChanged = (tab: EuiTabbedContentTab) => {
const path =
tab.id === tabs[1].id
? getContentSourcePath(DISPLAY_SETTINGS_RESULT_DETAIL_PATH, sourceId, isOrganization)
: getContentSourcePath(DISPLAY_SETTINGS_SEARCH_RESULT_PATH, sourceId, isOrganization);

KibanaLogic.values.navigateToUrl(path);
handleSelectedTabChanged(tab.id as TabId);
};

const handleFormSubmit = (e: FormEvent) => {
Expand All @@ -103,7 +91,7 @@ export const DisplaySettings: React.FC<DisplaySettingsProps> = ({ tabId }) => {
return (
<>
<UnsavedChangesPrompt
hasUnsavedChanges={hasDocuments && unsavedChanges}
hasUnsavedChanges={!navigatingBetweenTabs && hasUnsavedChanges}
messageText={UNSAVED_MESSAGE}
/>
<form onSubmit={handleFormSubmit}>
Expand Down
40 changes: 39 additions & 1 deletion ...e_search/views/content_sources/components/display_settings/display_settings_logic.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* 2.0.
*/

import { LogicMounter, mockFlashMessageHelpers, mockHttpValues } from '../../../../../__mocks__';
import {
LogicMounter,
mockFlashMessageHelpers,
mockHttpValues,
mockKibanaValues,
} from '../../../../../__mocks__';
import { exampleResult } from '../../../../__mocks__/content_sources.mock';

import { nextTick } from '@kbn/test/jest';
Expand All @@ -25,6 +30,7 @@ import { DisplaySettingsLogic, defaultSearchResultConfig } from './display_setti

describe('DisplaySettingsLogic', () => {
const { http } = mockHttpValues;
const { navigateToUrl } = mockKibanaValues;
const { clearFlashMessages, flashAPIErrors, setSuccessMessage } = mockFlashMessageHelpers;
const { mount } = new LogicMounter(DisplaySettingsLogic);

Expand All @@ -40,6 +46,7 @@ describe('DisplaySettingsLogic', () => {
serverRoute: '',
editFieldIndex: null,
dataLoading: true,
navigatingBetweenTabs: false,
addFieldModalVisible: false,
titleFieldHover: false,
urlFieldHover: false,
Expand Down Expand Up @@ -203,6 +210,12 @@ describe('DisplaySettingsLogic', () => {
});
});

it('setNavigatingBetweenTabs', () => {
DisplaySettingsLogic.actions.setNavigatingBetweenTabs(true);

expect(DisplaySettingsLogic.values.navigatingBetweenTabs).toEqual(true);
});

it('addDetailField', () => {
const newField = { label: 'Monkey', fieldName: 'primate' };
DisplaySettingsLogic.actions.setServerResponseData(serverProps);
Expand Down Expand Up @@ -351,6 +364,31 @@ describe('DisplaySettingsLogic', () => {
expect(flashAPIErrors).toHaveBeenCalledWith('this is an error');
});
});

describe('handleSelectedTabChanged', () => {
beforeEach(() => {
DisplaySettingsLogic.actions.onInitializeDisplaySettings(serverProps);
});

it('calls sets navigatingBetweenTabs', async () => {
const setNavigatingBetweenTabsSpy = jest.spyOn(
DisplaySettingsLogic.actions,
'setNavigatingBetweenTabs'
);
DisplaySettingsLogic.actions.handleSelectedTabChanged('search_results');
await nextTick();

expect(setNavigatingBetweenTabsSpy).toHaveBeenCalledWith(true);
expect(navigateToUrl).toHaveBeenCalledWith('/p/sources/123/display_settings/');
});

it('calls calls correct route for "result_detail"', async () => {
DisplaySettingsLogic.actions.handleSelectedTabChanged('result_detail');
await nextTick();

expect(navigateToUrl).toHaveBeenCalledWith('/p/sources/123/display_settings/result_detail');
});
});
});

describe('selectors', () => {
Expand Down
39 changes: 39 additions & 0 deletions ...kplace_search/views/content_sources/components/display_settings/display_settings_logic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ import {
flashAPIErrors,
} from '../../../../../shared/flash_messages';
import { HttpLogic } from '../../../../../shared/http';
import { KibanaLogic } from '../../../../../shared/kibana';
import { AppLogic } from '../../../../app_logic';
import {
DISPLAY_SETTINGS_RESULT_DETAIL_PATH,
DISPLAY_SETTINGS_SEARCH_RESULT_PATH,
getContentSourcePath,
} from '../../../../routes';
import { DetailField, SearchResultConfig, OptionValue, Result } from '../../../../types';
import { SourceLogic } from '../../source_logic';

Expand All @@ -34,6 +40,8 @@ export interface DisplaySettingsInitialData extends DisplaySettingsResponseProps
serverRoute: string;
}

export type TabId = 'search_results' | 'result_detail';

interface DisplaySettingsActions {
initializeDisplaySettings(): void;
setServerData(): void;
Expand All @@ -51,6 +59,8 @@ interface DisplaySettingsActions {
setDetailFields(result: DropResult): { result: DropResult };
openEditDetailField(editFieldIndex: number | null): number | null;
removeDetailField(index: number): number;
setNavigatingBetweenTabs(navigatingBetweenTabs: boolean): boolean;
handleSelectedTabChanged(tabId: TabId): TabId;
addDetailField(newField: DetailField): DetailField;
updateDetailField(
updatedField: DetailField,
Expand All @@ -73,6 +83,7 @@ interface DisplaySettingsValues {
serverRoute: string;
editFieldIndex: number | null;
dataLoading: boolean;
navigatingBetweenTabs: boolean;
addFieldModalVisible: boolean;
titleFieldHover: boolean;
urlFieldHover: boolean;
Expand Down Expand Up @@ -109,6 +120,8 @@ export const DisplaySettingsLogic = kea<
setDetailFields: (result: DropResult) => ({ result }),
openEditDetailField: (editFieldIndex: number | null) => editFieldIndex,
removeDetailField: (index: number) => index,
setNavigatingBetweenTabs: (navigatingBetweenTabs: boolean) => navigatingBetweenTabs,
handleSelectedTabChanged: (tabId: TabId) => tabId,
addDetailField: (newField: DetailField) => newField,
updateDetailField: (updatedField: DetailField, index: number) => ({ updatedField, index }),
toggleFieldEditorModal: () => true,
Expand Down Expand Up @@ -224,6 +237,12 @@ export const DisplaySettingsLogic = kea<
onInitializeDisplaySettings: () => false,
},
],
navigatingBetweenTabs: [
false,
{
setNavigatingBetweenTabs: (_, navigatingBetweenTabs) => navigatingBetweenTabs,
},
],
addFieldModalVisible: [
false,
{
Expand Down Expand Up @@ -330,6 +349,26 @@ export const DisplaySettingsLogic = kea<
toggleFieldEditorModal: () => {
clearFlashMessages();
},

handleSelectedTabChanged: async (tabId, breakpoint) => {
const { isOrganization } = AppLogic.values;
const { sourceId } = values;
const path =
tabId === 'result_detail'
? getContentSourcePath(DISPLAY_SETTINGS_RESULT_DETAIL_PATH, sourceId, isOrganization)
: getContentSourcePath(DISPLAY_SETTINGS_SEARCH_RESULT_PATH, sourceId, isOrganization);

// This method is needed because the shared `UnsavedChangesPrompt` component is triggered
// when navigating between tabs. We set a boolean flag that tells the prompt there are no
// unsaved changes when navigating between the tabs and reset it one the transition is complete
// in order to restore the intended functionality when navigating away with unsaved changes.
actions.setNavigatingBetweenTabs(true);

await breakpoint();

KibanaLogic.values.navigateToUrl(path);
actions.setNavigatingBetweenTabs(false);
},
}),
});

Expand Down
5 changes: 4 additions & 1 deletion .../public/applications/workplace_search/views/content_sources/components/source_sub_nav.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ export const SourceSubNav: React.FC = () => {
<SideNavLink to={getContentSourcePath(SOURCE_SCHEMAS_PATH, id, isOrganization)}>
{NAV.SCHEMA}
</SideNavLink>
<SideNavLink to={getContentSourcePath(SOURCE_DISPLAY_SETTINGS_PATH, id, isOrganization)}>
<SideNavLink
shouldShowActiveForSubroutes
to={getContentSourcePath(SOURCE_DISPLAY_SETTINGS_PATH, id, isOrganization)}
>
{NAV.DISPLAY_SETTINGS}
</SideNavLink>
</>
Expand Down

0 comments on commit 3bc2952

Please sign in to comment.