fix: dropdown menu portals (#2740)

2023-01-26 00:52:49 -08:00 · 2023-01-26 00:52:49 -08:00 · 3893b26d30
commit 3893b26d30
parent 1ba0ad234b
8 changed files with 144 additions and 59 deletions
--- a/apps/trading/client-pages/portfolio/account-history-container.tsx
+++ b/apps/trading/client-pages/portfolio/account-history-container.tsx
@ -139,16 +139,19 @@ const AccountHistoryManager = ({
    <div className="h-full w-full flex flex-col gap-8">
      <div className="w-full flex flex-col-reverse lg:flex-row items-start lg:items-center justify-between gap-4 px-2">
        <div className="flex items-center gap-4 shrink-0">
-          <DropdownMenu>
-            <DropdownMenuTrigger>
-              {accountType
-                ? `${
-                    AccountTypeMapping[
-                      accountType as keyof typeof Schema.AccountType
-                    ]
-                  } Account`
-                : t('Select account type')}
-            </DropdownMenuTrigger>
+          <DropdownMenu
+            trigger={
+              <DropdownMenuTrigger>
+                {accountType
+                  ? `${
+                      AccountTypeMapping[
+                        accountType as keyof typeof Schema.AccountType
+                      ]
+                    } Account`
+                  : t('Select account type')}
+              </DropdownMenuTrigger>
+            }
+          >
            <DropdownMenuContent>
              {[
                Schema.AccountType.ACCOUNT_TYPE_GENERAL,
@ -164,10 +167,13 @@ const AccountHistoryManager = ({
              ))}
            </DropdownMenuContent>
          </DropdownMenu>
-          <DropdownMenu>
-            <DropdownMenuTrigger>
-              {asset ? asset.symbol : t('Select asset')}
-            </DropdownMenuTrigger>
+          <DropdownMenu
+            trigger={
+              <DropdownMenuTrigger>
+                {asset ? asset.symbol : t('Select asset')}
+              </DropdownMenuTrigger>
+            }
+          >
            <DropdownMenuContent>
              {assets.map((a) => (
                <DropdownMenuItem key={a.id} onClick={() => setAsset(a)}>
--- a/apps/trading/components/vega-wallet-connect-button/vega-wallet-connect-button.spec.tsx
+++ b/apps/trading/components/vega-wallet-connect-button/vega-wallet-connect-button.spec.tsx
@ -3,6 +3,7 @@ import { VegaWalletContext } from '@vegaprotocol/wallet';
 import type { VegaWalletContextShape } from '@vegaprotocol/wallet';
 import { VegaWalletConnectButton } from './vega-wallet-connect-button';
 import { truncateByChars } from '@vegaprotocol/react-helpers';
+import userEvent from '@testing-library/user-event';

 const mockUpdateDialogOpen = jest.fn();
 jest.mock('@vegaprotocol/wallet', () => ({
@ -31,7 +32,7 @@ it('Not connected', () => {
  expect(mockUpdateDialogOpen).toHaveBeenCalled();
 });

-it('Connected', () => {
+it('Connected', async () => {
  const pubKey = { publicKey: '123456__123456', name: 'test' };
  render(
    generateJsx({
@ -42,6 +43,6 @@ it('Connected', () => {

  const button = screen.getByTestId('manage-vega-wallet');
  expect(button).toHaveTextContent(truncateByChars(pubKey.publicKey));
-  fireEvent.click(button);
+  userEvent.click(button);
  expect(mockUpdateDialogOpen).not.toHaveBeenCalled();
 });
--- a/apps/trading/components/vega-wallet-connect-button/vega-wallet-connect-button.tsx
+++ b/apps/trading/components/vega-wallet-connect-button/vega-wallet-connect-button.tsx
@ -142,15 +142,21 @@ export const VegaWalletConnectButton = () => {
    return (
      <>
        <div className="hidden lg:block">
-          <DropdownMenu open={dropdownOpen}>
-            <DropdownMenuTrigger
-              data-testid="manage-vega-wallet"
-              onClick={() => setDropdownOpen((curr) => !curr)}
-            >
-              {activeKey && <span className="uppercase">{activeKey.name}</span>}
-              {': '}
-              {truncateByChars(pubKey)}
-            </DropdownMenuTrigger>
+          <DropdownMenu
+            open={dropdownOpen}
+            trigger={
+              <DropdownMenuTrigger
+                data-testid="manage-vega-wallet"
+                onClick={() => setDropdownOpen((curr) => !curr)}
+              >
+                {activeKey && (
+                  <span className="uppercase">{activeKey.name}</span>
+                )}
+                {': '}
+                {truncateByChars(pubKey)}
+              </DropdownMenuTrigger>
+            }
+          >
            <DropdownMenuContent
              onInteractOutside={() => setDropdownOpen(false)}
            >
--- a/libs/candles-chart/src/lib/candles-chart.tsx
+++ b/libs/candles-chart/src/lib/candles-chart.tsx
@ -60,10 +60,13 @@ export const CandlesChartContainer = ({
  return (
    <div className="h-full flex flex-col">
      <div className="px-4 py-2 flex flex-row flex-wrap gap-4">
-        <DropdownMenu>
-          <DropdownMenuTrigger>
-            {t(`Interval: ${intervalLabels[interval]}`)}
-          </DropdownMenuTrigger>
+        <DropdownMenu
+          trigger={
+            <DropdownMenuTrigger>
+              {t(`Interval: ${intervalLabels[interval]}`)}
+            </DropdownMenuTrigger>
+          }
+        >
          <DropdownMenuContent>
            <DropdownMenuRadioGroup
              value={interval}
@ -84,10 +87,13 @@ export const CandlesChartContainer = ({
            </DropdownMenuRadioGroup>
          </DropdownMenuContent>
        </DropdownMenu>
-        <DropdownMenu>
-          <DropdownMenuTrigger>
-            <Icon name={chartTypeIcon.get(chartType) as IconName} />
-          </DropdownMenuTrigger>
+        <DropdownMenu
+          trigger={
+            <DropdownMenuTrigger>
+              <Icon name={chartTypeIcon.get(chartType) as IconName} />
+            </DropdownMenuTrigger>
+          }
+        >
          <DropdownMenuContent>
            <DropdownMenuRadioGroup
              value={chartType}
@ -104,8 +110,9 @@ export const CandlesChartContainer = ({
            </DropdownMenuRadioGroup>
          </DropdownMenuContent>
        </DropdownMenu>
-        <DropdownMenu>
-          <DropdownMenuTrigger>{t('Overlays')}</DropdownMenuTrigger>
+        <DropdownMenu
+          trigger={<DropdownMenuTrigger>{t('Overlays')}</DropdownMenuTrigger>}
+        >
          <DropdownMenuContent>
            {Object.values(Overlay).map((overlay) => (
              <DropdownMenuCheckboxItem
@ -128,8 +135,9 @@ export const CandlesChartContainer = ({
            ))}
          </DropdownMenuContent>
        </DropdownMenu>
-        <DropdownMenu>
-          <DropdownMenuTrigger>{t('Studies')}</DropdownMenuTrigger>
+        <DropdownMenu
+          trigger={<DropdownMenuTrigger>{t('Studies')}</DropdownMenuTrigger>}
+        >
          <DropdownMenuContent>
            {Object.values(Study).map((study) => (
              <DropdownMenuCheckboxItem
--- a/libs/environment/src/components/network-switcher/network-switcher.tsx
+++ b/libs/environment/src/components/network-switcher/network-switcher.tsx
@ -96,13 +96,18 @@ export const NetworkSwitcher = () => {
  const menuRef = useRef<HTMLButtonElement | null>(null);

  return (
-    <DropdownMenu open={isOpen} onOpenChange={handleOpen}>
-      <DropdownMenuTrigger
-        ref={menuRef}
-        className="flex justify-between items-center"
-      >
-        {envTriggerMapping[VEGA_ENV]}
-      </DropdownMenuTrigger>
+    <DropdownMenu
+      open={isOpen}
+      onOpenChange={handleOpen}
+      trigger={
+        <DropdownMenuTrigger
+          ref={menuRef}
+          className="flex justify-between items-center"
+        >
+          {envTriggerMapping[VEGA_ENV]}
+        </DropdownMenuTrigger>
+      }
+    >
      <DropdownMenuContent
        align="start"
        style={{ minWidth: `${menuRef.current?.offsetWidth || 290}px` }}
--- a/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.spec.tsx
+++ b/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.spec.tsx
@ -0,0 +1,34 @@
+import {
+  DropdownMenu,
+  DropdownMenuContent,
+  DropdownMenuTrigger,
+} from './dropdown-menu';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+describe('DropdownMenu', () => {
+  const text = 'Dropdown menu content';
+
+  // Upgrade from @radix-ui/react-dropdown-menu 0.1.6 to 2.0.2 renders
+  // dropdowns inline (rather than portals). Currently not using a portal
+  // will break the UI due to z-index issues
+  it('renders using a portal', async () => {
+    render(
+      <div className="test-wrapper">
+        <DropdownMenu
+          trigger={<DropdownMenuTrigger>Trigger</DropdownMenuTrigger>}
+        >
+          <DropdownMenuContent>
+            <p>{text}</p>
+          </DropdownMenuContent>
+        </DropdownMenu>
+      </div>
+    );
+
+    userEvent.click(screen.getByText(/trigger/i));
+    const contentElement = await screen.findByText(text);
+    expect(contentElement).toBeInTheDocument();
+    // if content is within .test-wrapper then its not been rendered in a portal
+    expect(contentElement.closest('.test-wrapper')).toBe(null);
+  });
+});
--- a/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.stories.tsx
+++ b/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.stories.tsx
@ -29,16 +29,21 @@ export const CheckboxItems = () => {
  console.log(checkboxItems);

  return (
-    <DropdownMenu>
-      <DropdownMenuTrigger>
-        <span>Select many things</span>
-      </DropdownMenuTrigger>
+    <DropdownMenu
+      trigger={
+        <DropdownMenuTrigger>
+          <span>Select many things</span>
+        </DropdownMenuTrigger>
+      }
+    >
      <DropdownMenuContent>
        {checkboxItems.map(({ label, state: [checked, setChecked] }) => (
          <DropdownMenuCheckboxItem
            key={label}
            checked={checked}
-            onCheckedChange={setChecked}
+            onCheckedChange={(checked) =>
+              setChecked(typeof checked === 'boolean' ? checked : false)
+            }
          >
            {label}
            <DropdownMenuItemIndicator />
@ -55,10 +60,13 @@ export const RadioItems = () => {

  return (
    <div style={{ textAlign: 'center', padding: 50 }}>
-      <DropdownMenu>
-        <DropdownMenuTrigger>
-          <span>Open</span>
-        </DropdownMenuTrigger>
+      <DropdownMenu
+        trigger={
+          <DropdownMenuTrigger>
+            <span>Open</span>
+          </DropdownMenuTrigger>
+        }
+      >
        <DropdownMenuContent>
          <DropdownMenuItem onSelect={() => console.log('minimize')}>
            Minimize window
@ -92,10 +100,13 @@ export const IconMenu = () => {

  return (
    <div style={{ textAlign: 'center', padding: 50 }}>
-      <DropdownMenu>
-        <DropdownMenuTrigger>
-          <Icon name="cog" />
-        </DropdownMenuTrigger>
+      <DropdownMenu
+        trigger={
+          <DropdownMenuTrigger>
+            <Icon name="cog" />
+          </DropdownMenuTrigger>
+        }
+      >
        <DropdownMenuContent>
          {iconMenuItems.map(({ label }) => (
            <DropdownMenuItem key={label}>{label}</DropdownMenuItem>
--- a/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.tsx
+++ b/libs/ui-toolkit/src/components/dropdown-menu/dropdown-menu.tsx
@ -1,5 +1,6 @@
 import * as DropdownMenuPrimitive from '@radix-ui/react-dropdown-menu';
 import classNames from 'classnames';
+import type { ReactNode } from 'react';
 import { forwardRef } from 'react';
 import { Icon } from '../icon';

@ -12,11 +13,24 @@ const itemClass = classNames(
  'whitespace-nowrap'
 );

+type DropdownMenuProps = DropdownMenuPrimitive.DropdownMenuProps & {
+  trigger: ReactNode;
+};
 /**
 * Contains all the parts of a dropdown menu.
 */
-export const DropdownMenu = DropdownMenuPrimitive.Root;
-
+export const DropdownMenu = ({
+  children,
+  trigger,
+  ...props
+}: DropdownMenuProps) => {
+  return (
+    <DropdownMenuPrimitive.Root {...props}>
+      {trigger}
+      <DropdownMenuPrimitive.Portal>{children}</DropdownMenuPrimitive.Portal>
+    </DropdownMenuPrimitive.Root>
+  );
+};
 /**
 * The button that toggles the dropdown menu.
 * By default, the {@link DropdownMenuContent} will position itself against the trigger.