From 87e795123a36958e7c2596a12f58d775bba2beff Mon Sep 17 00:00:00 2001 From: Sergey Myssak <sergey.myssak@gmail.com> Date: Tue, 16 May 2023 06:18:59 +0600 Subject: [PATCH] Fix bottom bar visibility using create portal (#3336) (#3978) Signed-off-by: Sergey Myssak <sergey.myssak@gmail.com> Co-authored-by: Andrey Myssak <andreymyssak@gmail.com> --- CHANGELOG.md | 1 + src/core/public/rendering/_base.scss | 1 - .../public/rendering/app_containers.test.tsx | 3 + src/core/public/rendering/app_containers.tsx | 6 +- .../management_app/_advanced_settings.scss | 4 - .../components/form/form.test.tsx | 9 ++ .../management_app/components/form/form.tsx | 121 +++++++++--------- 7 files changed, 79 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f49fa05b2c6f..0e7670ad7a16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Table Visualization] Fix table rendering empty unused space ([#3797](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3797)) - [Table Visualization] Fix data table not adjusting height on the initial load ([#3816](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3816)) - Cleanup unused url ([#3847](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3847)) +- [BUG] Docked navigation impacts visibility of bottom bar component ([#3978](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3978)) ### 🚞 Infrastructure diff --git a/src/core/public/rendering/_base.scss b/src/core/public/rendering/_base.scss index e59008f08259..1333e48f6ca5 100644 --- a/src/core/public/rendering/_base.scss +++ b/src/core/public/rendering/_base.scss @@ -5,7 +5,6 @@ */ // SASSTODO: Naming here is too embedded and high up that changing them could cause major breaks #opensearch-dashboards-body { - overflow-x: hidden; min-height: 100%; } diff --git a/src/core/public/rendering/app_containers.test.tsx b/src/core/public/rendering/app_containers.test.tsx index fd43c79514c1..157253f5f757 100644 --- a/src/core/public/rendering/app_containers.test.tsx +++ b/src/core/public/rendering/app_containers.test.tsx @@ -43,6 +43,7 @@ describe('AppWrapper', () => { expect(component.getDOMNode()).toMatchInlineSnapshot(` <div class="app-wrapper" + id="app-wrapper" > app-content </div> @@ -53,6 +54,7 @@ describe('AppWrapper', () => { expect(component.getDOMNode()).toMatchInlineSnapshot(` <div class="app-wrapper hidden-chrome" + id="app-wrapper" > app-content </div> @@ -63,6 +65,7 @@ describe('AppWrapper', () => { expect(component.getDOMNode()).toMatchInlineSnapshot(` <div class="app-wrapper" + id="app-wrapper" > app-content </div> diff --git a/src/core/public/rendering/app_containers.tsx b/src/core/public/rendering/app_containers.tsx index dab85769bd0b..8ccf795f8dcf 100644 --- a/src/core/public/rendering/app_containers.tsx +++ b/src/core/public/rendering/app_containers.tsx @@ -37,7 +37,11 @@ export const AppWrapper: React.FunctionComponent<{ chromeVisible$: Observable<boolean>; }> = ({ chromeVisible$, children }) => { const visible = useObservable(chromeVisible$); - return <div className={classNames('app-wrapper', { 'hidden-chrome': !visible })}>{children}</div>; + return ( + <div id="app-wrapper" className={classNames('app-wrapper', { 'hidden-chrome': !visible })}> + {children} + </div> + ); }; export const AppContainer: React.FunctionComponent<{ diff --git a/src/plugins/advanced_settings/public/management_app/_advanced_settings.scss b/src/plugins/advanced_settings/public/management_app/_advanced_settings.scss index 82704fc93062..a33082c3ef64 100644 --- a/src/plugins/advanced_settings/public/management_app/_advanced_settings.scss +++ b/src/plugins/advanced_settings/public/management_app/_advanced_settings.scss @@ -76,7 +76,3 @@ .mgtAdvancedSettingsForm__button { width: 100%; } - -.osdBody--mgtAdvancedSettingsHasBottomBar .mgtPage__body { - padding-bottom: $euiSizeXL * 2; -} diff --git a/src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx b/src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx index 7f22894fb8a6..a0edaa5ab602 100644 --- a/src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx +++ b/src/plugins/advanced_settings/public/management_app/components/form/form.test.tsx @@ -29,6 +29,7 @@ */ import React from 'react'; +import ReactDOM from 'react-dom'; import { shallowWithI18nProvider, mountWithI18nProvider } from 'test_utils/enzyme_helpers'; import { UiSettingsType } from '../../../../../../core/public'; @@ -38,6 +39,11 @@ import { notificationServiceMock } from '../../../../../../core/public/mocks'; import { SettingsChanges } from '../../types'; import { Form } from './form'; +jest.mock('react-dom', () => ({ + ...jest.requireActual('react-dom'), + createPortal: jest.fn((element) => element), +})); + jest.mock('../field', () => ({ Field: () => { return 'field'; @@ -45,6 +51,8 @@ jest.mock('../field', () => ({ })); beforeAll(() => { + ReactDOM.createPortal = jest.fn((children: any) => children); + const localStorage: Record<string, any> = { 'core.chrome.isLocked': true, }; @@ -60,6 +68,7 @@ beforeAll(() => { }); afterAll(() => { + (ReactDOM.createPortal as jest.Mock).mockClear(); delete (window as any).localStorage; }); diff --git a/src/plugins/advanced_settings/public/management_app/components/form/form.tsx b/src/plugins/advanced_settings/public/management_app/components/form/form.tsx index 92b7a792d2d2..a74199771d2a 100644 --- a/src/plugins/advanced_settings/public/management_app/components/form/form.tsx +++ b/src/plugins/advanced_settings/public/management_app/components/form/form.tsx @@ -47,8 +47,9 @@ import { import { FormattedMessage } from '@osd/i18n/react'; import { isEmpty } from 'lodash'; import { i18n } from '@osd/i18n'; +import { DocLinksStart, ToastsStart } from 'opensearch-dashboards/public'; +import { createPortal } from 'react-dom'; import { toMountPoint } from '../../../../../opensearch_dashboards_react/public'; -import { DocLinksStart, ToastsStart } from '../../../../../../core/public'; import { getCategoryName } from '../../lib'; import { Field, getEditableValue } from '../field'; @@ -336,63 +337,69 @@ export class Form extends PureComponent<FormProps> { }; renderBottomBar = () => { - const areChangesInvalid = this.areChangesInvalid(); - return ( - <EuiBottomBar data-test-subj="advancedSetting-bottomBar"> - <EuiFlexGroup - justifyContent="spaceBetween" - alignItems="center" - responsive={false} - gutterSize="s" - > - <EuiFlexItem grow={false} className="mgtAdvancedSettingsForm__unsavedCount"> - <p id="aria-describedby.countOfUnsavedSettings">{this.renderCountOfUnsaved()}</p> - </EuiFlexItem> - <EuiFlexItem /> - <EuiFlexItem grow={false}> - <EuiButtonEmpty - color="ghost" - size="s" - iconType="cross" - onClick={this.clearAllUnsaved} - aria-describedby="aria-describedby.countOfUnsavedSettings" - data-test-subj="advancedSetting-cancelButton" - > - {i18n.translate('advancedSettings.form.cancelButtonLabel', { - defaultMessage: 'Cancel changes', - })} - </EuiButtonEmpty> - </EuiFlexItem> - <EuiFlexItem grow={false}> - <EuiToolTip - content={ - areChangesInvalid && - i18n.translate('advancedSettings.form.saveButtonTooltipWithInvalidChanges', { - defaultMessage: 'Fix invalid settings before saving.', - }) - } - > - <EuiButton - className="mgtAdvancedSettingsForm__button" - disabled={areChangesInvalid} - color="secondary" - fill + try { + const areChangesInvalid = this.areChangesInvalid(); + const bottomBar = ( + <EuiBottomBar data-test-subj="advancedSetting-bottomBar" position="sticky"> + <EuiFlexGroup + justifyContent="spaceBetween" + alignItems="center" + responsive={false} + gutterSize="s" + > + <EuiFlexItem grow={false} className="mgtAdvancedSettingsForm__unsavedCount"> + <p id="aria-describedby.countOfUnsavedSettings">{this.renderCountOfUnsaved()}</p> + </EuiFlexItem> + <EuiFlexItem /> + <EuiFlexItem grow={false}> + <EuiButtonEmpty + color="ghost" size="s" - iconType="check" - onClick={this.saveAll} + iconType="cross" + onClick={this.clearAllUnsaved} aria-describedby="aria-describedby.countOfUnsavedSettings" - isLoading={this.state.loading} - data-test-subj="advancedSetting-saveButton" + data-test-subj="advancedSetting-cancelButton" > - {i18n.translate('advancedSettings.form.saveButtonLabel', { - defaultMessage: 'Save changes', + {i18n.translate('advancedSettings.form.cancelButtonLabel', { + defaultMessage: 'Cancel changes', })} - </EuiButton> - </EuiToolTip> - </EuiFlexItem> - </EuiFlexGroup> - </EuiBottomBar> - ); + </EuiButtonEmpty> + </EuiFlexItem> + <EuiFlexItem grow={false}> + <EuiToolTip + content={ + areChangesInvalid && + i18n.translate('advancedSettings.form.saveButtonTooltipWithInvalidChanges', { + defaultMessage: 'Fix invalid settings before saving.', + }) + } + > + <EuiButton + className="mgtAdvancedSettingsForm__button" + disabled={areChangesInvalid} + color="secondary" + fill + size="s" + iconType="check" + onClick={this.saveAll} + aria-describedby="aria-describedby.countOfUnsavedSettings" + isLoading={this.state.loading} + data-test-subj="advancedSetting-saveButton" + > + {i18n.translate('advancedSettings.form.saveButtonLabel', { + defaultMessage: 'Save changes', + })} + </EuiButton> + </EuiToolTip> + </EuiFlexItem> + </EuiFlexGroup> + </EuiBottomBar> + ); + + return createPortal(bottomBar, document.getElementById('app-wrapper')!); + } catch (e) { + return null; + } }; render() { @@ -401,12 +408,6 @@ export class Form extends PureComponent<FormProps> { const currentCategories: Category[] = []; const hasUnsavedChanges = !isEmpty(unsavedChanges); - if (hasUnsavedChanges) { - document.body.classList.add('osdBody--mgtAdvancedSettingsHasBottomBar'); - } else { - document.body.classList.remove('osdBody--mgtAdvancedSettingsHasBottomBar'); - } - categories.forEach((category) => { if (visibleSettings[category] && visibleSettings[category].length) { currentCategories.push(category);