Skip to content

Commit

Permalink
LG-4446: refactor PopoverContext and replace PortalContext (#2456)
Browse files Browse the repository at this point in the history
* PopoverContext supports additional props and marks isPopoverOpen and setIsPopoverOpen deprecated

* Replace PortalContext with newly extended PopoverContext

* Changesets

* Add todo in select spec and update todos with jira tix
  • Loading branch information
stephl3 committed Sep 3, 2024
1 parent 3921387 commit 8aee4f8
Show file tree
Hide file tree
Showing 21 changed files with 386 additions and 174 deletions.
10 changes: 10 additions & 0 deletions .changeset/popular-steaks-grab.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@leafygreen-ui/code': patch
'@leafygreen-ui/copyable': patch
'@leafygreen-ui/modal': patch
'@leafygreen-ui/popover': patch
---

[LG-4446](https://jira.mongodb.org/browse/LG-4446): In order to consolidate popover-related contexts, the `PortalContext` values for `portalContainer` and `scrollContainer` are consolidated in the `PopoverContext`.

`usePopoverPortalContainer` is replaced by `usePopoverContext` and `PortalContextProvider` is replaced by `PopoverProvider`. There are no functional changes.
29 changes: 29 additions & 0 deletions .changeset/smooth-experts-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
'@leafygreen-ui/leafygreen-provider': major
---

[LG-4446](https://jira.mongodb.org/browse/LG-4446): In order to consolidate popover-related contexts, the `PortalContext` values for `portalContainer` and `scrollContainer` are consolidated in the `PopoverContext`.

`usePopoverPortalContainer` is replaced by `usePopoverContext` and `PortalContextProvider` is replaced by `PopoverProvider`.

`usePopoverPortalContainer` (used internally) and `PortalContextProvider` are no longer exported. See below migration guidance.

#### Migration guide

##### Old
```js
<PortalContextProvider
popover={{
portalContainer: yourPortalContainer,
scrollContainer: yourScrollContainer,
}}
>
```

##### New
```js
<PopoverProvider
portalContainer={yourPortalContainer}
scrollContainer={yourScrollContainer}
>
```
4 changes: 2 additions & 2 deletions packages/code/src/CopyButton/CopyButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import CopyIcon from '@leafygreen-ui/icon/dist/Copy';
import IconButton from '@leafygreen-ui/icon-button';
import {
useDarkMode,
usePopoverPortalContainer,
usePopoverContext,
} from '@leafygreen-ui/leafygreen-provider';
import { keyMap } from '@leafygreen-ui/lib';
import Tooltip, { Align, Justify } from '@leafygreen-ui/tooltip';
Expand All @@ -27,7 +27,7 @@ function CopyButton({ onCopy, contents }: CopyProps) {
const [open, setOpen] = useState(false);
const buttonRef = useRef<HTMLButtonElement>(null);
const { theme } = useDarkMode();
const { portalContainer } = usePopoverPortalContainer();
const { portalContainer } = usePopoverContext();

/**
* toggles `open` state of tooltip
Expand Down
4 changes: 2 additions & 2 deletions packages/copyable/src/Copyable/Copyable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useIdAllocator } from '@leafygreen-ui/hooks';
import CopyIcon from '@leafygreen-ui/icon/dist/Copy';
import {
useDarkMode,
usePopoverPortalContainer,
usePopoverContext,
} from '@leafygreen-ui/leafygreen-provider';
import { BaseFontSize } from '@leafygreen-ui/tokens';
import Tooltip, { Align, Justify, TriggerEvent } from '@leafygreen-ui/tooltip';
Expand Down Expand Up @@ -54,7 +54,7 @@ export default function Copyable({
const [buttonRef, setButtonRef] = useState<HTMLButtonElement>();
const codeRef = useRef<HTMLElement>(null);

const { portalContainer } = usePopoverPortalContainer();
const { portalContainer } = usePopoverContext();

const baseFontSize = useUpdatedBaseFontSize();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ export const MenuWrapper = forwardRef<HTMLDivElement, MenuWrapperProps>(
className={cx(menuStyles[theme], className)}
{...props}
>
{/* Prevents the opening and closing state of a select dropdown from propagating up to other PopoverProviders in parent components. E.g. Modal */}
{/*
* Prevents the opening and closing state of a select dropdown from propagating up
* to other PopoverProviders in parent components. E.g. Modal
*
* TODO: https://jira.mongodb.org/browse/LG-4474
*/}
<PopoverProvider>{children}</PopoverProvider>
</Popover>
);
Expand Down
3 changes: 2 additions & 1 deletion packages/leafygreen-provider/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
"access": "public"
},
"dependencies": {
"@leafygreen-ui/hooks": "^8.1.3",
"@leafygreen-ui/lib": "^13.3.0",
"@leafygreen-ui/hooks": "^8.1.3"
"react-transition-group": "^4.4.5"
},
"gitHead": "dd71a2d404218ccec2e657df9c0263dc1c15b9e0",
"homepage": "https://github.com/mongodb/leafygreen-ui/tree/main/packages/leafygreen-provider",
Expand Down
4 changes: 2 additions & 2 deletions packages/leafygreen-provider/src/LeafyGreenContext.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import { LeafyGreenProviderProps } from './LeafyGreenContext';
import LeafyGreenProvider, {
useBaseFontSize,
useDarkMode,
usePopoverPortalContainer,
usePopoverContext,
} from '.';

afterAll(cleanup);

const ContextChecker = () => {
const { darkMode } = useDarkMode();
const baseFontSize = useBaseFontSize();
const { portalContainer, scrollContainer } = usePopoverPortalContainer();
const { portalContainer, scrollContainer } = usePopoverContext();

return (
<>
Expand Down
64 changes: 42 additions & 22 deletions packages/leafygreen-provider/src/LeafyGreenContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,27 @@ import { DarkModeProps } from '@leafygreen-ui/lib';

import DarkModeProvider, { useDarkModeContext } from './DarkModeContext';
import { OverlayProvider } from './OverlayContext';
import PortalContextProvider, {
PortalContextValues,
usePopoverPortalContainer,
} from './PortalContext';
import {
PopoverContextType,
PopoverProvider,
usePopoverContext,
} from './PopoverContext';
import TypographyProvider, {
TypographyProviderProps,
useBaseFontSize,
} from './TypographyContext';
import UsingKeyboardProvider from './UsingKeyboardContext';

type PopoverPortalContainerType = Pick<
PopoverContextType,
'portalContainer' | 'scrollContainer'
>;

export type LeafyGreenProviderProps = {
/**
* Define a container HTMLElement for components that utilize the `Portal` component
*/
popoverPortalContainer?: PortalContextValues['popover'];
popoverPortalContainer?: PopoverPortalContainerType;
} & TypographyProviderProps &
DarkModeProps;

Expand All @@ -29,8 +35,9 @@ function LeafyGreenProvider({
popoverPortalContainer: popoverPortalContainerProp,
darkMode: darkModeProp,
}: PropsWithChildren<LeafyGreenProviderProps>) {
// if the prop is set, we use that
// if the prop is not set, we use outer context
/**
* If `darkMode` prop is provided, use that. Otherwise, use context value
*/
const { contextDarkMode: inheritedDarkMode } = useDarkModeContext();
const [darkModeState, setDarkMode] = useState(
darkModeProp ?? inheritedDarkMode,
Expand All @@ -40,26 +47,39 @@ function LeafyGreenProvider({
setDarkMode(darkModeProp ?? inheritedDarkMode);
}, [darkModeProp, inheritedDarkMode]);

// Similarly with base font size
/**
* If `baseFontSize` prop is provided, use that. Otherwise, use context value
*/
const inheritedFontSize = useBaseFontSize();
const baseFontSize = fontSizeProp ?? inheritedFontSize;
// and popover portal container
const inheritedContainer = usePopoverPortalContainer();
const popoverPortalContainer =
popoverPortalContainerProp ?? inheritedContainer;

/**
* If `popoverPortalContainer` prop is provided, use that. Otherwise, use context value
*/
const popoverContext = usePopoverContext();
const inheritedPopoverContextContainers: PopoverPortalContainerType =
Object.fromEntries(
Object.entries(popoverContext).filter(([key, _]) =>
['portalContainer', 'scrollContainer'].includes(key),
),
);
const popoverContextContainerValues =
popoverPortalContainerProp ?? inheritedPopoverContextContainers;

return (
<UsingKeyboardProvider>
<PortalContextProvider popover={popoverPortalContainer}>
<TypographyProvider baseFontSize={baseFontSize}>
<DarkModeProvider
contextDarkMode={darkModeState}
setDarkMode={setDarkMode}
>
<OverlayProvider>{children}</OverlayProvider>
</DarkModeProvider>
</TypographyProvider>
</PortalContextProvider>
<TypographyProvider baseFontSize={baseFontSize}>
<DarkModeProvider
contextDarkMode={darkModeState}
setDarkMode={setDarkMode}
>
<OverlayProvider>
<PopoverProvider {...popoverContextContainerValues}>
{children}
</PopoverProvider>
</OverlayProvider>
</DarkModeProvider>
</TypographyProvider>
</UsingKeyboardProvider>
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import React, { PropsWithChildren } from 'react';
import { act, fireEvent, render, waitFor } from '@testing-library/react';
import React from 'react';
import { act, render, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { renderHook } from '@leafygreen-ui/testing-lib';

import { PopoverProvider, type PopoverState, usePopoverContext } from '.';
import { PopoverProvider, usePopoverContext } from './PopoverContext';
import { type PopoverProviderProps } from './PopoverContext.types';

const childTestID = 'popover-provider';
const buttonTestId = 'test-button';
Expand All @@ -24,9 +26,9 @@ function TestContextComponent() {
);
}

function renderProvider() {
function renderProvider(props?: PopoverProviderProps) {
const utils = render(
<PopoverProvider>
<PopoverProvider {...props}>
<TestContextComponent />
</PopoverProvider>,
);
Expand All @@ -40,32 +42,29 @@ describe('packages/leafygreen-provider/PopoverContext', () => {
expect(container.firstChild).toBe(testChild);
});

test('isPopoverOpen is initialized as false', () => {
test('`isPopoverOpen` is initialized as false', () => {
const { testChild } = renderProvider();
expect(testChild.textContent).toBe('false');
});

test('when passed true, setIsPopoverOpen sets isPopoverOpen to true', () => {
test('when passed true, `setIsPopoverOpen` sets `isPopoverOpen` to true', () => {
const { testChild, getByTestId } = renderProvider();

// The button's click handler fires setIsPopoverOpen(true)
fireEvent.click(getByTestId(buttonTestId));
userEvent.click(getByTestId(buttonTestId));

expect(testChild.textContent).toBe('true');
});
});

describe('usePopoverContext', () => {
test('is `false` by default', () => {
test('`isPopoverOpen` is `false` by default', () => {
const { result } = renderHook(usePopoverContext);
expect(result.current.isPopoverOpen).toBeFalsy();
});

test('setter updates the value', async () => {
const { result, rerender } = renderHook<
PropsWithChildren<{}>,
PopoverState
>(usePopoverContext, {
test('`setIsPopoverOpen` updates the value of `isPopoverOpen`', async () => {
const { result, rerender } = renderHook(usePopoverContext, {
wrapper: ({ children }) => <PopoverProvider>{children}</PopoverProvider>,
});

Expand All @@ -76,6 +75,24 @@ describe('usePopoverContext', () => {
});
});

test('passes provider props correctly', () => {
const mockOnEnter = jest.fn();
const customProps = {
onEnter: mockOnEnter,
popoverZIndex: 2,
usePortal: true,
};
const { result } = renderHook(usePopoverContext, {
wrapper: ({ children }) => (
<PopoverProvider {...customProps}>{children}</PopoverProvider>
),
});

expect(result.current).toHaveProperty('onEnter', mockOnEnter);
expect(result.current).toHaveProperty('popoverZIndex', 2);
expect(result.current).toHaveProperty('usePortal', true);
});

describe('with test component', () => {
function renderTestComponent() {
const utils = render(<TestContextComponent />);
Expand All @@ -92,7 +109,7 @@ describe('usePopoverContext', () => {
const { testChild, getByTestId } = renderTestComponent();

// The button's click handler fires setIsPopoverOpen(true)
fireEvent.click(getByTestId(buttonTestId));
userEvent.click(getByTestId(buttonTestId));

expect(testChild.textContent).toBe('false');
});
Expand Down
48 changes: 23 additions & 25 deletions packages/leafygreen-provider/src/PopoverContext/PopoverContext.tsx
Original file line number Diff line number Diff line change
@@ -1,56 +1,54 @@
import React, { createContext, useContext, useMemo, useState } from 'react';
import React, {
createContext,
PropsWithChildren,
useContext,
useMemo,
useState,
} from 'react';
import PropTypes from 'prop-types';

export interface PopoverState {
/**
* Whether the most immediate popover ancestor is open
*/
isPopoverOpen: boolean;
/**
* Sets the internal state
* @internal
*/
setIsPopoverOpen: React.Dispatch<React.SetStateAction<boolean>>;
}

export const PopoverContext = createContext<PopoverState>({
import {
PopoverContextType,
PopoverProviderProps,
} from './PopoverContext.types';

export const PopoverContext = createContext<PopoverContextType>({
isPopoverOpen: false,
setIsPopoverOpen: () => {},
});

/**
* Access the popover state
* @returns `isPopoverOpen: boolean`
* Access the popover context
*/
export function usePopoverContext(): PopoverState {
export const usePopoverContext = (): PopoverContextType => {
return useContext(PopoverContext);
}

interface PopoverProviderProps {
children?: React.ReactNode;
}
};

/**
* Creates a Popover context.
* Call `usePopoverContext` to access the popover state
*/
export function PopoverProvider({ children }: PopoverProviderProps) {
export const PopoverProvider = ({
children,
...props
}: PropsWithChildren<PopoverProviderProps>) => {
const [isPopoverOpen, setIsPopoverOpen] = useState<boolean>(false);

const providerValue = useMemo(
() => ({
isPopoverOpen,
setIsPopoverOpen,
...props,
}),
[isPopoverOpen],
[isPopoverOpen, props],
);

return (
<PopoverContext.Provider value={providerValue}>
{children}
</PopoverContext.Provider>
);
}
};

PopoverProvider.displayName = 'PopoverProvider';

Expand Down
Loading

0 comments on commit 8aee4f8

Please sign in to comment.