From 9ada27526c32ee7e53e4ee5bf7da54d6abff2006 Mon Sep 17 00:00:00 2001
From: John Knox <jknox@fb.com>
Date: Fri, 24 Jul 2020 07:11:49 -0700
Subject: [PATCH] Change RatingButton to use Popover2

Summary:
The original Popover component doesn't work when the popover goes out of bounds of the current container, and overflow is not visible, which is the case in the title bar.

This changes it to use a new absolutely positioned one instead.

Reviewed By: mweststrate

Differential Revision: D22724110

fbshipit-source-id: f41b6d154ccb66a94b63533fe9c30d2a2c49f1db
---
 desktop/app/src/chrome/RatingButton.tsx | 37 +++++++++++++------------
 desktop/app/src/init.tsx                | 33 ++++++++++++----------
 2 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/desktop/app/src/chrome/RatingButton.tsx b/desktop/app/src/chrome/RatingButton.tsx
index 86153e927..6ec6fda7f 100644
--- a/desktop/app/src/chrome/RatingButton.tsx
+++ b/desktop/app/src/chrome/RatingButton.tsx
@@ -7,10 +7,9 @@
  * @format
  */
 
-import React, {Component, ReactElement} from 'react';
+import React, {Component, ReactElement, RefObject} from 'react';
 import {
   Glyph,
-  Popover,
   FlexColumn,
   FlexRow,
   Button,
@@ -18,6 +17,7 @@ import {
   styled,
   Input,
 } from 'flipper';
+import Popover from '../ui/components/Popover2';
 import GK from '../fb-stubs/GK';
 import * as UserFeedback from '../fb-stubs/UserFeedback';
 import {FeedbackPrompt} from '../fb-stubs/UserFeedback';
@@ -295,6 +295,8 @@ class RatingButton extends Component<PropsFromState, State> {
     hasTriggered: false,
   };
 
+  glyphRef: RefObject<HTMLDivElement> = React.createRef();
+
   constructor(props: PropsFromState) {
     super(props);
     if (GK.get('flipper_rating')) {
@@ -351,7 +353,11 @@ class RatingButton extends Component<PropsFromState, State> {
     }
     return (
       <div style={{position: 'relative'}}>
-        <div onClick={this.onClick.bind(this)}>
+        <div
+          role="button"
+          tabIndex={0}
+          onClick={this.onClick.bind(this)}
+          ref={this.glyphRef}>
           <Glyph
             name="star"
             color="grey"
@@ -359,20 +365,17 @@ class RatingButton extends Component<PropsFromState, State> {
           />
         </div>
         {this.state.isShown ? (
-          <Popover
-            onDismiss={() => {}}
-            children={
-              <FeedbackComponent
-                submitRating={this.submitRating.bind(this)}
-                submitComment={this.submitComment.bind(this)}
-                close={() => {
-                  this.setState({isShown: false});
-                }}
-                dismiss={this.onClick.bind(this)}
-                promptData={promptData}
-              />
-            }
-          />
+          <Popover id="rating-button" targetRef={this.glyphRef}>
+            <FeedbackComponent
+              submitRating={this.submitRating.bind(this)}
+              submitComment={this.submitComment.bind(this)}
+              close={() => {
+                this.setState({isShown: false});
+              }}
+              dismiss={this.onClick.bind(this)}
+              promptData={promptData}
+            />
+          </Popover>
         ) : null}
       </div>
     );
diff --git a/desktop/app/src/init.tsx b/desktop/app/src/init.tsx
index 58bd12dc2..cef429851 100644
--- a/desktop/app/src/init.tsx
+++ b/desktop/app/src/init.tsx
@@ -35,6 +35,7 @@ import {CacheProvider} from '@emotion/core';
 import {enableMapSet} from 'immer';
 import os from 'os';
 import QuickPerformanceLogger, {FLIPPER_QPL_EVENTS} from './fb-stubs/QPL';
+import {PopoverProvider} from './ui/components/PopoverProvider';
 
 if (process.env.NODE_ENV === 'development' && os.platform() === 'darwin') {
   // By default Node.JS has its internal certificate storage and doesn't use
@@ -65,21 +66,23 @@ const AppFrame = () => {
 
   return (
     <TooltipProvider>
-      <ContextMenuProvider>
-        <Provider store={store}>
-          <CacheProvider value={cache}>
-            {warnEmployee ? (
-              <WarningEmployee
-                onClick={() => {
-                  setWarnEmployee(false);
-                }}
-              />
-            ) : (
-              <App logger={logger} />
-            )}
-          </CacheProvider>
-        </Provider>
-      </ContextMenuProvider>
+      <PopoverProvider>
+        <ContextMenuProvider>
+          <Provider store={store}>
+            <CacheProvider value={cache}>
+              {warnEmployee ? (
+                <WarningEmployee
+                  onClick={() => {
+                    setWarnEmployee(false);
+                  }}
+                />
+              ) : (
+                <App logger={logger} />
+              )}
+            </CacheProvider>
+          </Provider>
+        </ContextMenuProvider>
+      </PopoverProvider>
     </TooltipProvider>
   );
 };