Skip to content

Commit

Permalink
Close any open system flyout when changing view mode of the dashboard
Browse files Browse the repository at this point in the history
Signed-off-by: Miki <miki@amazon.com>
  • Loading branch information
AMoo-Miki committed Jun 28, 2024
1 parent b85e177 commit 517ec4b
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 4 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/6923.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
fix:
- Close any open system flyout when changing view mode of the dashboard ([#6923](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6923))

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/core/public/overlays/flyout/flyout_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const createStartContractMock = () => {
close: jest.fn(),
onClose: Promise.resolve(),
}),
close: jest.fn(),
};
return startContract;
};
Expand Down
37 changes: 37 additions & 0 deletions src/core/public/overlays/flyout/flyout_service.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,43 @@ describe('FlyoutService', () => {
});
});
});

describe('closeFlyout()', () => {
it('resolves onClose on the previous ref', async () => {
const ref = flyouts.open(mountText('Flyout content'));
const onCloseComplete = jest.fn();
ref.onClose.then(onCloseComplete);
flyouts.close();
await ref.onClose;
expect(onCloseComplete).toBeCalledTimes(1);
});

it('can be called multiple times', async () => {
flyouts.open(mountText('Flyout content'));
expect(mockReactDomUnmount).not.toHaveBeenCalled();
flyouts.close();
expect(mockReactDomUnmount.mock.calls).toMatchSnapshot();
flyouts.close();
expect(mockReactDomUnmount).toHaveBeenCalledTimes(1);
});

it("doesn't affect an inactive flyout", async () => {
const ref = flyouts.open(mountText('Flyout content'));
flyouts.close();
const onCloseComplete = jest.fn();
ref.onClose.then(onCloseComplete);
await ref.onClose;

mockReactDomUnmount.mockClear();
onCloseComplete.mockClear();

flyouts.close();
flyouts.close();
expect(mockReactDomUnmount).toBeCalledTimes(0);
expect(onCloseComplete).toBeCalledTimes(0);
});
});

describe('FlyoutRef#close()', () => {
it('resolves the onClose Promise', async () => {
const ref = flyouts.open(mountText('Flyout content'));
Expand Down
11 changes: 11 additions & 0 deletions src/core/public/overlays/flyout/flyout_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ export interface OverlayFlyoutStart {
* @return {@link OverlayRef} A reference to the opened flyout panel.
*/
open(mount: MountPoint, options?: OverlayFlyoutOpenOptions): OverlayRef;

/**
* Closes any open flyout panel.
*/
close(): void;
}

/**
Expand Down Expand Up @@ -149,6 +154,12 @@ export class FlyoutService {

return flyout;
},
close: () => {
if (this.activeFlyout) {
this.activeFlyout.close();
this.cleanupDom();
}
},
};
}

Expand Down
10 changes: 6 additions & 4 deletions src/core/public/overlays/overlay_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,13 @@ import { overlayModalServiceMock } from './modal/modal_service.mock';
import { overlaySidecarServiceMock } from './sidecar/sidecar_service.mock';

const createStartContractMock = () => {
const overlayStart = overlayModalServiceMock.createStartContract();
const overlayModalStart = overlayModalServiceMock.createStartContract();
const overlayFlyoutStart = overlayFlyoutServiceMock.createStartContract();
const startContract: DeeplyMockedKeys<OverlayStart> = {
openFlyout: overlayFlyoutServiceMock.createStartContract().open,
openModal: overlayStart.open,
openConfirm: overlayStart.openConfirm,
openFlyout: overlayFlyoutStart.open,
closeFlyout: overlayFlyoutStart.close,
openModal: overlayModalStart.open,
openConfirm: overlayModalStart.openConfirm,
banners: overlayBannersServiceMock.createStartContract(),
sidecar: overlaySidecarServiceMock.createStartContract(),
};
Expand Down
3 changes: 3 additions & 0 deletions src/core/public/overlays/overlay_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class OverlayService {
return {
banners,
openFlyout: flyouts.open.bind(flyouts),
closeFlyout: flyouts.close.bind(flyouts),
openModal: modals.open.bind(modals),
openConfirm: modals.openConfirm.bind(modals),
sidecar: sidecars,
Expand All @@ -81,6 +82,8 @@ export interface OverlayStart {
banners: OverlayBannersStart;
/** {@link OverlayFlyoutStart#open} */
openFlyout: OverlayFlyoutStart['open'];
/** {@link OverlayFlyoutStart#close} */
closeFlyout: OverlayFlyoutStart['close'];
/** {@link OverlayModalStart#open} */
openModal: OverlayModalStart['open'];
/** {@link OverlayModalStart#openConfirm} */
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ export const getNavActions = (

// If there are no changes, do not show the discard window
if (!willLoseChanges) {
overlays.closeFlyout();

Check warning on line 311 in src/plugins/dashboard/public/application/utils/get_nav_actions.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/get_nav_actions.tsx#L311

Added line #L311 was not covered by tests
stateContainer.transitions.set('viewMode', newMode);
return;
}
Expand Down Expand Up @@ -349,6 +350,8 @@ export const getNavActions = (
}
}

overlays.closeFlyout();

Check warning on line 353 in src/plugins/dashboard/public/application/utils/get_nav_actions.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/dashboard/public/application/utils/get_nav_actions.tsx#L353

Added line #L353 was not covered by tests

// Set the isDirty flag back to false since we discard all the changes
dashboard.setIsDirty(false);
}
Expand Down

0 comments on commit 517ec4b

Please sign in to comment.