Skip to content
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

Close any open flyouts when changing view mode of the dashboard #6923

Merged
merged 1 commit into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@

// 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 @@
}
}

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
Loading