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

[CCI] Allow to portale BottomBar to any referenced DOM element #758

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

### 📈 Features/Enhancements

- Allow to portale BottomBar to any referenced DOM element ([#758](https://github.com/opensearch-project/oui/pull/758))
- Update ouiTextSubduedColor in `next` dark theme ([#973](https://github.com/opensearch-project/oui/pull/973))

### 🐛 Bug Fixes
Expand Down
1 change: 1 addition & 0 deletions scripts/jest/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
},
"setupFiles": [
"<rootDir>/scripts/jest/setup/enzyme.js",
"<rootDir>/scripts/jest/setup/html_element.js",
joshuarrrr marked this conversation as resolved.
Show resolved Hide resolved
"<rootDir>/scripts/jest/setup/throw_on_console_error.js"
],
"setupFilesAfterEnv": [
Expand Down
24 changes: 24 additions & 0 deletions scripts/jest/setup/html_element.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

if (typeof window !== 'undefined') {
HTMLElement.prototype.insertAdjacentElement = function (position, element) {
switch (position) {
case 'beforebegin':
this.parentNode.insertBefore(element, this);
break;
case 'afterend':
if (this.nextSibling) {
this.parentNode.insertBefore(element, this.nextSibling);
} else {
this.parentNode.appendChild(element);
}
break;
// add other cases if needed
default:
throw new Error(`Unsupported position: ${position}`);
}
};
}
63 changes: 42 additions & 21 deletions src/components/bottom_bar/__snapshots__/bottom_bar.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,48 @@ exports[`OuiBottomBar props bodyClassName is rendered 1`] = `
</section>
`;

exports[`OuiBottomBar props insert root prop is altered 1`] = `
Array [
<section
aria-label="Page level controls"
class="ouiBottomBar ouiBottomBar--fixed ouiBottomBar--paddingMedium"
>
<h2
class="ouiScreenReaderOnly"
>
Page level controls
</h2>
</section>,
<p
aria-live="assertive"
class="ouiScreenReaderOnly"
>
There is a new region landmark with page level controls at the end of the document.
</p>,
]
`;

exports[`OuiBottomBar props insert sibling prop is altered 1`] = `
Array [
<section
aria-label="Page level controls"
class="ouiBottomBar ouiBottomBar--fixed ouiBottomBar--paddingMedium"
>
<h2
class="ouiScreenReaderOnly"
>
Page level controls
</h2>
</section>,
<p
aria-live="assertive"
class="ouiScreenReaderOnly"
>
There is a new region landmark with page level controls at the end of the document.
</p>,
]
`;

exports[`OuiBottomBar props landmarkHeading 1`] = `
Array [
<section
Expand Down Expand Up @@ -268,24 +310,3 @@ Array [
</p>,
]
`;

exports[`OuiBottomBar props usePortal can be false 1`] = `
Array [
<section
aria-label="Page level controls"
class="ouiBottomBar ouiBottomBar--fixed ouiBottomBar--paddingMedium"
>
<h2
class="ouiScreenReaderOnly"
>
Page level controls
</h2>
</section>,
<p
aria-live="assertive"
class="ouiScreenReaderOnly"
>
There is a new region landmark with page level controls at the end of the document.
</p>,
]
`;
14 changes: 12 additions & 2 deletions src/components/bottom_bar/bottom_bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,18 @@ describe('OuiBottomBar', () => {
expect(component).toMatchSnapshot();
});

test('usePortal can be false', () => {
const component = render(<OuiBottomBar usePortal={false} />);
test('insert root prop is altered', () => {
const component = render(
<OuiBottomBar insert={{ root: document.body }} />
);

expect(component).toMatchSnapshot();
});

test('insert sibling prop is altered', () => {
const component = render(
<OuiBottomBar insert={{ sibling: document.body, position: 'after' }} />
);

expect(component).toMatchSnapshot();
});
Expand Down
56 changes: 25 additions & 31 deletions src/components/bottom_bar/bottom_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ import React, {
} from 'react';
import { useCombinedRefs } from '../../services';
import { OuiScreenReaderOnly } from '../accessibility';
import { CommonProps, ExclusiveUnion } from '../common';
import { CommonProps } from '../common';
import { OuiI18n } from '../i18n';
import { useResizeObserver } from '../observer/resize_observer';
import { OuiPortal } from '../portal';
import { OuiPortal, OuiPortalInsert } from '../portal';

type BottomBarPaddingSize = 'none' | 's' | 'm' | 'l';

Expand All @@ -58,32 +58,30 @@ export const paddingSizeToClassNameMap: {
export const POSITIONS = ['static', 'fixed', 'sticky'] as const;
export type _BottomBarPosition = typeof POSITIONS[number];

type _BottomBarExclusivePositions = ExclusiveUnion<
{
position?: 'fixed';
export type OuiBottomBarProps = CommonProps &
HTMLAttributes<HTMLElement> & {
/**
* Whether to wrap in an OuiPortal which appends the component to the body element.
* Only works if `position` is `fixed`.
* How to position the bottom bar against its parent.
* Defaults to `fixed`.
*/
position?: 'static' | 'sticky' | 'fixed';
/**
* Whether to wrap in OuiPortal. Can be configured using "insert" prop.
*/
usePortal?: boolean;
/**
* Whether the component should apply padding on the document body element to afford for its own displacement height.
* Only works if `usePortal` is true and `position` is `fixed`.
* Configuration for placing children in the DOM. By default, attaches children to the body element.
* Only works if `usePortal` is true.
*/
affordForDisplacement?: boolean;
},
{
insert?: OuiPortalInsert;
/**
* How to position the bottom bar against its parent.
* Whether to apply padding to the document body to afford for its own displacement height.
* Only works if `position` is `fixed`.
*/
position: 'static' | 'sticky';
}
>;
export type OuiBottomBarProps = CommonProps &
HTMLAttributes<HTMLElement> &
_BottomBarExclusivePositions & {
affordForDisplacement?: boolean;
/**
* Padding applied to the bar. Default is 'm'.
* Padding applied to the bar.
* Defaults to 'm'.
*/
paddingSize?: BottomBarPaddingSize;
/**
Expand Down Expand Up @@ -126,12 +124,13 @@ export const OuiBottomBar = forwardRef<
{
position = 'fixed',
paddingSize = 'm',
affordForDisplacement = true,
affordForDisplacement,
children,
className,
bodyClassName,
landmarkHeading,
usePortal = true,
usePortal,
insert,
left,
right,
bottom,
Expand All @@ -141,18 +140,13 @@ export const OuiBottomBar = forwardRef<
},
ref
) => {
// Force some props if `fixed` position, but not if the user has supplied these
affordForDisplacement =
position !== 'fixed' ? false : affordForDisplacement;
usePortal = position !== 'fixed' ? false : usePortal;

const [resizeRef, setResizeRef] = useState<HTMLElement | null>(null);
const setRef = useCombinedRefs([setResizeRef, ref]);
// TODO: Allow this hooke to be conditional
const dimensions = useResizeObserver(resizeRef);

useEffect(() => {
if (affordForDisplacement && usePortal) {
if (affordForDisplacement) {
document.body.style.paddingBottom = `${dimensions.height}px`;
}

Expand All @@ -161,15 +155,15 @@ export const OuiBottomBar = forwardRef<
}

return () => {
if (affordForDisplacement && usePortal) {
if (affordForDisplacement) {
document.body.style.paddingBottom = '';
}

if (bodyClassName) {
document.body.classList.remove(bodyClassName);
}
};
}, [affordForDisplacement, usePortal, dimensions, bodyClassName]);
}, [affordForDisplacement, dimensions, bodyClassName]);

const classes = classNames(
'ouiBottomBar',
Expand Down Expand Up @@ -230,7 +224,7 @@ export const OuiBottomBar = forwardRef<
</>
);

return usePortal ? <OuiPortal>{bar}</OuiPortal> : bar;
return usePortal ? <OuiPortal insert={insert}>{bar}</OuiPortal> : bar;
}
);

Expand Down
7 changes: 2 additions & 5 deletions src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { OuiScreenReaderOnly } from '../accessibility';

import { OuiPanel, PanelPaddingSize, OuiPanelProps } from '../panel';

import { OuiPortal } from '../portal';
import { OuiPortal, OuiPortalInsert } from '../portal';

import { OuiMutationObserver } from '../observer/mutation_observer';

Expand Down Expand Up @@ -141,10 +141,7 @@ export interface OuiPopoverProps {
* Passed directly to OuiPortal for DOM positioning. Both properties are
* required if prop is specified
*/
insert?: {
sibling: HTMLElement;
position: 'before' | 'after';
};
insert?: OuiPortalInsert;
/**
* Visibility state of the popover
*/
Expand Down
26 changes: 12 additions & 14 deletions src/components/portal/__snapshots__/portal.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`OuiPortal is rendered 1`] = `
<div>
<OuiPortal>
<Portal
containerInfo={
<div>
Content
</div>
}
>
Content
</Portal>
</OuiPortal>
</div>
exports[`OuiPortal should render OuiPortal 1`] = `
<OuiPortal>
<Portal
containerInfo={
<div>
Content
</div>
}
>
Content
</Portal>
</OuiPortal>
`;
2 changes: 1 addition & 1 deletion src/components/portal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@
* under the License.
*/

export { OuiPortal, OuiPortalProps } from './portal';
export { OuiPortal, OuiPortalProps, OuiPortalInsert } from './portal';
43 changes: 37 additions & 6 deletions src/components/portal/portal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,44 @@ import { mount } from 'enzyme';
import { OuiPortal } from './portal';

describe('OuiPortal', () => {
test('is rendered', () => {
const component = mount(
<div>
<OuiPortal>Content</OuiPortal>
</div>
);
afterEach(() => {
document.body.innerHTML = '';
});

it('should render OuiPortal', () => {
const component = mount(<OuiPortal>Content</OuiPortal>);

expect(document.body.innerHTML).toEqual('<div>Content</div>');
expect(component).toMatchSnapshot();
});

it('should attach Content inside an element', () => {
const container = document.createElement('div');
container.setAttribute('id', 'container');
document.body.appendChild(container);
document.body.appendChild(document.createElement('div'));

mount(<OuiPortal insert={{ root: container }}>Content</OuiPortal>);

expect(document.body.innerHTML).toEqual(
'<div id="container">Content</div><div></div>'
);
});

it('should attach Content before an element', () => {
const container = document.createElement('div');
container.setAttribute('id', 'container');
document.body.appendChild(container);

mount(
<OuiPortal insert={{ sibling: container, position: 'before' }}>
Content
</OuiPortal>,
{ attachTo: document.body }
);

expect(document.body.innerHTML).toEqual(
'<div>Content</div><div id="container"></div>'
);
});
});
Loading