Fix tooltip placement bugs by upgrading Popper

signalapp · Aug 3, 2021 · 3b476fb · 3b476fb
1 parent 26b7652
commit 3b476fb
Show file tree

Hide file tree

Showing 15 changed files with 85 additions and 136 deletions.
diff --git a/ACKNOWLEDGMENTS.md b/ACKNOWLEDGMENTS.md
@@ -5,6 +5,10 @@
 
 Signal Desktop makes use of the following open source projects.
 
+## @popperjs/core
+
+    License: MIT
+
 ## @sindresorhus/is
 
     MIT License
@@ -2478,10 +2482,6 @@ Signal Desktop makes use of the following open source projects.
 
     License: MIT
 
-## popper.js
-
-    License: MIT
-
 ## protobufjs
 
     This license applies to all parts of protobuf.js except those files

diff --git a/package.json b/package.json
@@ -69,6 +69,7 @@
     "fs-xattr": "0.3.0"
   },
   "dependencies": {
+    "@popperjs/core": "2.9.2",
     "@signalapp/signal-client": "0.8.1",
     "@sindresorhus/is": "0.8.0",
     "@types/pino": "6.3.6",
@@ -123,7 +124,6 @@
     "pify": "3.0.0",
     "pino": "6.11.1",
     "pino-multi-stream": "5.3.0",
-    "popper.js": "1.15.0",
     "protobufjs": "6.10.2",
     "proxy-agent": "4.0.1",
     "quill": "1.3.7",
@@ -135,7 +135,7 @@
     "react-dropzone": "10.1.7",
     "react-hot-loader": "4.12.11",
     "react-measure": "2.3.0",
-    "react-popper": "1.3.7",
+    "react-popper": "2.2.5",
     "react-quill": "2.0.0-beta.2",
     "react-redux": "7.1.0",
     "react-router-dom": "5.0.1",

diff --git a/sticker-creator/components/StickerFrame.tsx b/sticker-creator/components/StickerFrame.tsx
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import * as React from 'react';
@@ -260,7 +260,12 @@ export const StickerFrame = React.memo(
               previewActive &&
               previewPopperRoot
                 ? createPortal(
-                    <Popper placement="bottom">
+                    <Popper
+                      placement="bottom"
+                      modifiers={[
+                        { name: 'offset', options: { offset: [undefined, 8] } },
+                      ]}
+                    >
                       {({ ref, style, arrowProps, placement }) => (
                         <StickerPreview
                           ref={ref}

diff --git a/sticker-creator/elements/StickerPreview.scss b/sticker-creator/elements/StickerPreview.scss
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 @import '../mixins';
@@ -39,26 +39,6 @@
   border-radius: 0 6px 6px 0;
 }
 
-.bottom {
-  composes: base;
-  margin-top: 8px;
-}
-
-.top {
-  composes: base;
-  margin-bottom: 8px;
-}
-
-.left {
-  composes: base;
-  margin-right: 8px;
-}
-
-.right {
-  composes: base;
-  margin-left: 8px;
-}
-
 .arrow {
   position: absolute;
   width: 0;

diff --git a/sticker-creator/elements/StickerPreview.tsx b/sticker-creator/elements/StickerPreview.tsx
@@ -1,9 +1,9 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import * as React from 'react';
 import { PopperArrowProps } from 'react-popper';
-import { Placement } from 'popper.js';
+import { Placement } from '@popperjs/core';
 import * as styles from './StickerPreview.scss';
 import { MessageBubble } from './MessageBubble';
 import { MessageSticker, Props as MessageStickerProps } from './MessageSticker';
@@ -26,22 +26,6 @@ const renderMessages = (
   </>
 );
 
-const getBaseClass = (placement?: Placement) => {
-  if (placement === 'top') {
-    return styles.top;
-  }
-
-  if (placement === 'right') {
-    return styles.right;
-  }
-
-  if (placement === 'left') {
-    return styles.left;
-  }
-
-  return styles.bottom;
-};
-
 const getArrowClass = (placement?: Placement) => {
   if (placement === 'top') {
     return styles.arrowBottom;
@@ -64,7 +48,7 @@ export const StickerPreview = React.memo(
       const i18n = useI18n();
 
       return (
-        <div className={getBaseClass(placement)} ref={ref} style={style}>
+        <div className={styles.base} ref={ref} style={style}>
           {arrowProps ? (
             <div
               ref={arrowProps.ref}

diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss
@@ -4989,7 +4989,6 @@ button.module-image__border-overlay:focus {
   height: 56px;
   border-radius: 30px;
   position: relative;
-  margin: 4px 0;
   z-index: 2;
 
   animation: {
@@ -9908,8 +9907,6 @@ $contact-modal-padding: 18px;
   }
 
   &[data-placement='top'] {
-    margin-bottom: 12px;
-
     .module-tooltip-arrow {
       bottom: 0;
     }
@@ -9921,8 +9918,6 @@ $contact-modal-padding: 18px;
   }
 
   &[data-placement='right'] {
-    margin-left: 12px;
-
     .module-tooltip-arrow {
       left: 0;
     }
@@ -9934,8 +9929,6 @@ $contact-modal-padding: 18px;
   }
 
   &[data-placement='bottom'] {
-    margin-top: 12px;
-
     .module-tooltip-arrow {
       top: 0;
     }
@@ -9947,8 +9940,6 @@ $contact-modal-padding: 18px;
   }
 
   &[data-placement='left'] {
-    margin-right: 12px;
-
     .module-tooltip-arrow {
       right: 0;
     }

diff --git a/ts/components/MediaQualitySelector.tsx b/ts/components/MediaQualitySelector.tsx
@@ -103,7 +103,7 @@ export const MediaQualitySelector = ({
       </Reference>
       {menuShowing && popperRoot
         ? createPortal(
-            <Popper placement="top-start" positionFixed>
+            <Popper placement="top-start" strategy="fixed">
               {({ ref, style, placement }) => (
                 <div
                   className="MediaQualitySelector__popper"

diff --git a/ts/components/Tooltip.tsx b/ts/components/Tooltip.tsx
@@ -7,6 +7,7 @@ import { noop } from 'lodash';
 import { Manager, Reference, Popper } from 'react-popper';
 import { Theme, themeClassName } from '../util/theme';
 import { multiRef } from '../util/multiRef';
+import { offsetDistanceModifier } from '../util/popperUtil';
 
 type EventWrapperPropsType = {
   children: React.ReactNode;
@@ -96,7 +97,7 @@ export const Tooltip: React.FC<PropsType> = ({
           </TooltipEventWrapper>
         )}
       </Reference>
-      <Popper placement={direction}>
+      <Popper placement={direction} modifiers={[offsetDistanceModifier(12)]}>
         {({ arrowProps, placement, ref, style }) =>
           showTooltip && (
             <div

diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx
@@ -63,6 +63,7 @@ import { createRefMerger } from '../_util';
 import { emojiToData } from '../emoji/lib';
 import { SmartReactionPicker } from '../../state/smart/ReactionPicker';
 import { getCustomColorStyle } from '../../util/getCustomColorStyle';
+import { offsetDistanceModifier } from '../../util/popperUtil';
 
 type Trigger = {
   handleContextClick: (event: React.MouseEvent<HTMLDivElement>) => void;
@@ -1429,7 +1430,7 @@ export class Message extends React.Component<Props, State> {
         {reactionPickerRoot &&
           createPortal(
             // eslint-disable-next-line consistent-return
-            <Popper placement="top">
+            <Popper placement="top" modifiers={[offsetDistanceModifier(4)]}>
               {({ ref, style }) => (
                 <SmartReactionPicker
                   ref={ref}

diff --git a/ts/components/emoji/EmojiButton.tsx b/ts/components/emoji/EmojiButton.tsx
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import * as React from 'react';
@@ -129,7 +129,7 @@ export const EmojiButton = React.memo(
         </Reference>
         {open && popperRoot
           ? createPortal(
-              <Popper placement="top-start" positionFixed>
+              <Popper placement="top-start" strategy="fixed">
                 {({ ref, style }) => (
                   <EmojiPicker
                     ref={ref}

diff --git a/ts/components/stickers/StickerButton.tsx b/ts/components/stickers/StickerButton.tsx
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import * as React from 'react';
@@ -10,6 +10,7 @@ import { StickerPicker } from './StickerPicker';
 import { countStickers } from './lib';
 import { StickerPackType, StickerType } from '../../state/ducks/stickers';
 import { LocalizerType } from '../../types/Util';
+import { offsetDistanceModifier } from '../../util/popperUtil';
 
 export type OwnProps = {
   readonly i18n: LocalizerType;
@@ -202,7 +203,11 @@ export const StickerButton = React.memo(
           )}
         </Reference>
         {!open && !showIntroduction && installedPack ? (
-          <Popper placement={position} key={installedPack.id}>
+          <Popper
+            placement={position}
+            key={installedPack.id}
+            modifiers={[offsetDistanceModifier(6)]}
+          >
             {({ ref, style, placement, arrowProps }) => (
               <button
                 type="button"
@@ -239,7 +244,7 @@ export const StickerButton = React.memo(
           </Popper>
         ) : null}
         {!open && showIntroduction ? (
-          <Popper placement={position}>
+          <Popper placement={position} modifiers={[offsetDistanceModifier(6)]}>
             {({ ref, style, placement, arrowProps }) => (
               <button
                 type="button"

diff --git a/ts/quill/emoji/completion.tsx b/ts/quill/emoji/completion.tsx
@@ -19,6 +19,7 @@ import {
 import { Emoji } from '../../components/emoji/Emoji';
 import { EmojiPickDataType } from '../../components/emoji/EmojiPicker';
 import { getBlotTextPartitions, matchBlotTextPartitions } from '../util';
+import { sameWidthModifier } from '../../util/popperUtil';
 
 const Keyboard = Quill.import('modules/keyboard');
 
@@ -265,25 +266,7 @@ export class EmojiCompletion {
     }
 
     const element = createPortal(
-      <Popper
-        placement="top"
-        modifiers={{
-          width: {
-            enabled: true,
-            fn: oldData => {
-              const data = oldData;
-              const { width, left } = data.offsets.reference;
-
-              data.styles.width = `${width}px`;
-              data.offsets.popper.width = width;
-              data.offsets.popper.left = left;
-
-              return data;
-            },
-            order: 840,
-          },
-        }}
-      >
+      <Popper placement="top-start" modifiers={[sameWidthModifier]}>
         {({ ref, style }) => (
           <div
             ref={ref}

diff --git a/ts/quill/mentions/completion.tsx b/ts/quill/mentions/completion.tsx
@@ -14,6 +14,7 @@ import { Avatar } from '../../components/Avatar';
 import { LocalizerType } from '../../types/Util';
 import { MemberRepository } from '../memberRepository';
 import { matchBlotTextPartitions } from '../util';
+import { sameWidthModifier } from '../../util/popperUtil';
 
 export type MentionCompletionOptions = {
   i18n: LocalizerType;
@@ -218,25 +219,7 @@ export class MentionCompletion {
     }
 
     const element = createPortal(
-      <Popper
-        placement="top"
-        modifiers={{
-          width: {
-            enabled: true,
-            fn: oldData => {
-              const data = oldData;
-              const { width, left } = data.offsets.reference;
-
-              data.styles.width = `${width}px`;
-              data.offsets.popper.width = width;
-              data.offsets.popper.left = left;
-
-              return data;
-            },
-            order: 840,
-          },
-        }}
-      >
+      <Popper placement="top-start" modifiers={[sameWidthModifier]}>
         {({ ref, style }) => (
           <div
             ref={ref}

diff --git a/ts/util/popperUtil.ts b/ts/util/popperUtil.ts
@@ -0,0 +1,33 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Modifier } from '@popperjs/core';
+import type { OffsetModifier } from '@popperjs/core/lib/modifiers/offset';
+
+/**
+ * Shorthand for the [offset modifier][0] when you just wanna set the distance.
+ *
+ * [0]: https://popper.js.org/docs/v2/modifiers/offset/
+ */
+export const offsetDistanceModifier = (
+  distance: number
+): Partial<OffsetModifier> => ({
+  name: 'offset',
+  options: { offset: [undefined, distance] },
+});
+
+/**
+ * Make the popper element the same width as the reference, even when you resize.
+ *
+ * Should probably be used with the "top-start", "top-end", "bottom-start", or
+ * "bottom-end" placement.
+ */
+export const sameWidthModifier: Modifier<'sameWidth', unknown> = {
+  name: 'sameWidth',
+  enabled: true,
+  phase: 'write',
+  fn({ state }) {
+    // eslint-disable-next-line no-param-reassign
+    state.elements.popper.style.width = `${state.rects.reference.width}px`;
+  },
+};