From c09145da0f138d398e079ed09c9fb75287941cc9 Mon Sep 17 00:00:00 2001
From: Marcus Eide <eide@spotify.com>
Date: Fri, 25 Sep 2020 10:23:17 +0200
Subject: [PATCH] Refactor tooltip placements

---
 .../src/components/FeatureFlagsItem.tsx       | 18 +++---
 .../src/components/OAuthProviderSettings.tsx  |  1 +
 .../src/components/OIDCProviderSettings.tsx   |  1 +
 .../src/components/PinButton.tsx              | 35 ++++++-----
 .../src/components/ProviderSettingsItem.tsx   | 60 +++++++++----------
 .../src/components/ThemeToggle.tsx            | 34 ++++++++---
 6 files changed, 81 insertions(+), 68 deletions(-)

diff --git a/plugins/user-settings/src/components/FeatureFlagsItem.tsx b/plugins/user-settings/src/components/FeatureFlagsItem.tsx
index 7f7f990e48..92065f2ed6 100644
--- a/plugins/user-settings/src/components/FeatureFlagsItem.tsx
+++ b/plugins/user-settings/src/components/FeatureFlagsItem.tsx
@@ -38,16 +38,16 @@ export const FlagItem = ({ flag, enabled, toggleHandler }: Props) => (
       secondary={`Registered in ${flag.pluginId} plugin`}
     />
     <ListItemSecondaryAction>
-      <ToggleButton
-        size="small"
-        value="flag"
-        selected={enabled}
-        onChange={() => toggleHandler(flag.name)}
-      >
-        <Tooltip placement="top" arrow title={enabled ? 'Disable' : 'Enable'}>
+      <Tooltip placement="top" arrow title={enabled ? 'Disable' : 'Enable'}>
+        <ToggleButton
+          size="small"
+          value="flag"
+          selected={enabled}
+          onChange={() => toggleHandler(flag.name)}
+        >
           <CheckIcon color={enabled ? 'primary' : undefined} />
-        </Tooltip>
-      </ToggleButton>
+        </ToggleButton>
+      </Tooltip>
     </ListItemSecondaryAction>
   </ListItem>
 );
