From fdd976a15ab3922065137b1ffad72321b0ccaef1 Mon Sep 17 00:00:00 2001
From: Marcus Eide <eide@spotify.com>
Date: Fri, 2 Oct 2020 10:35:16 +0200
Subject: [PATCH] Move small components out

---
 .../src/components/FeatureFlagsList.tsx       |  4 +-
 .../src/components/PinButton.tsx              | 10 +-
 .../src/components/ThemeToggle.tsx            | 98 ++++++++++---------
 3 files changed, 61 insertions(+), 51 deletions(-)

diff --git a/plugins/user-settings/src/components/FeatureFlagsList.tsx b/plugins/user-settings/src/components/FeatureFlagsList.tsx
index ec4ad51457..2636d0e66f 100644
--- a/plugins/user-settings/src/components/FeatureFlagsList.tsx
+++ b/plugins/user-settings/src/components/FeatureFlagsList.tsx
@@ -25,9 +25,7 @@ import {
 } from '@backstage/core';
 import { FlagItem } from './FeatureFlagsItem';
 
-type Props = {
-  featureFlags: FeatureFlagsRegistryItem[];
-};
+type Props = { featureFlags: FeatureFlagsRegistryItem[] };
 
 export const FeatureFlagsList = ({ featureFlags }: Props) => {
   const featureFlagApi = useApi(featureFlagsApiRef);
diff --git a/plugins/user-settings/src/components/PinButton.tsx b/plugins/user-settings/src/components/PinButton.tsx
index f8017ef215..e36fc365f8 100644
--- a/plugins/user-settings/src/components/PinButton.tsx
+++ b/plugins/user-settings/src/components/PinButton.tsx
@@ -26,14 +26,16 @@ import LockOpenIcon from '@material-ui/icons/LockOpen';
 import { ToggleButton } from '@material-ui/lab';
 import { SidebarPinStateContext } from '@backstage/core';
 
+type PinIconProps = { isPinned: boolean };
+
+const PinIcon = ({ isPinned }: PinIconProps) =>
+  isPinned ? <LockIcon color="primary" /> : <LockOpenIcon />;
+
 export const PinButton = () => {
   const { isPinned, toggleSidebarPinState } = useContext(
     SidebarPinStateContext,
   );
 
-  const PinIcon = () =>
-    isPinned ? <LockIcon color="primary" /> : <LockOpenIcon />;
-
   return (
     <ListItem>
       <ListItemText
@@ -54,7 +56,7 @@ export const PinButton = () => {
               toggleSidebarPinState();
             }}
           >
-            <PinIcon />
+            <PinIcon isPinned={isPinned} />
           </ToggleButton>
         </Tooltip>
       </ListItemSecondaryAction>
diff --git a/plugins/user-settings/src/components/ThemeToggle.tsx b/plugins/user-settings/src/components/ThemeToggle.tsx
index b041e1f76b..111bd838a8 100644
--- a/plugins/user-settings/src/components/ThemeToggle.tsx
+++ b/plugins/user-settings/src/components/ThemeToggle.tsx
@@ -17,7 +17,7 @@
 import React, { cloneElement } from 'react';
 import { useObservable } from 'react-use';
 import AutoIcon from '@material-ui/icons/BrightnessAuto';
-import { AppTheme, appThemeApiRef, useApi } from '@backstage/core';
+import { appThemeApiRef, useApi } from '@backstage/core';
 import ToggleButton from '@material-ui/lab/ToggleButton';
 import ToggleButtonGroup from '@material-ui/lab/ToggleButtonGroup';
 import {
@@ -27,6 +27,42 @@ import {
   Tooltip,
 } from '@material-ui/core';
 
+type ThemeIconProps = {
+  id: string;
+  activeId: string | undefined;
+  icon: JSX.Element | undefined;
+};
+
+const ThemeIcon = ({ id, activeId, icon }: ThemeIconProps) =>
+  icon ? (
+    cloneElement(icon, {
+      color: activeId === id ? 'primary' : undefined,
+    })
+  ) : (
+    <AutoIcon color={activeId === id ? 'primary' : undefined} />
+  );
+
+type TooltipToggleButtonProps = {
+  children: JSX.Element;
+  title: string;
+  value: string;
+};
+
+// ToggleButtonGroup uses React.children.map instead of context
+// so wrapping with Tooltip breaks ToggleButton functionality.
+const TooltipToggleButton = ({
+  children,
+  title,
+  value,
+  ...props
+}: TooltipToggleButtonProps) => (
+  <Tooltip placement="top" arrow title={title}>
+    <ToggleButton value={value} {...props}>
+      {children}
+    </ToggleButton>
+  </Tooltip>
+);
+
 export const ThemeToggle = () => {
   const appThemeApi = useApi(appThemeApiRef);
   const themeId = useObservable(
@@ -47,36 +83,6 @@ export const ThemeToggle = () => {
     }
   };
 
-  // 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;
-    return themeIcon ? (
-      cloneElement(themeIcon, {
-        color: themeId === theme.id ? 'primary' : undefined,
-      })
-    ) : (
-      <AutoIcon color={themeId === theme.id ? 'primary' : undefined} />
-    );
-  };
-
   return (
     <ListItem>
       <ListItemText primary="Theme" secondary="Change the theme mode" />
@@ -87,20 +93,24 @@ export const ThemeToggle = () => {
           value={themeId ?? 'auto'}
           onChange={handleSetTheme}
         >
-          {themeIds.map(theme => (
-            <TooltipToggleButton
-              key={theme.id}
-              title={`Select ${theme.title}`}
-              value={theme.variant}
-            >
-              <ThemeIcon theme={theme} />
-            </TooltipToggleButton>
-          ))}
-          <ToggleButton value="auto">
-            <Tooltip placement="top" arrow title="Select auto theme">
+          {themeIds.map(theme => {
+            const themeIcon = themeIds.find(t => t.id === theme.id)?.icon;
+
+            return (
+              <TooltipToggleButton
+                key={theme.id}
+                title={`Select ${theme.title}`}
+                value={theme.variant}
+              >
+                <ThemeIcon id={theme.id} icon={themeIcon} activeId={themeId} />
+              </TooltipToggleButton>
+            );
+          })}
+          <Tooltip placement="top" arrow title="Select auto theme">
+            <ToggleButton value="auto">
               <AutoIcon color={themeId === undefined ? 'primary' : undefined} />
-            </Tooltip>
-          </ToggleButton>
+            </ToggleButton>
+          </Tooltip>
         </ToggleButtonGroup>
       </ListItemSecondaryAction>
     </ListItem>