microsoft · layershifter · Jan 31, 2020 · Jan 31, 2020 · Jan 31, 2020 · Jan 31, 2020
@@ -34,6 +34,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Update iconOnly button hover, focus styles and add new `background5` and `backgroundHover2` design tokens in Teams theme @codepretty ([#2211](https://github.com/microsoft/fluent-ui-react/pull/2211))
 - Fix `forceUpdate` to get synced updates in React's Concurrent mode @layershifter ([#2268](https://github.com/microsoft/fluent-ui-react/pull/2268))
 - Adding actionable items into `Carousel` @kolaps33 ([#2271](https://github.com/microsoft/fluent-ui-react/pull/2271))
+- Fix positioning fixes for `actions` in `ChatMessage` @layershifter ([#2300](https://github.com/microsoft/fluent-ui-react/pull/2300))
 
 ### Features
 - Allow `useRef` hook used for storing debugging data to be defined in any order with other hooks in functional components @layershifter, @mnajdova ([#2236](https://github.com/microsoft/fluent-ui-react/pull/2236))

@@ -0,0 +1,16 @@
+import { ChatMessage } from '@fluentui/react'
+
+const selectors = {
+  message: `.${ChatMessage.className}`,
+}
+
+const config: ScreenerTestsConfig = {
+  themes: ['teams', 'teamsDark', 'teamsHighContrast'],
+  steps: [
+    builder => builder.hover(selectors.message).snapshot('Hovers the first message'),
+    builder => builder.click(selectors.message).snapshot('Focus the first message via mouse click'),
+    (builder, keys) => builder.keys('body', keys.tab).snapshot('Focuses a message via keyboard'),
+  ],
+}
+
+export default config
@@ -27,7 +27,7 @@ const ChatExampleInScrollableShorthand = () => {
   })
   const [width] = useRangeKnob({
     name: 'width',
-    initialValue: '400px',
+    initialValue: '500px',
     min: '100px',
     max: '500px',
     step: 10,
@@ -81,6 +81,22 @@ const ChatExampleInScrollableShorthand = () => {
       ),
       key: 'message-3',
     },
+    {
+      message: (
+        <Chat.Message actionMenu={actionMenu} content="How are you?" unstable_overflow={overflow} />
+      ),
+      key: 'message-4',
+    },
+    {
+      message: (
+        <Chat.Message
+          actionMenu={actionMenu}
+          content="Do you want something?"
+          unstable_overflow={overflow}
+        />
+      ),
+      key: 'message-5',
+    },
   ]
 
   return (

@@ -17,18 +17,16 @@ const reactions: ShorthandCollection<ReactionProps> = [
     content: '1K',
     key: 'likes',
     variables: { meReacting: true },
+    children: (Component, props) => <ReactionPopup {...props} />,
   },
   {
     icon: 'emoji',
     content: 2,
     key: 'smiles',
+    children: (Component, props) => <ReactionPopup {...props} />,
   },
 ]
 
-const reactionsWithPopup = reactions.map(reaction => render =>
-  render(reaction, (Component, props) => <ReactionPopup {...props} />),
-)
-
 const janeAvatar = {
   image: 'public/images/avatar/small/ade.jpg',
   status: { color: 'green', icon: 'check' },
@@ -95,7 +93,7 @@ const ChatWithPopover = () => {
                   </div>
                 }
                 reactionGroup={{
-                  items: reactionsWithPopup,
+                  items: reactions,
                 }}
                 timestamp="Yesterday, 10:15 PM"
               />
@@ -113,7 +111,7 @@ const ChatWithPopover = () => {
                   </div>
                 }
                 reactionGroup={{
-                  items: reactionsWithPopup,
+                  items: reactions,
                 }}
                 timestamp="Yesterday, 10:15 PM"
               />

@@ -36,16 +36,19 @@ class ReactionPopup extends React.Component<ReactionProps, { open: boolean }> {
             aria-label={getAriaLabel(this.props)}
           />
         }
-        content={
-          <Menu
-            items={['Jane Doe', 'John Doe']}
-            vertical
-            variables={{ borderColor: 'transparent' }}
-            onKeyDown={this.handleKeyDownOnMenu}
-          />
-        }
+        content={{
+          children: () => (
+            <Menu
+              items={['Jane Doe', 'John Doe']}
+              vertical
+              variables={{ borderColor: 'transparent' }}
+              onKeyDown={this.handleKeyDownOnMenu}
+            />
+          ),
+        }}
         inline
         on="hover"
+        position="below"
         open={this.state.open}
         onOpenChange={this.handleOpenChange}
       />

@@ -8,6 +8,7 @@ import {
 import { ChatMessageVariables } from './chatMessageVariables'
 import { screenReaderContainerStyles } from '../../../../utils/accessibility/Styles/accessibilityStyles'
 import { pxToRem } from '../../../../utils'
+import initialPopperStyles from '../../../../utils/positioner/initialStyles'
 import getBorderFocusStyles from '../../getBorderFocusStyles'
 
 const chatMessageStyles: ComponentSlotStylesPrepared<
@@ -95,6 +96,8 @@ const chatMessageStyles: ComponentSlotStylesPrepared<
     // we need higher zIndex for the action menu in order to be displayed above the focus border of the chat message
     zIndex: 1000,
 
+    ...(initialPopperStyles as ICSSInJSStyle),
+
     ...(_.isNil(v.showActionMenu) && {
       overflow: p.focused ? 'visible' : 'hidden',
       // hide and squash actions menu to prevent accidental hovers over its invisible area

@@ -1,6 +1,7 @@
 import { ComponentSlotStylesPrepared, ICSSInJSStyle } from '@fluentui/styles'
 import { PopupProps } from '../../../../components/Popup/Popup'
 import { PopupVariables } from './popupVariables'
+import initialPopperStyles from '../../../../utils/positioner/initialStyles'
 
 const popupStyles: ComponentSlotStylesPrepared<PopupProps, PopupVariables> = {
   root: (): ICSSInJSStyle => ({}),
@@ -11,13 +12,7 @@ const popupStyles: ComponentSlotStylesPrepared<PopupProps, PopupVariables> = {
     color: v.contentColor,
     background: v.contentBackgroundColor,
 
-    // Prevents scroll issue, waiting for Popper.js to add this style once initiated.
-    // https://github.com/mui-org/material-ui/issues/16740
-    position: 'fixed',
-    // Fix Popper.js initial positioning display issue
-    // https://github.com/mui-org/material-ui/issues/17774
-    top: 0,
-    left: '0px /* @noflip */',
+    ...(initialPopperStyles as ICSSInJSStyle),
   }),
 }
 

@@ -1,11 +1,30 @@
-import * as React from 'react'
+import { Ref, isRefObject } from '@fluentui/react-component-ref'
 import * as _ from 'lodash'
 import PopperJS, * as _PopperJS from 'popper.js'
-import { Ref, isRefObject } from '@fluentui/react-component-ref'
+import * as React from 'react'
 
-import { getPlacement, applyRtlToOffset } from './positioningHelper'
-import { PopperProps, PopperChildrenFn } from './types'
 import getScrollParent from './getScrollParent'
+import { getPlacement, applyRtlToOffset } from './positioningHelper'
+import { PopperProps } from './types'
+
+/**
+ * Memoize a result using deep equality. This hook has two advantages over
+ * React.useMemo: it uses deep equality to compare memo keys, and it guarantees
+ * that the memo function will only be called if the keys are unequal.
+ * React.useMemo cannot be relied on to do this, since it is only a performance
+ * optimization (see https://reactjs.org/docs/hooks-reference.html#usememo).
+ *
+ * Copied from https://github.com/apollographql/react-apollo/blob/master/packages/hooks/src/utils/useDeepMemo.ts.
+ */
+function useDeepMemo<TKey, TValue>(memoFn: () => TValue, key: TKey): TValue {
+  const ref = React.useRef<{ key: TKey; value: TValue }>()
+
+  if (!ref.current || !_.isEqual(key, ref.current.key)) {
+    ref.current = { key, value: memoFn() }
+  }
+
+  return ref.current.value
+}
 
 // `popper.js` has a UMD build without `.default`, it breaks CJS builds:
 // https://github.com/rollup/rollup/issues/1267#issuecomment-446681320
@@ -35,6 +54,7 @@ const createPopper = (
 
   return instance
 }
+
 /**
  * Popper relies on the 3rd party library [Popper.js](https://github.com/FezVrasta/popper.js) for positioning.
  */
@@ -58,18 +78,61 @@ const Popper: React.FunctionComponent<PopperProps> = props => {
 
   const popperRef = React.useRef<PopperJS>()
   const contentRef = React.useRef<HTMLElement>(null)
+
   const latestPlacement = React.useRef<PopperJS.Placement>(proposedPlacement)
   const [computedPlacement, setComputedPlacement] = React.useState<PopperJS.Placement>(
     proposedPlacement,
   )
 
-  const computedModifiers: PopperJS.Modifiers = React.useMemo(
+  const hasScrollableElement = React.useMemo(() => {
+    const scrollParentElement = getScrollParent(contentRef.current)
+
+    return scrollParentElement !== scrollParentElement.ownerDocument.body
+  }, [contentRef])
+  // Is a broken dependency and can cause potential bugs, we should rethink this as all other refs
+  // in this component.
+
+  const computedModifiers: PopperJS.Modifiers = useDeepMemo(
     () =>
-      offset && {
-        offset: { offset: rtl ? applyRtlToOffset(offset, position) : offset },
-        keepTogether: { enabled: false },
-      },
-    [rtl, offset, position],
+      _.merge(
+        /**
+         * This prevents blurrines in chrome, when the coordinates are odd numbers alternative
+         * would be to use `fn` and manipulate the computed style or ask popper to fix it but
+         * since there is presumably only handful of poppers displayed on the page, the
+         * performance impact should be minimal.
+         */
+        { computeStyle: { gpuAcceleration: false } },
+
+        { flip: { padding: 0, flipVariationsByContent: true } },
+        { preventOverflow: { padding: 0 } },
+
+        offset && {
+          offset: { offset: rtl ? applyRtlToOffset(offset, position) : offset },
+          keepTogether: { enabled: false },
+        },
+
+        /**
+         * When the popper box is placed in the context of a scrollable element, we need to set
+         * preventOverflow.escapeWithReference to true and flip.boundariesElement to 'scrollParent'
+         * (default is 'viewport') so that the popper box will stick with the targetRef when we
+         * scroll targetRef out of the viewport.
+         */
+        hasScrollableElement && {
+          preventOverflow: { escapeWithReference: true },
+          flip: { boundariesElement: 'scrollParent' },
+        },
+
+        userModifiers,
+
+        /**
+         * unstable_pinned disables the flip modifier by setting flip.enabled to false; this
+         * disables automatic repositioning of the popper box; it will always be placed according to
+         * the values of `align` and `position` props, regardless of the size of the component, the
+         * reference element or the viewport.
+         */
+        unstable_pinned && { flip: { enabled: false } },
+      ),
+    [hasScrollableElement, position, offset, rtl, unstable_pinned, userModifiers],
   )
 
   const scheduleUpdate = React.useCallback(() => {
@@ -102,48 +165,6 @@ const Popper: React.FunctionComponent<PopperProps> = props => {
       return
     }
 
-    const pointerTargetRefElement = pointerTargetRef && pointerTargetRef.current
-    const scrollParentElement = getScrollParent(contentRef.current)
-    const popperHasScrollableParent = scrollParentElement !== scrollParentElement.ownerDocument.body
-
-    const modifiers: PopperJS.Modifiers = _.merge(
-      { preventOverflow: { padding: 0 } },
-      { flip: { padding: 0, flipVariationsByContent: true } },
-      /**
-       * This prevents blurrines in chrome, when the coordinates are odd numbers
-       * alternative would be to use `fn`, call _PopperJS.default.Defaults.modifiers.computeStyle.fn(data, options)
-       * and manipulate the computeed style or ask popper to fix it
-       * but since there is presumably only handful of poppers displayed on the page, the performance impact should be minimal
-       */
-      { computeStyle: { gpuAcceleration: false } },
-      /**
-       * When the popper box is placed in the context of a scrollable element, we need to set
-       * preventOverflow.escapeWithReference to true and flip.boundariesElement to 'scrollParent' (default is 'viewport')
-       * so that the popper box will stick with the targetRef when we scroll targetRef out of the viewport.
-       */
-      popperHasScrollableParent && {
-        preventOverflow: { escapeWithReference: true },
-        flip: { boundariesElement: 'scrollParent' },
-      },
-      /**
-       * unstable_pinned disables the flip modifier by setting flip.enabled to false; this disables automatic
-       * repositioning of the popper box; it will always be placed according to the values of `align` and
-       * `position` props, regardless of the size of the component, the reference element or the viewport.
-       */
-      unstable_pinned && { flip: { enabled: false } },
-      computedModifiers,
-      userModifiers,
-      /**
-       * This modifier is necessary in order to render the pointer.
-       */
-      {
-        arrow: {
-          enabled: !!pointerTargetRefElement,
-          element: pointerTargetRefElement,
-        },
-      },
-    )
-
     const handleUpdate = (data: PopperJS.Data) => {
       // PopperJS performs computations that might update the computed placement: auto positioning, flipping the
       // placement in case the popper box should be rendered at the edge of the viewport and does not fit
@@ -156,21 +177,32 @@ const Popper: React.FunctionComponent<PopperProps> = props => {
     const options: PopperJS.PopperOptions = {
       placement: proposedPlacement,
       positionFixed,
-      modifiers,
+      modifiers: {
+        ...computedModifiers,
+
+        /**
+         * This modifier is necessary in order to render the pointer. Refs are resolved in effects, so it can't be
+         * placed under computed modifiers. Deep merge is not required as this modifier has only these properties.
+         */
+        arrow: {
+          enabled: !!(pointerTargetRef && pointerTargetRef.current),
+          element: pointerTargetRef && pointerTargetRef.current,
+        },
+      },
       onCreate: handleUpdate,
       onUpdate: handleUpdate,
     }
 
     popperRef.current = createPopper(reference, contentRef.current, options)
   }, [
     // TODO review dependencies for popperHasScrollableParent
-    computedModifiers,
     enabled,
-    userModifiers,
+    computedModifiers,
+    pointerTargetRef,
     positionFixed,
     proposedPlacement,
-    unstable_pinned,
     targetRef,
+    unstable_pinned,
   ])
 
   React.useLayoutEffect(() => {
@@ -182,13 +214,10 @@ const Popper: React.FunctionComponent<PopperProps> = props => {
 
   const child =
     typeof children === 'function'
-      ? (children as PopperChildrenFn)({
-          placement: computedPlacement,
-          scheduleUpdate,
-        })
-      : children
+      ? children({ placement: computedPlacement, scheduleUpdate })
+      : (children as React.ReactElement)
 
-  return <Ref innerRef={contentRef}>{React.Children.only(child) as React.ReactElement}</Ref>
+  return <Ref innerRef={contentRef}>{React.Children.only(child)}</Ref>
 }
 
 Popper.defaultProps = {

@@ -0,0 +1,13 @@
+import * as React from 'react'
+
+const initialStyles: React.CSSProperties = {
+  // Prevents scroll issue, waiting for Popper.js to add this style once initiated.
+  // https://github.com/mui-org/material-ui/issues/16740
+  position: 'fixed',
+  // Fix Popper.js initial positioning display issue
+  // https://github.com/mui-org/material-ui/issues/17774
+  top: 0,
+  left: '0px /* @noflip */',
+}
+
+export default initialStyles