diff --git a/plugins/user-settings/src/components/OAuthProviderSettings.tsx b/plugins/user-settings/src/components/OAuthProviderSettings.tsx
index 2ecc5536ea..669dd89822 100644
--- a/plugins/user-settings/src/components/OAuthProviderSettings.tsx
+++ b/plugins/user-settings/src/components/OAuthProviderSettings.tsx
@@ -71,6 +71,7 @@ export const OAuthProviderSettings: FC<OAuthProviderSidebarProps> = ({
   return (
     <ProviderSettingsItem
       title={title}
+      description={apiRef.description}
       icon={icon}
       signedIn={signedIn}
       api={api}
diff --git a/plugins/user-settings/src/components/OIDCProviderSettings.tsx b/plugins/user-settings/src/components/OIDCProviderSettings.tsx
index 1516b3514d..1e6005cb82 100644
--- a/plugins/user-settings/src/components/OIDCProviderSettings.tsx
+++ b/plugins/user-settings/src/components/OIDCProviderSettings.tsx
@@ -72,6 +72,7 @@ export const OIDCProviderSettings: FC<OIDCProviderSidebarProps> = ({
   return (
     <ProviderSettingsItem
       title={title}
+      description={apiRef.description}
       icon={icon}
       signedIn={signedIn}
       api={api}
diff --git a/plugins/user-settings/src/components/PinButton.tsx b/plugins/user-settings/src/components/PinButton.tsx
index be4024f954..8e50eae9d4 100644
--- a/plugins/user-settings/src/components/PinButton.tsx
+++ b/plugins/user-settings/src/components/PinButton.tsx
@@ -31,15 +31,8 @@ export const SidebarPinButton = () => {
     SidebarPinStateContext,
   );
 
-  const PinIcon = () => (
-    <Tooltip
-      placement="top"
-      arrow
-      title={`${isPinned ? 'Unpin' : 'Pin'} Sidebar`}
-    >
-      {isPinned ? <LockIcon color="primary" /> : <LockOpenIcon />}
-    </Tooltip>
-  );
+  const PinIcon = () =>
+    isPinned ? <LockIcon color="primary" /> : <LockOpenIcon />;
 
   return (
     <ListItem>
@@ -48,16 +41,22 @@ export const SidebarPinButton = () => {
         secondary="Prevent the sidebar from collapsing"
       />
       <ListItemSecondaryAction>
-        <ToggleButton
-          size="small"
-          value="pin"
-          selected={isPinned}
-          onChange={() => {
-            toggleSidebarPinState();
-          }}
+        <Tooltip
+          placement="top"
+          arrow
+          title={`${isPinned ? 'Unpin' : 'Pin'} Sidebar`}
         >
-          <PinIcon />
-        </ToggleButton>
+          <ToggleButton
+            size="small"
+            value="pin"
+            selected={isPinned}
+            onChange={() => {
+              toggleSidebarPinState();
+            }}
+          >
+            <PinIcon />
+          </ToggleButton>
+        </Tooltip>
       </ListItemSecondaryAction>
     </ListItem>
   );
diff --git a/plugins/user-settings/src/components/ProviderSettingsItem.tsx b/plugins/user-settings/src/components/ProviderSettingsItem.tsx
index 018e6a1326..0fb3d3e97d 100644
--- a/plugins/user-settings/src/components/ProviderSettingsItem.tsx
+++ b/plugins/user-settings/src/components/ProviderSettingsItem.tsx
@@ -35,51 +35,47 @@ import {
 
 type OAuthProviderSidebarProps = {
   title: string;
+  description: string;
   icon: IconComponent;
   apiRef: ApiRef<SessionApi>;
 };
 
 export const ProviderSettingsItem: FC<OAuthProviderSidebarProps> = ({
   title,
+  description,
   icon: Icon,
-  apiRef,
-}) => {
-  const api = useApi(apiRef);
-  const [signedIn, setSignedIn] = useState(false);
-
-  useEffect(() => {
-    let didCancel = false;
-
-    const subscription = api
-      .sessionState$()
-      .subscribe((sessionState: SessionState) => {
-        if (!didCancel) {
-          setSignedIn(sessionState === SessionState.SignedIn);
-        }
-      });
-
-    return () => {
-      didCancel = true;
-      subscription.unsubscribe();
-    };
-  }, [api]);
-
-  return (
-    <ListItem>
-      <ListItemIcon>
-        <Icon />
-      </ListItemIcon>
-      <ListItemText primary={title} />
-      <ListItemSecondaryAction>
+  signedIn,
+  api,
+  signInHandler,
+}: Props) => (
+  <ListItem>
+    <ListItemIcon>
+      <Icon />
+    </ListItemIcon>
+    <ListItemText
+      primary={title}
+      secondary={
+        <Tooltip placement="top" arrow title={description}>
+          <span>{description}</span>
+        </Tooltip>
+      }
+      secondaryTypographyProps={{ noWrap: true, style: { width: '80%' } }}
+    />
+    <ListItemSecondaryAction>
+      <Tooltip
+        placement="top"
+        arrow
+        title={signedIn ? `Sign out from ${title}` : `Sign in to ${title}`}
+      >
         <ToggleButton
           size="small"
           value={title}
           selected={signedIn}
-          onChange={() => (signedIn ? api.signOut() : api.signIn())}
+          onChange={() => (signedIn ? api.logout() : signInHandler())}
         >
           <PowerButton color={signedIn ? 'primary' : undefined} />
-        </Tooltip>
-      </ToggleButton>
+        </ToggleButton>
+      </Tooltip>
     </ListItemSecondaryAction>
   </ListItem>
 );
diff --git a/plugins/user-settings/src/components/ThemeToggle.tsx b/plugins/user-settings/src/components/ThemeToggle.tsx
index 9acfc3407f..166d0454c5 100644
--- a/plugins/user-settings/src/components/ThemeToggle.tsx
+++ b/plugins/user-settings/src/components/ThemeToggle.tsx
@@ -47,21 +47,34 @@ export const SidebarThemeToggle = () => {
     }
   };
 
+  // ToggleButtonGroup uses React.children.map instead of context
+  // so wrapping with Tooltip breaks ToggleButton functionality.
+  const TooltipToggleButton = ({
+    children,
+    title,
+    value,
+    ...props
+  }: {
+    children: JSX.Element;
+    title: string;
+    value: string;
+  }) => (
+    <Tooltip placement="top" arrow title={title}>
+      <ToggleButton value={value} {...props}>
+        {children}
+      </ToggleButton>
+    </Tooltip>
+  );
+
   const ThemeIcon = ({ theme }: { theme: AppTheme }) => {
     const themeIcon = themeIds.find(t => t.id === theme.id)?.icon;
-    const icon = themeIcon ? (
+    return themeIcon ? (
       cloneElement(themeIcon, {
         color: themeId === theme.id ? 'primary' : undefined,
       })
     ) : (
       <AutoIcon color={themeId === theme.id ? 'primary' : undefined} />
     );
-
-    return (
-      <Tooltip placement="top" arrow title={`Select ${theme.variant} theme`}>
-        {icon}
-      </Tooltip>
-    );
   };
 
   return (
@@ -75,9 +88,12 @@ export const SidebarThemeToggle = () => {
           onChange={handleSetTheme}
         >
           {themeIds.map(theme => (
-            <ToggleButton key={theme.id} value={theme.variant}>
+            <TooltipToggleButton
+              title={`Select ${theme.variant} theme`}
+              value={theme.variant}
+            >
               <ThemeIcon theme={theme} />
-            </ToggleButton>
+            </TooltipToggleButton>
           ))}
           <ToggleButton value="auto">
             <Tooltip placement="top" arrow title="Select auto theme">