Fix bottom bar visibility using create portal (opensearch-project#3336)

SergeyMyssak · andreymyssak · SergeyMyssak · commit a0139954b454 · 2023-05-07T07:55:13.000+06:00
Co-authored-by: Andrey Myssak &lt;andreymyssak@gmail.com&gt;
Signed-off-by: Sergey Myssak &lt;sergey.myssak@gmail.com&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,13 +39,20 @@ 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';
   },
 }));
 
 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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,6 @@`
`5`	`5`	`*/`
`6`	`6`	`// SASSTODO: Naming here is too embedded and high up that changing them could cause major breaks`
`7`	`7`	`#opensearch-dashboards-body {`
`8`		`- overflow-x: hidden;`
`9`	`8`	`min-height: 100%;`
`10`	`9`	`}`
`11`	`10`
Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,3 @@`
`76`	`76`	`.mgtAdvancedSettingsForm__button {`
`77`	`77`	`width: 100%;`
`78`	`78`	`}`
`79`		`-`
`80`		`-.osdBody--mgtAdvancedSettingsHasBottomBar .mgtPage__body {`
`81`		`- padding-bottom: $euiSizeXL * 2;`
`82`		`-}`