Skip to content

Commit

Permalink
Fix portals not correctly inheriting color from its parent EuiTheme
Browse files Browse the repository at this point in the history
-  there's already a `data-euiportal` wrapper around all portals to bogart, so no need to create one

- in an attempt to reduce snapshot churn in Kibana's side of things, I've opted to only render a CSS class if the theme color is different from the `body` color

+ [tech debt] switch `portal.test.tsx` tests to RTL render
  • Loading branch information
cee-chen committed May 15, 2023
1 parent 43774c3 commit 3c71c2a
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 26 deletions.
41 changes: 25 additions & 16 deletions src/components/portal/__snapshots__/portal.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiPortal is rendered 1`] = `
<div>
<EuiPortal>
<Portal
containerInfo={
<div
data-euiportal="true"
>
Content
</div>
}
>
Content
</Portal>
</EuiPortal>
</div>
exports[`EuiPortal renders 1`] = `
<body>
<div />
<div
data-euiportal="true"
>
Content
</div>
</body>
`;

exports[`EuiPortal renders portals with a different theme provider from the global theme with the correct base color 1`] = `
<body>
<div>
<span
class="euiThemeProvider emotion-euiColorMode-inverse-colorClassName"
/>
</div>
<div
class="emotion-euiColorMode-inverse-colorClassName"
data-euiportal="true"
>
Content
</div>
</body>
`;
24 changes: 16 additions & 8 deletions src/components/portal/portal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,40 @@
*/

import React from 'react';
import { mount } from 'enzyme';
import { render } from '../../test/rtl';
import { EuiThemeProvider } from '../../services';

import { EuiPortal } from './portal';

describe('EuiPortal', () => {
it('is rendered', () => {
const component = mount(
<div>
it('renders', () => {
const { baseElement } = render(<EuiPortal>Content</EuiPortal>);

expect(baseElement).toMatchSnapshot();
});

it('renders portals with a different theme provider from the global theme with the correct base color', () => {
const { baseElement } = render(
<EuiThemeProvider colorMode="inverse">
<EuiPortal>Content</EuiPortal>
</div>
</EuiThemeProvider>
);

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

describe('behavior', () => {
it('portalRef', () => {
const portalRef = jest.fn();

const component = mount(
const { unmount } = render(
<EuiPortal portalRef={portalRef}>Content</EuiPortal>
);

expect(portalRef).toHaveBeenCalledTimes(1);
expect(portalRef.mock.calls[0][0]).toBeInstanceOf(HTMLDivElement);

component.unmount();
unmount();

expect(portalRef).toHaveBeenCalledTimes(2);
expect(portalRef.mock.calls[1][0]).toBeNull();
Expand Down
16 changes: 16 additions & 0 deletions src/components/portal/portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

import { Component, ReactNode } from 'react';
import { createPortal } from 'react-dom';

import { EuiNestedThemeContext } from '../../services';
import { keysOf } from '../common';

interface InsertPositionsMap {
Expand Down Expand Up @@ -41,6 +43,8 @@ export interface EuiPortalProps {
}

export class EuiPortal extends Component<EuiPortalProps> {
static contextType = EuiNestedThemeContext;

portalNode: HTMLDivElement | null = null;

constructor(props: EuiPortalProps) {
Expand All @@ -63,6 +67,7 @@ export class EuiPortal extends Component<EuiPortalProps> {
}

componentDidMount() {
this.setThemeColor();
this.updatePortalRef(this.portalNode);
}

Expand All @@ -73,6 +78,17 @@ export class EuiPortal extends Component<EuiPortalProps> {
this.updatePortalRef(null);
}

// Set the inherited color of the portal based on the wrapping EuiThemeProvider
setThemeColor() {
if (this.portalNode && this.context) {
const { hasDifferentColorFromGlobalTheme, colorClassName } = this.context;

if (hasDifferentColorFromGlobalTheme && this.props.insert == null) {
this.portalNode.classList.add(colorClassName);
}
}
}

updatePortalRef(ref: HTMLDivElement | null) {
if (this.props.portalRef) {
this.props.portalRef(ref);
Expand Down
2 changes: 2 additions & 0 deletions src/services/theme/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ export const EuiThemeContext = createContext<EuiThemeComputed>(
);
export const EuiNestedThemeContext = createContext<EuiThemeNested>({
isGlobalTheme: true,
hasDifferentColorFromGlobalTheme: false,
bodyColor: '',
colorClassName: '',
});
8 changes: 6 additions & 2 deletions src/services/theme/provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const EuiThemeProvider = <T extends {} = {}>({
children,
wrapperProps,
}: PropsWithChildren<EuiThemeProviderProps<T>>) => {
const { isGlobalTheme } = useContext(EuiNestedThemeContext);
const { isGlobalTheme, bodyColor } = useContext(EuiNestedThemeContext);
const parentSystem = useContext(EuiSystemContext);
const parentModifications = useContext(EuiModificationsContext);
const parentColorMode = useContext(EuiColorModeContext);
Expand Down Expand Up @@ -145,12 +145,16 @@ export const EuiThemeProvider = <T extends {} = {}>({
const nestedThemeContext = useMemo(() => {
return {
isGlobalTheme: false, // The theme that determines the global body styles
bodyColor: isGlobalTheme ? theme.colors.text : bodyColor,
hasDifferentColorFromGlobalTheme: isGlobalTheme
? false
: bodyColor !== theme.colors.text,
colorClassName: css`
label: euiColorMode-${_colorMode};
color: ${theme.colors.text};
`,
};
}, [theme, _colorMode]);
}, [theme, isGlobalTheme, bodyColor, _colorMode]);

const renderedChildren = useMemo(() => {
if (isGlobalTheme) {
Expand Down
2 changes: 2 additions & 0 deletions src/services/theme/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,7 @@ export type EuiThemeComputed<T = {}> = ComputedThemeShape<EuiThemeShape & T> & {

export type EuiThemeNested = {
isGlobalTheme: boolean;
hasDifferentColorFromGlobalTheme: boolean;
bodyColor: string;
colorClassName: string;
};

0 comments on commit 3c71c2a

Please sign in to comment.