cypress/integration/Popup/Popup.visual.js

@@ -8,6 +8,7 @@ describe('Popup: visual', () => {
     cy.get('[data-tid="button-popup"]').click()
     cy.get('[data-tid="popup-content"]').should('be.visible')
 
+    // This screenshot contains invalid position of an unknown reason
     cy.percySnapshot('Popup: inside a Modal')
   })
 
@@ -19,4 +20,22 @@ describe('Popup: visual', () => {
 
     cy.percySnapshot('Popup: floating Button')
   })
+
+  it('flowing', () => {
+    cy.visit('/maximize/popup-example-flowing')
+
+    cy.get('.ui.button').click()
+    cy.get('.ui.popup').should('be.visible')
+
+    cy.percySnapshot('Popup: flowing')
+  })
+
+  it('positionFixed', () => {
+    cy.visit('/maximize/popup-example-position-fixed')
+
+    cy.get('.ui.button').click()
+    cy.get('.ui.popup').should('be.visible')
+
+    cy.percySnapshot('Popup: positionFixed')
+  })
 })

docs/src/examples/modules/Popup/Usage/PopupExamplePopper.js

@@ -0,0 +1,18 @@
+import React from 'react'
+import { Button, Popup } from 'semantic-ui-react'
+
+const PopupExamplePopper = () => (
+  <Popup
+    content={
+      <>
+        A wrapping element in this Popup will have custom <code>id</code> &{' '}
+        <code>zIndex</code>.
+      </>
+    }
+    on='click'
+    popper={{ id: 'popper-container', style: { zIndex: 2000 } }}
+    trigger={<Button>Open a popup</Button>}
+  />
+)
+
+export default PopupExamplePopper

docs/src/examples/modules/Popup/Usage/PopupExamplePositionFixed.js

@@ -0,0 +1,17 @@
+import React from 'react'
+import { Button, Popup } from 'semantic-ui-react'
+
+const PopupExamplePositionFixed = () => (
+  <Popup
+    content={
+      <>
+        This Popup is positioned with <code>position: fixed</code>
+      </>
+    }
+    on='click'
+    positionFixed
+    trigger={<Button>Open a popup</Button>}
+  />
+)
+
+export default PopupExamplePositionFixed

docs/src/examples/modules/Popup/Usage/index.js

