Skip to content

Commit

Permalink
Fix bottom bar visibility using create portal (#3336)
Browse files Browse the repository at this point in the history
Co-authored-by: Andrey Myssak <andreymyssak@gmail.com>
Signed-off-by: Sergey Myssak <sergey.myssak@gmail.com>
  • Loading branch information
SergeyMyssak and andreymyssak committed May 4, 2023
1 parent ca0bb8f commit 994135c
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 66 deletions.
1 change: 0 additions & 1 deletion src/core/public/rendering/_base.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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%;
}

Expand Down
3 changes: 3 additions & 0 deletions src/core/public/rendering/app_containers.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('AppWrapper', () => {
expect(component.getDOMNode()).toMatchInlineSnapshot(`
<div
class="app-wrapper"
id="app-wrapper"
>
app-content
</div>
Expand All @@ -53,6 +54,7 @@ describe('AppWrapper', () => {
expect(component.getDOMNode()).toMatchInlineSnapshot(`
<div
class="app-wrapper hidden-chrome"
id="app-wrapper"
>
app-content
</div>
Expand All @@ -63,6 +65,7 @@ describe('AppWrapper', () => {
expect(component.getDOMNode()).toMatchInlineSnapshot(`
<div
class="app-wrapper"
id="app-wrapper"
>
app-content
</div>
Expand Down
6 changes: 5 additions & 1 deletion src/core/public/rendering/app_containers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,3 @@
.mgtAdvancedSettingsForm__button {
width: 100%;
}

.osdBody--mgtAdvancedSettingsHasBottomBar .mgtPage__body {
padding-bottom: $euiSizeXL * 2;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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,
};
Expand All @@ -60,6 +68,7 @@ beforeAll(() => {
});

afterAll(() => {
(ReactDOM.createPortal as jest.Mock).mockClear();
delete (window as any).localStorage;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down

0 comments on commit 994135c

Please sign in to comment.