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

chore(react-portal): refactor portals creation #33174

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@

```ts

import type { ComponentState } from '@fluentui/react-utilities';
import { elementContains } from '@fluentui/react-utilities';
import * as React_2 from 'react';
import { setVirtualParent } from '@fluentui/react-utilities';
import type { Slot } from '@fluentui/react-utilities';

export { elementContains }

Expand All @@ -23,7 +25,7 @@ export type PortalProps = {
};

// @public (undocumented)
export type PortalState = Pick<PortalProps, 'children'> & {
export type PortalState = ComponentState<PortalInternalSlots> & Pick<PortalProps, 'children'> & {
mountNode: HTMLElement | null | undefined;
virtualParentRootRef: React_2.MutableRefObject<HTMLSpanElement | null>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ import * as React from 'react';
import { Portal } from './Portal';
Copy link
Collaborator

@fabricteam fabricteam Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual regressions to review in the fluentuiv9 Visual Regression Report

Avatar Converged 2 screenshots
Image Name Diff(in Pixels) Image Type
Avatar Converged.badgeMask.chromium.png 5 Changed
Avatar Converged.Badge Mask RTL.chromium.png 2 Changed
Dialog 4 screenshots
Image Name Diff(in Pixels) Image Type
Dialog.nested.chromium.png 12768 Changed
Dialog.Nested RTL.chromium.png 12727 Changed
Dialog.Nested Dark Mode.chromium.png 4761 Changed
Dialog.Nested High Contrast.chromium.png 9003 Changed
Drawer 1 screenshots
Image Name Diff(in Pixels) Image Type
Drawer.Full Overlay Dark Mode.chromium.png 970 Changed


describe('Portal', () => {
/**
* Note: see more visual regression tests for Portal in /apps/vr-tests.
*/
it('renders a default state', () => {
const children = 'test';
const { getByText } = render(<Portal>{children}</Portal>);

expect(getByText(children)).toMatchSnapshot();
it('creates an element and attaches it to "document.body"', () => {
const { getByText } = render(<Portal>Test</Portal>);
const element = getByText('Test');

expect(document.body.children).toContain(element);
});

it('applies "dir" attribute based on a context value', () => {
Expand All @@ -33,13 +30,16 @@ describe('Portal', () => {
expect(getByText('RTL')).toHaveAttribute('dir', 'rtl');
});

it('applies "className"', () => {
const { getByText } = render(<Portal mountNode={{ className: 'foo' }}>Test</Portal>);

expect(getByText('Test')).toHaveClass('foo');
});

it('applies "zIndex" style', () => {
const { getByText } = render(<Portal>Test</Portal>);
const element = getByText('Test');

expect(element).toHaveStyle({
zIndex: 1000000,
});
expect(getByText('Test')).toHaveStyle({ zIndex: 1000000 });
});

it('should not set virtual parent if mount node contains virtual parent', () => {
Expand All @@ -59,4 +59,46 @@ describe('Portal', () => {
const mountNode = container.querySelector<HTMLSpanElement>('#container');
expect((getParent(mountNode) as HTMLElement).id).toBe('parent');
});

describe('mountNode', () => {
it('renders portal content into the specified mount node', () => {
const mountNode = document.createElement('div');

mountNode.id = 'mount-node';
document.body.appendChild(mountNode);

const { getByText } = render(
<Portal mountNode={mountNode}>
<span>Test</span>
</Portal>,
);
const portalEl = getByText('Test');

expect(portalEl).toBeInstanceOf(HTMLSpanElement);
expect(portalEl.parentElement).toBe(mountNode);
});

it('does not add attributes to a mount node', () => {
const mountNode = document.createElement('div');

mountNode.id = 'mount-node';
document.body.appendChild(mountNode);

render(
<Portal mountNode={mountNode}>
<span>Test</span>
</Portal>,
);

expect(mountNode).toMatchInlineSnapshot(`
<div
id="mount-node"
>
<span>
Test
</span>
</div>
`);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import type { ComponentState, Slot } from '@fluentui/react-utilities';
import * as React from 'react';

export type PortalInternalSlots = {
root?: Slot<'div'>;
};

export type PortalProps = {
/**
* React children
Expand All @@ -14,11 +19,12 @@ export type PortalProps = {
mountNode?: HTMLElement | null | { element?: HTMLElement | null; className?: string };
};

export type PortalState = Pick<PortalProps, 'children'> & {
mountNode: HTMLElement | null | undefined;
export type PortalState = ComponentState<PortalInternalSlots> &
Pick<PortalProps, 'children'> & {
mountNode: HTMLElement | null | undefined;

/**
* Ref to the root span element as virtual parent
*/
virtualParentRootRef: React.MutableRefObject<HTMLSpanElement | null>;
};
/**
* Ref to the root span element as virtual parent
*/
virtualParentRootRef: React.MutableRefObject<HTMLSpanElement | null>;
};

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
/** @jsxRuntime automatic */
/** @jsxImportSource @fluentui/react-jsx-runtime */
import { assertSlots } from '@fluentui/react-utilities';
import * as ReactDOM from 'react-dom';
import * as React from 'react';
import type { PortalState } from './Portal.types';

import type { PortalState, PortalInternalSlots } from './Portal.types';

/**
* Render the final JSX of Portal
*/
export const renderPortal_unstable = (state: PortalState): React.ReactElement => {
assertSlots<PortalInternalSlots>(state);

return (
<span hidden ref={state.virtualParentRootRef}>
{state.mountNode && ReactDOM.createPortal(state.children, state.mountNode)}
</span>
<>
<span hidden ref={state.virtualParentRootRef} />
{state.mountNode &&
ReactDOM.createPortal(state.root ? <state.root>{state.children}</state.root> : state.children, state.mountNode)}
</>
);
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import { setVirtualParent } from '@fluentui/react-utilities';
import { mergeClasses } from '@griffel/react';
import {
useFluent_unstable as useFluent,
useThemeClassName_unstable as useThemeClassName,
} from '@fluentui/react-shared-contexts';
import { useFocusVisible } from '@fluentui/react-tabster';
import { setVirtualParent, slot, useMergedRefs } from '@fluentui/react-utilities';
import * as React from 'react';

import { toMountNodeProps } from '../../utils/toMountNodeProps';
import { usePortalMountNode } from './usePortalMountNode';
import type { PortalProps, PortalState } from './Portal.types';
import { usePortalMountNodeStylesStyles } from './usePortalMountNodeStyles.styles';

/**
* Create the state required to render Portal.
Expand All @@ -15,17 +21,42 @@ import type { PortalProps, PortalState } from './Portal.types';
export const usePortal_unstable = (props: PortalProps): PortalState => {
const { element, className } = toMountNodeProps(props.mountNode);

const { dir, targetDocument } = useFluent();
const virtualParentRootRef = React.useRef<HTMLSpanElement>(null);
const fallbackElement = usePortalMountNode({ disabled: !!element, className });

const mountNode = element ?? fallbackElement;
const classes = usePortalMountNodeStylesStyles();
const themeClassName = useThemeClassName();

const ref = useMergedRefs(useFocusVisible<HTMLDivElement>());
const state: PortalState = {
components: {
root: 'div',
},

children: props.children,
mountNode,
root: slot.optional(
{
className: mergeClasses(themeClassName, classes.root, className),
dir,
ref,

'data-portal-node': true,
},
{ elementType: 'div' },
),

mountNode: targetDocument?.body,
virtualParentRootRef,
};

if (element) {
state.mountNode = element;
state.root = undefined;
}

React.useEffect(() => {
const mountNode = element ?? ref.current;

if (!mountNode) {
return;
}
Expand Down Expand Up @@ -80,7 +111,7 @@ export const usePortal_unstable = (props: PortalProps): PortalState => {
setVirtualParent(mountNode, undefined);
};
}
}, [virtualParentRootRef, mountNode]);
}, [virtualParentRootRef, element, ref]);

return state;
};

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,5 @@
exports[`TagPicker renders a default state 1`] = `
<div>
Default Picker
<span
hidden=""
/>
</div>
`;
Loading
Loading