@@ -94,6 +94,40 @@ const PopupUsageExamples = () => (
       examplePath='modules/Popup/Usage/PopupExamplePopperDependencies'
       renderHtml={false}
     />
+    <ComponentExample
+      title={
+        <>
+          <code>popper</code> element
+        </>
+      }
+      description={
+        <>
+          From <code>semantic-ui-react@2.0.0</code> we are using an additional
+          wrapping element around <code>Popup</code> for positioning, see{' '}
+          <a href='https://github.com/Semantic-Org/Semantic-UI-React/pull/3947'>
+            Semantic-Org/Semantic-UI-React#3947
+          </a>{' '}
+          for more details. To pass props to this element <code>popper</code>{' '}
+          shorthand can be used.
+        </>
+      }
+      examplePath='modules/Popup/Usage/PopupExamplePopper'
+    />
+    <ComponentExample
+      title={
+        <>
+          Positioning via <code>position: fixed</code>
+        </>
+      }
+      description={
+        <>
+          If your reference element is in a <code>fixed</code> container, use{' '}
+          <code>positionFixed</code>. This will prevent any jumpiness since no
+          repositioning is needed.
+        </>
+      }
+      examplePath='modules/Popup/Usage/PopupExamplePositionFixed'
+    />
     <ComponentExample
       title='Actions'
       description='A popup can be triggered on hover, click, focus or multiple actions.'

docs/src/pages/MigrationGuide.mdx

@@ -32,6 +32,12 @@ The `popperModifiers` prop should be defined as an array with changed format (se
 +<Popup popperModifiers={[{ name: 'preventOverflow', options: { padding: 0 } }]} />
 ```
 
+### a wrapping element around `Popup`
+
+We started to use an additional wrapping element around `Popup` for positioning, see [Semantic-Org/Semantic-UI-React#3947](https://github.com/Semantic-Org/Semantic-UI-React/pull/3947) for more details. To pass props to this element `popper` shorthand can be used, see [an example](/modules/popup/#usage-position-fixed).
+
+⚠️We also made a fix in [Semantic-Org/Semantic-UI-React#4094](https://github.com/Semantic-Org/Semantic-UI-React/pull/4094) to transfer `zIndex` value to avoid any breaks.
+
 ## `Responsive` component was removed
 
 `Responsive` component was deprecated in 1.1.0. There are two main reasons for the removal: there is no connection between breakpoints and pure SSR (server side rendering) support.

src/modules/Popup/Popup.d.ts

@@ -119,6 +119,9 @@ export interface StrictPopupProps extends StrictPortalProps {
   /** Tells `Popper.js` to use the `position: fixed` strategy to position the popover. */
   positionFixed?: boolean
 
+  /** A wrapping element for an actual content that will be used for positioning. */
+  popper?: SemanticShorthandItem<React.HTMLAttributes<HTMLDivElement>>
+
   /** An array containing custom settings for the Popper.js modifiers. */
   popperModifiers?: any[]
 

src/modules/Popup/Popup.js

@@ -2,13 +2,14 @@ import EventStack from '@semantic-ui-react/event-stack'
 import cx from 'clsx'
 import _ from 'lodash'
 import PropTypes from 'prop-types'
-import React, { Component, createRef } from 'react'
+import React, { Component } from 'react'
 import { Popper } from 'react-popper'
 import shallowEqual from 'shallowequal'
 
 import {
   eventStack,
   childrenUtils,
+  createHTMLDivision,
   customPropTypes,
   getElementType,
   getUnhandledProps,
@@ -32,7 +33,9 @@ export default class Popup extends Component {
   state = {}
 
   open = false
-  triggerRef = createRef()
+  zIndexWasSynced = false
+
+  triggerRef = React.createRef()
 
   static getDerivedStateFromProps(props, state) {
     if (state.closed || state.disabled) return {}
@@ -145,6 +148,7 @@ export default class Popup extends Component {
       flowing,
       header,
       inverted,
+      popper,
       size,
       style,
       wide,
@@ -174,25 +178,36 @@ export default class Popup extends Component {
       ...style,
     }
 
-    return (
-      // https://github.com/popperjs/popper-core/blob/f1f9d1ab75b6b0e962f90a5b2a50f6cfd307d794/src/createPopper.js#L136-L137
-      // Heads up!
-      // A wrapping `div` there is a pure magic, it's required as Popper warns on margins that are
-      // defined by SUI CSS. It also means that this `div` will be positioned instead of `content`.
-      <div ref={popperRef} style={popperStyle}>
-        <ElementType {...contentRestProps} className={classes} style={styles}>
-          {childrenUtils.isNil(children) ? (
-            <>
-              {PopupHeader.create(header, { autoGenerateKey: false })}
-              {PopupContent.create(content, { autoGenerateKey: false })}
-            </>
-          ) : (
-            children
-          )}
-          {hideOnScroll && <EventStack on={this.hideOnScroll} name='scroll' target='window' />}
-        </ElementType>
-      </div>
+    const innerElement = (
+      <ElementType {...contentRestProps} className={classes} style={styles}>
+        {childrenUtils.isNil(children) ? (
+          <>
+            {PopupHeader.create(header, { autoGenerateKey: false })}
+            {PopupContent.create(content, { autoGenerateKey: false })}
+          </>
+        ) : (
+          children
+        )}
+        {hideOnScroll && <EventStack on={this.hideOnScroll} name='scroll' target='window' />}
+      </ElementType>
     )
+
+    // https://github.com/popperjs/popper-core/blob/f1f9d1ab75b6b0e962f90a5b2a50f6cfd307d794/src/createPopper.js#L136-L137
+    // Heads up!
+    // A wrapping `div` there is a pure magic, it's required as Popper warns on margins that are
+    // defined by SUI CSS. It also means that this `div` will be positioned instead of `content`.
+    return createHTMLDivision(popper || {}, {
+      overrideProps: {
+        children: innerElement,
+        ref: popperRef,
+        style: {
+          // Fixes layout for floated elements
+          // https://github.com/Semantic-Org/Semantic-UI-React/issues/4092
+          display: 'flex',
+          ...popperStyle,
+        },
+      },
+    })
   }
 
   render() {
@@ -202,6 +217,7 @@ export default class Popup extends Component {
       eventsEnabled,
       offset,
       pinned,
+      popper,
       popperModifiers,
       position,
       positionFixed,
@@ -220,6 +236,37 @@ export default class Popup extends Component {
       { name: 'preventOverflow', enabled: !!offset },
       { name: 'offset', enabled: !!offset, options: { offset } },
       ...popperModifiers,
+
+      // We are syncing zIndex from `.ui.popup.content` to avoid layering issues as in SUIR we are using an additional
+      // `div` for Popper.js
+      // https://github.com/Semantic-Org/Semantic-UI-React/issues/4083
+      {
+        name: 'syncZIndex',
+        enabled: true,
+        phase: 'beforeRead',
+        fn: ({ state }) => {
+          if (this.zIndexWasSynced) {
+            return
+          }
+
+          // if zIndex defined in <Popup popper={{ style: {} }} /> there is no sense to override it
+          const definedZIndex = popper?.style?.zIndex
+
+          if (_.isUndefined(definedZIndex)) {
+            // eslint-disable-next-line no-param-reassign
+            state.elements.popper.style.zIndex = window.getComputedStyle(
+              state.elements.popper.firstChild,
+            ).zIndex
+          }
+
+          this.zIndexWasSynced = true
+        },
+        effect: () => {
+          return () => {
+            this.zIndexWasSynced = false
+          }
+        },
+      },
     ]
     debug('popper modifiers:', modifiers)
 
@@ -352,6 +399,9 @@ Popup.propTypes = {
   /** Tells `Popper.js` to use the `position: fixed` strategy to position the popover. */
   positionFixed: PropTypes.bool,
 
+  /** A wrapping element for an actual content that will be used for positioning. */
+  popper: customPropTypes.itemShorthand,
+
   /** An array containing custom settings for the Popper.js modifiers. */
   popperModifiers: PropTypes.array,
 

test/specs/modules/Modal/Modal-test.js

@@ -30,6 +30,16 @@ let wrapper
 const wrapperMount = (...args) => (wrapper = mount(...args))
 const wrapperShallow = (...args) => (wrapper = shallow(...args))
 
+// cleanup in `useEffect()` is executed async, so we need to perform a proper cleanup first to avoid
+// collisions with other tests
+function waitForClassesCleanup(done, customAssertions = () => {}) {
+  wrapper.unmount()
+  assertWithTimeout(() => {
+    assertBodyClasses('dimmed', false)
+    customAssertions()
+  }, done)
+}
+
 describe('Modal', () => {
   beforeEach(() => {
     if (wrapper && wrapper.unmount) {
@@ -236,54 +246,68 @@ describe('Modal', () => {
   })
 
   describe('dimmer', () => {
-    it('adds a "dimmer" className to the body', () => {
-      wrapperMount(<Modal open />)
-      assertBodyContains('.ui.page.modals.dimmer.transition.visible.active')
+    describe('ss', () => {
+      it('adds a "dimmer" className to the body', (done) => {
+        wrapperMount(<Modal open />)
+
+        assertBodyContains('.ui.page.modals.dimmer.transition.visible.active')
+        waitForClassesCleanup(done)
+      })
     })
 
     describe('can be "true"', () => {
-      it('adds/removes body classes "dimmable dimmed" on mount/unmount', () => {
+      it('adds/removes body classes "dimmable dimmed" on mount/unmount', (done) => {
         assertBodyClasses('dimmable dimmed', false)
 
         wrapperMount(<Modal open dimmer />)
         assertBodyClasses('dimmable dimmed')
 
         wrapper.setProps({ open: false })
         assertBodyClasses('dimmable dimmed', false)
+
+        waitForClassesCleanup(done)
       })
     })
 
     describe('blurring', () => {
-      it('adds/removes body classes "dimmable dimmed blurring" on mount/unmount', () => {
+      it('adds/removes body classes "dimmable dimmed blurring" on mount/unmount', (done) => {
         assertBodyClasses('dimmable dimmed blurring', false)
 
         wrapperMount(<Modal open dimmer='blurring' />)
         assertBodyClasses('dimmable dimmed blurring')
 
         wrapper.setProps({ open: false })
         assertBodyClasses('dimmable dimmed blurring', false)
+
+        waitForClassesCleanup(done)
       })
 
-      it('adds a dimmer to the body', () => {
+      it('adds a dimmer to the body', (done) => {
         wrapperMount(<Modal open dimmer='blurring' />)
+
         assertBodyContains('.ui.page.modals.dimmer.transition.visible.active')
+        waitForClassesCleanup(done)
       })
     })
 
     describe('inverted', () => {
-      it('adds/removes body classes "dimmable dimmed" on mount/unmount', () => {
+      it('adds/removes body classes "dimmable dimmed" on mount/unmount', (done) => {
         assertBodyClasses('dimmable dimmed', false)
 
         wrapperMount(<Modal open dimmer />)
         assertBodyClasses('dimmable dimmed')
 
         wrapper.setProps({ open: false })
         assertBodyClasses('dimmable dimmed', false)
+
+        waitForClassesCleanup(done)
       })
 
-      it('adds an inverted dimmer to the body', () => {
+      it('adds an inverted dimmer to the body', (done) => {
         wrapperMount(<Modal open dimmer='inverted' />)
+
         assertBodyContains('.ui.inverted.page.modals.dimmer.transition.visible.active')
+        waitForClassesCleanup(done)
       })
     })
 
@@ -509,9 +533,11 @@ describe('Modal', () => {
       window.innerHeight = innerHeight
     })
 
-    it('does not add the scrolling class to the body by default', () => {
+    it('does not add the scrolling class to the body by default', (done) => {
       wrapperMount(<Modal open />)
+
       assertBodyClasses('scrolling', false)
+      waitForClassesCleanup(done)
     })
 
     it('does not add the scrolling class to the body when equal to the window height', (done) => {
@@ -588,12 +614,8 @@ describe('Modal', () => {
       window.innerHeight = 10
       wrapperMount(<Modal open>foo</Modal>)
 
-      requestAnimationFrame(() => {
-        assertBodyClasses('scrolling')
-        wrapper.unmount()
-
+      waitForClassesCleanup(done, () => {
         assertBodyClasses('scrolling', false)
-        done()
       })
     })
   })