Repository: equinor/design-system
Pull request #3600 (2024-08-28 11:17:04)

Bugfix/deselect multiple does not work with complex items
Blind attempt at fixing edge case of #3259


Patch:
diff --git a/packages/eds-core-react/src/components/Autocomplete/Autocomplete.tsx b/packages/eds-core-react/src/components/Autocomplete/Autocomplete.tsx
index 78865a4a..03d8954d 100644
--- a/packages/eds-core-react/src/components/Autocomplete/Autocomplete.tsx
+++ b/packages/eds-core-react/src/components/Autocomplete/Autocomplete.tsx
@@ -610,9 +610,14 @@ function AutocompleteInner<T>(
             if (selectedItem === AllSymbol) {
               toggleAllSelected()
             } else if (multiple) {
-              selectedItems.includes(selectedItem)
-                ? removeSelectedItem(selectedItem)
-                : addSelectedItem(selectedItem)
+              const shouldRemove = itemCompare
+                ? selectedItems.some((i) => itemCompare(selectedItem, i))
+                : selectedItems.includes(selectedItem)
+              if (shouldRemove) {
+                removeSelectedItem(selectedItem)
+              } else {
+                addSelectedItem(selectedItem)
+              }
             } else {
               setSelectedItems([selectedItem])
             }
diff --git a/packages/eds-core-react/src/components/Popover/Popover.stories.tsx b/packages/eds-core-react/src/components/Popover/Popover.stories.tsx
index 4da13239..3041cad6 100644
--- a/packages/eds-core-react/src/components/Popover/Popover.stories.tsx
+++ b/packages/eds-core-react/src/components/Popover/Popover.stories.tsx
@@ -148,7 +148,6 @@ export const ActivateOnClick: StoryFn<PopoverProps> = () => {
         onClose={closePopover}
         open={isOpen}
         placement="top"
-        withinPortal
       >
         <Popover.Header>
           <Popover.Title>Title</Popover.Title>
@@ -199,7 +198,6 @@ export const ActivateOnHover: StoryFn<PopoverProps> = () => {
         onClose={handleClose}
         open={isOpen}
         placement="top"
-        withinPortal
       >
         <Popover.Header>
           <Popover.Title>Title</Popover.Title>
@@ -237,7 +235,6 @@ export const WithCloseButton: StoryFn<PopoverProps> = () => {
         open={isOpen}
         placement="top"
         trapFocus
-        withinPortal
       >
         <Popover.Header>
           <Popover.Title>Title</Popover.Title>
@@ -297,7 +294,6 @@ export const PersistentPopover: StoryFn<PopoverProps> = () => {
         open={isOpen}
         placement="top"
         trapFocus
-        withinPortal
       >
         <Popover.Header>
           <Popover.Title>With Autocomplete</Popover.Title>
@@ -345,7 +341,6 @@ PersistentPopover.storyName = 'Persistent popover'
         onClose={closePopover}
         open={isOpen}
         placement="top"
-        withinPortal
       >
         <Popover.Header>
           <Popover.Title>Title</Popover.Title>
@@ -404,7 +399,6 @@ export const AppLauncher: StoryFn<PopoverProps> = () => {
         onClose={closePopover}
         placement="top"
         trapFocus
-        withinPortal
       >
         <Popover.Content>
           <Wrapper>
diff --git a/packages/eds-core-react/src/components/Popover/Popover.tsx b/packages/eds-core-react/src/components/Popover/Popover.tsx
index bf9e2b6f..080dfb7f 100644
--- a/packages/eds-core-react/src/components/Popover/Popover.tsx
+++ b/packages/eds-core-react/src/components/Popover/Popover.tsx
@@ -14,7 +14,7 @@ import {
   mergeRefs,
   useToken,
   outlineTemplate,
-  useIsInDialog,
+  useIsomorphicLayoutEffect,
 } from '@equinor/eds-utils'
 import { popover as popoverToken } from './Popover.tokens'
 import { useEds } from '../EdsProvider'
@@ -28,7 +28,6 @@ import {
   useFloating,
   useInteractions,
   useDismiss,
-  FloatingPortal,
   FloatingFocusManager,
 } from '@floating-ui/react'
 
@@ -38,22 +37,35 @@ const PopoverPaper = styled(Paper)(({ theme }) => {
   } = theme
 
   return css`
+    position: relative;
     ${typographyTemplate(theme.typography)}
     background: ${theme.background};
     ${bordersTemplate(theme.border)}
-    z-index: 1400;
     &:focus-visible {
       ${outlineTemplate(paper.states.focus.outline)}
     }
   `
 })
 
+const StyledPopover = styled('div').withConfig({
+  shouldForwardProp: () => true, //workaround to avoid warning until popover gets added to react types
+})<{ popover: string }>`
+  inset: unset;
+  border: 0;
+  padding: 0;
+  margin: 0;
+  overflow: visible;
+  background-color: transparent;
+  &::backdrop {
+    background-color: transparent;
+  }
+`
+
 const ArrowWrapper = styled.div(({ theme }) => {
   return css`
     position: absolute;
     width: ${theme.entities.arrow.width};
     height: ${theme.entities.arrow.height};
-    z-index: -1;
   `
 })
 
@@ -91,7 +103,9 @@ export type PopoverProps = {
   anchorEl?: HTMLElement | null
   /** Is Popover open */
   open: boolean
-  /** initializes react portal for dropdown, default to false. */
+  /**
+   * @deprecated  Popover now uses the native popover api instead of react portal. This prop will be removed in a future version
+   */
   withinPortal?: boolean
   /** Determines whether focus should be trapped within dropdown,
    * default to false. */
@@ -113,6 +127,11 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
     },
     ref,
   ) {
+    if (withinPortal) {
+      console.warn(
+        'Popover "withinPortal" prop has been deprecated. Popover now uses the native popover api',
+      )
+    }
     const arrowRef = useRef<HTMLDivElement>(null)
 
     const {
@@ -146,6 +165,14 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
 
     const { getFloatingProps } = useInteractions([useDismiss(context)])
 
+    useIsomorphicLayoutEffect(() => {
+      if (open) {
+        refs.floating.current?.showPopover()
+      } else {
+        refs.floating.current?.hidePopover()
+      }
+    }, [open, refs.floating])
+
     useEffect(() => {
       if (arrowRef.current) {
         const staticSide = {
@@ -183,21 +210,16 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
     }, [arrowRef.current, arrowX, arrowY, finalPlacement])
 
     const props = {
-      open,
       ...rest,
     }
 
     const { density } = useEds()
     const token = useToken({ density }, popoverToken)
 
-    //temporary fix when inside dialog. Should be replaced by popover api when it is ready
-    const inDialog = useIsInDialog(anchorEl)
-
     const popover = (
       <ThemeProvider theme={token}>
-        <PopoverPaper
-          elevation="overlay"
-          {...props}
+        <StyledPopover
+          popover="manual"
           {...getFloatingProps({
             ref: popoverRef,
             style: {
@@ -208,39 +230,27 @@ export const Popover = forwardRef<HTMLDivElement, PopoverProps>(
             },
           })}
         >
-          <ArrowWrapper ref={arrowRef} className="arrow">
-            <PopoverArrow className="arrowSvg">
-              <path d="M0.504838 4.86885C-0.168399 4.48524 -0.168399 3.51476 0.504838 3.13115L6 8.59227e-08L6 8L0.504838 4.86885Z" />
-            </PopoverArrow>
-          </ArrowWrapper>
-          <InnerWrapper>{children}</InnerWrapper>
-        </PopoverPaper>
+          <PopoverPaper elevation="overlay" {...props}>
+            <ArrowWrapper ref={arrowRef} className="arrow">
+              <PopoverArrow className="arrowSvg">
+                <path d="M0.504838 4.86885C-0.168399 4.48524 -0.168399 3.51476 0.504838 3.13115L6 8.59227e-08L6 8L0.504838 4.86885Z" />
+              </PopoverArrow>
+            </ArrowWrapper>
+            <InnerWrapper>{children}</InnerWrapper>
+          </PopoverPaper>
+        </StyledPopover>
       </ThemeProvider>
     )
 
     return (
       <>
-        {withinPortal && !inDialog ? (
-          <FloatingPortal id="eds-popover-container">
-            {open && trapFocus
-              ? open && (
-                  <FloatingFocusManager context={context} modal={true}>
-                    {popover}
-                  </FloatingFocusManager>
-                )
-              : open && popover}
-          </FloatingPortal>
-        ) : (
-          <>
-            {trapFocus
-              ? open && (
-                  <FloatingFocusManager context={context} modal={true}>
-                    {popover}
-                  </FloatingFocusManager>
-                )
-              : open && popover}
-          </>
-        )}
+        {trapFocus
+          ? open && (
+              <FloatingFocusManager context={context} modal={true}>
+                {popover}
+              </FloatingFocusManager>
+            )
+          : open && popover}
       </>
     )
   },


Test patch:
diff --git a/packages/eds-core-react/src/components/Autocomplete/Autocomplete.test.tsx b/packages/eds-core-react/src/components/Autocomplete/Autocomplete.test.tsx
index 442b6aaf..04b95473 100644
--- a/packages/eds-core-react/src/components/Autocomplete/Autocomplete.test.tsx
+++ b/packages/eds-core-react/src/components/Autocomplete/Autocomplete.test.tsx
@@ -209,6 +209,47 @@ describe('Autocomplete', () => {
     })
   })
 
+  it('Can deselect complex options', async () => {
+    const onChange = jest.fn()
+    const opts = [
+      { label: 'Its', value: 'relationship' },
+      { label: 'Complicated', value: 'status' },
+    ]
+    render(
+      <Autocomplete
+        optionLabel={(o) => o.label}
+        itemCompare={(o1, o2) => o1.value === o2.value}
+        label={labelText}
+        options={opts}
+        data-testid="styled-autocomplete"
+        multiple={true}
+        onOptionsChange={onChange}
+        selectedOptions={[
+          {
+            label: 'Its',
+            value: 'relationship',
+          },
+        ]}
+      />,
+    )
+
+    const labeledNodes = await screen.findAllByLabelText(labelText)
+    const optionsList = labeledNodes[1]
+
+    const buttonNode = await screen.findByLabelText('toggle options', {
+      selector: 'button',
+    })
+
+    fireEvent.click(buttonNode)
+
+    const options = await within(optionsList).findAllByRole('option')
+    fireEvent.click(options[0])
+
+    await waitFor(() => {
+      expect(onChange).toHaveBeenCalledWith({ selectedItems: [] })
+    })
+  })
+
   it('Can open the options on button click', async () => {
     render(<Autocomplete options={items} label={labelText} />)
 
diff --git a/packages/eds-core-react/src/components/Popover/Popover.test.tsx b/packages/eds-core-react/src/components/Popover/Popover.test.tsx
index 48847abe..19d6a5ce 100644
--- a/packages/eds-core-react/src/components/Popover/Popover.test.tsx
+++ b/packages/eds-core-react/src/components/Popover/Popover.test.tsx
@@ -62,19 +62,15 @@ describe('Popover', () => {
 
     await waitFor(() => expect(container).toHaveStyleRule('background', 'red'))
   })
-  it('is visible when open is true & anchorEl is set', async () => {
+  it('is visible when open is true & anchorEl is set', () => {
     render(
       <TestPopover open placement="right-start" data-testid="popover">
         <div>some random content</div>
       </TestPopover>,
     )
-    const container = screen.getByTestId('popover')
-    await waitFor(() => {
-      expect(container).toBeDefined()
-    })
-    await waitFor(() => {
-      expect(container).toHaveAttribute('open')
-    })
+    const container = screen.queryByTestId('popover')
+    expect(container).toBeDefined()
+    expect(container).toBeVisible()
   })
 
   it('has rendered Popover Title', async () => {
diff --git a/packages/eds-core-react/src/components/Popover/__snapshots__/Popover.test.tsx.snap b/packages/eds-core-react/src/components/Popover/__snapshots__/Popover.test.tsx.snap
index 01d66032..ffb83dd9 100644
--- a/packages/eds-core-react/src/components/Popover/__snapshots__/Popover.test.tsx.snap
+++ b/packages/eds-core-react/src/components/Popover/__snapshots__/Popover.test.tsx.snap
@@ -7,6 +7,7 @@ exports[`Popover Matches snapshot 1`] = `
 }
 
 .c1 {
+  position: relative;
   margin: 0;
   color: var(--eds_ui__accordion_header_color, rgba(61, 61, 61, 1));
   font-family: Equinor;
@@ -16,7 +17,6 @@ exports[`Popover Matches snapshot 1`] = `
   text-align: left;
   background: var(--eds_ui_background__default, rgba(255, 255, 255, 1));
   border-radius: 4px;
-  z-index: 1400;
 }
 
 .c1:focus-visible {
@@ -28,7 +28,6 @@ exports[`Popover Matches snapshot 1`] = `
   position: absolute;
   width: 6px;
   height: 8px;
-  z-index: -1;
 }
 
 .c5 {
@@ -55,9 +54,6 @@ exports[`Popover Matches snapshot 1`] = `
 <div
   class="c0 c1 c2"
   data-testid="popover"
-  open=""
-  style="position: absolute; top: 14px; left: 8px;"
-  tabindex="-1"
 >
   <div
     class="c3 arrow"