From c6ff101a99194be011225283c07088136543a25c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Sat, 17 Oct 2020 20:38:40 +0200
Subject: [PATCH 1/7] UX improvements to Settings

---
 .../components/AuthProviders/AuthProviders.tsx   |  2 +-
 .../AuthProviders/ProviderSettingsItem.tsx       | 16 +++++++---------
 .../src/components/FeatureFlags/FeatureFlags.tsx |  2 +-
 .../components/FeatureFlags/FeatureFlagsItem.tsx | 15 ++++++---------
 .../src/components/General/General.tsx           |  8 ++++----
 5 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/plugins/user-settings/src/components/AuthProviders/AuthProviders.tsx b/plugins/user-settings/src/components/AuthProviders/AuthProviders.tsx
index 563bf44150..bc2a68578d 100644
--- a/plugins/user-settings/src/components/AuthProviders/AuthProviders.tsx
+++ b/plugins/user-settings/src/components/AuthProviders/AuthProviders.tsx
@@ -37,7 +37,7 @@ export const AuthProviders = ({ providerSettings }: Props) => {
   }
 
   return (
-    <InfoCard>
+    <InfoCard title="Available Providers">
       <List dense>{providers}</List>
     </InfoCard>
   );
diff --git a/plugins/user-settings/src/components/AuthProviders/ProviderSettingsItem.tsx b/plugins/user-settings/src/components/AuthProviders/ProviderSettingsItem.tsx
index 7213ee2b83..4bae3dbfbb 100644
--- a/plugins/user-settings/src/components/AuthProviders/ProviderSettingsItem.tsx
+++ b/plugins/user-settings/src/components/AuthProviders/ProviderSettingsItem.tsx
@@ -22,14 +22,13 @@ import {
   SessionState,
 } from '@backstage/core';
 import {
+  Button,
   ListItem,
   ListItemIcon,
   ListItemSecondaryAction,
   ListItemText,
   Tooltip,
 } from '@material-ui/core';
-import PowerButton from '@material-ui/icons/PowerSettingsNew';
-import { ToggleButton } from '@material-ui/lab';
 
 type Props = {
   title: string;
@@ -84,14 +83,13 @@ export const ProviderSettingsItem = ({
           arrow
           title={signedIn ? `Sign out from ${title}` : `Sign in to ${title}`}
         >
-          <ToggleButton
-            size="small"
-            value={title}
-            selected={signedIn}
-            onChange={() => (signedIn ? api.signOut() : api.signIn())}
+          <Button
+            variant="outlined"
+            color="primary"
+            onClick={() => (signedIn ? api.signOut() : api.signIn())}
           >
-            <PowerButton color={signedIn ? 'primary' : undefined} />
-          </ToggleButton>
+            {signedIn ? `Sign out` : `Sign in`}
+          </Button>
         </Tooltip>
       </ListItemSecondaryAction>
     </ListItem>
diff --git a/plugins/user-settings/src/components/FeatureFlags/FeatureFlags.tsx b/plugins/user-settings/src/components/FeatureFlags/FeatureFlags.tsx
index 71f59c52b8..e0f4d6f40e 100644
--- a/plugins/user-settings/src/components/FeatureFlags/FeatureFlags.tsx
+++ b/plugins/user-settings/src/components/FeatureFlags/FeatureFlags.tsx
@@ -62,7 +62,7 @@ export const FeatureFlags = () => {
   }
 
   return (
-    <InfoCard>
+    <InfoCard title="Feature Flags">
       <List dense>
         {featureFlags.map(featureFlag => {
           const enabled = Boolean(state[featureFlag.name]);
diff --git a/plugins/user-settings/src/components/FeatureFlags/FeatureFlagsItem.tsx b/plugins/user-settings/src/components/FeatureFlags/FeatureFlagsItem.tsx
index 92065f2ed6..460a15f56b 100644
--- a/plugins/user-settings/src/components/FeatureFlags/FeatureFlagsItem.tsx
+++ b/plugins/user-settings/src/components/FeatureFlags/FeatureFlagsItem.tsx
@@ -19,10 +19,9 @@ import {
   ListItem,
   ListItemSecondaryAction,
   ListItemText,
+  Switch,
   Tooltip,
 } from '@material-ui/core';
-import CheckIcon from '@material-ui/icons/CheckCircle';
-import { ToggleButton } from '@material-ui/lab';
 import { FeatureFlagsRegistryItem } from '@backstage/core';
 
 type Props = {
@@ -39,14 +38,12 @@ export const FlagItem = ({ flag, enabled, toggleHandler }: Props) => (
     />
     <ListItemSecondaryAction>
       <Tooltip placement="top" arrow title={enabled ? 'Disable' : 'Enable'}>
-        <ToggleButton
-          size="small"
-          value="flag"
-          selected={enabled}
+        <Switch
+          color="primary"
+          checked={enabled}
           onChange={() => toggleHandler(flag.name)}
-        >
-          <CheckIcon color={enabled ? 'primary' : undefined} />
-        </ToggleButton>
+          name={flag.name}
+        />
       </Tooltip>
     </ListItemSecondaryAction>
   </ListItem>
diff --git a/plugins/user-settings/src/components/General/General.tsx b/plugins/user-settings/src/components/General/General.tsx
index 00ac17dfe1..424f352502 100644
--- a/plugins/user-settings/src/components/General/General.tsx
+++ b/plugins/user-settings/src/components/General/General.tsx
@@ -21,12 +21,12 @@ import { Profile } from './Profile';
 import { ThemeToggle } from './ThemeToggle';
 
 export const General = () => (
-  <Grid container spacing={3}>
-    <Grid item md={12}>
+  <Grid container direction="row" spacing={3}>
+    <Grid item sm={6}>
       <Profile />
     </Grid>
-    <Grid item md={12}>
-      <InfoCard>
+    <Grid item sm={6}>
+      <InfoCard title="Appearance">
         <List dense>
           <ThemeToggle />
           <PinButton />

From 7a2f8b04cd1e2bae5db4071f62cfa6172155f3f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Sat, 17 Oct 2020 21:19:53 +0200
Subject: [PATCH 2/7] Review comments

---
 .../src/components/General/General.tsx        |  4 +--
 .../src/components/General/PinButton.tsx      | 25 ++++++-------------
 .../src/components/General/ThemeToggle.tsx    | 12 +++++++--
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/plugins/user-settings/src/components/General/General.tsx b/plugins/user-settings/src/components/General/General.tsx
index 424f352502..b6d30d0103 100644
--- a/plugins/user-settings/src/components/General/General.tsx
+++ b/plugins/user-settings/src/components/General/General.tsx
@@ -22,10 +22,10 @@ import { ThemeToggle } from './ThemeToggle';
 
 export const General = () => (
   <Grid container direction="row" spacing={3}>
-    <Grid item sm={6}>
+    <Grid item sm={12} md={6}>
       <Profile />
     </Grid>
-    <Grid item sm={6}>
+    <Grid item sm={12} md={6}>
       <InfoCard title="Appearance">
         <List dense>
           <ThemeToggle />
diff --git a/plugins/user-settings/src/components/General/PinButton.tsx b/plugins/user-settings/src/components/General/PinButton.tsx
index e36fc365f8..181574fccf 100644
--- a/plugins/user-settings/src/components/General/PinButton.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.tsx
@@ -19,18 +19,11 @@ import {
   ListItem,
   ListItemSecondaryAction,
   ListItemText,
+  Switch,
   Tooltip,
 } from '@material-ui/core';
-import LockIcon from '@material-ui/icons/Lock';
-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,
@@ -48,16 +41,12 @@ export const PinButton = () => {
           arrow
           title={`${isPinned ? 'Unpin' : 'Pin'} Sidebar`}
         >
-          <ToggleButton
-            size="small"
-            value="pin"
-            selected={isPinned}
-            onChange={() => {
-              toggleSidebarPinState();
-            }}
-          >
-            <PinIcon isPinned={isPinned} />
-          </ToggleButton>
+          <Switch
+            color="primary"
+            checked={isPinned}
+            onChange={() => toggleSidebarPinState()}
+            name="pin"
+          />
         </Tooltip>
       </ListItemSecondaryAction>
     </ListItem>
diff --git a/plugins/user-settings/src/components/General/ThemeToggle.tsx b/plugins/user-settings/src/components/General/ThemeToggle.tsx
index 111bd838a8..dc24a61f8f 100644
--- a/plugins/user-settings/src/components/General/ThemeToggle.tsx
+++ b/plugins/user-settings/src/components/General/ThemeToggle.tsx
@@ -102,12 +102,20 @@ export const ThemeToggle = () => {
                 title={`Select ${theme.title}`}
                 value={theme.variant}
               >
-                <ThemeIcon id={theme.id} icon={themeIcon} activeId={themeId} />
+                <>
+                  {theme.variant}&nbsp;
+                  <ThemeIcon
+                    id={theme.id}
+                    icon={themeIcon}
+                    activeId={themeId}
+                  />
+                </>
               </TooltipToggleButton>
             );
           })}
           <Tooltip placement="top" arrow title="Select auto theme">
-            <ToggleButton value="auto">
+            <ToggleButton value="auto" selected={themeId === undefined}>
+              Auto&nbsp;
               <AutoIcon color={themeId === undefined ? 'primary' : undefined} />
             </ToggleButton>
           </Tooltip>

From d90a8109c276423c386fde4e35182042d4e8e043 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Sun, 18 Oct 2020 07:53:22 +0200
Subject: [PATCH 3/7] Fix test?

---
 .../user-settings/src/components/General/PinButton.test.tsx    | 3 +--
 plugins/user-settings/src/components/General/PinButton.tsx     | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/user-settings/src/components/General/PinButton.test.tsx b/plugins/user-settings/src/components/General/PinButton.test.tsx
index 76af8ce8d4..88f636d918 100644
--- a/plugins/user-settings/src/components/General/PinButton.test.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.test.tsx
@@ -32,10 +32,9 @@ describe('<PinButton />', () => {
         </SidebarPinStateContext.Provider>,
       ),
     );
-
     expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument();
 
-    const pinButton = rendered.getByTitle('Pin Sidebar');
+    const pinButton = rendered.getByTestId('pin');
     fireEvent.click(pinButton);
     expect(mockToggleFn).toHaveBeenCalled();
   });
diff --git a/plugins/user-settings/src/components/General/PinButton.tsx b/plugins/user-settings/src/components/General/PinButton.tsx
index 181574fccf..bfcdb0c7ff 100644
--- a/plugins/user-settings/src/components/General/PinButton.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.tsx
@@ -46,6 +46,7 @@ export const PinButton = () => {
             checked={isPinned}
             onChange={() => toggleSidebarPinState()}
             name="pin"
+            data-test-id="pin"
           />
         </Tooltip>
       </ListItemSecondaryAction>

From 763ec1cd2f9c62fd4a7fb62fe0f41c1dfaaac832 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Sun, 18 Oct 2020 08:20:06 +0200
Subject: [PATCH 4/7] Another try

---
 plugins/user-settings/src/components/General/PinButton.test.tsx | 2 +-
 plugins/user-settings/src/components/General/PinButton.tsx      | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/user-settings/src/components/General/PinButton.test.tsx b/plugins/user-settings/src/components/General/PinButton.test.tsx
index 88f636d918..d85fa3ddd9 100644
--- a/plugins/user-settings/src/components/General/PinButton.test.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.test.tsx
@@ -35,7 +35,7 @@ describe('<PinButton />', () => {
     expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument();
 
     const pinButton = rendered.getByTestId('pin');
-    fireEvent.click(pinButton);
+    fireEvent.change(pinButton);
     expect(mockToggleFn).toHaveBeenCalled();
   });
 });
diff --git a/plugins/user-settings/src/components/General/PinButton.tsx b/plugins/user-settings/src/components/General/PinButton.tsx
index bfcdb0c7ff..a812079133 100644
--- a/plugins/user-settings/src/components/General/PinButton.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.tsx
@@ -47,6 +47,7 @@ export const PinButton = () => {
             onChange={() => toggleSidebarPinState()}
             name="pin"
             data-test-id="pin"
+            inputProps={{ 'aria-label': 'Pin Sidebar Switch' }}
           />
         </Tooltip>
       </ListItemSecondaryAction>

From ec892d5fab3494d967d0934afbad9a988271b531 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Mon, 19 Oct 2020 10:36:03 +0200
Subject: [PATCH 5/7] Update PinButton.tsx

---
 plugins/user-settings/src/components/General/PinButton.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/user-settings/src/components/General/PinButton.tsx b/plugins/user-settings/src/components/General/PinButton.tsx
index a812079133..d1ac8b2043 100644
--- a/plugins/user-settings/src/components/General/PinButton.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.tsx
@@ -46,7 +46,7 @@ export const PinButton = () => {
             checked={isPinned}
             onChange={() => toggleSidebarPinState()}
             name="pin"
-            data-test-id="pin"
+            data-testid="pin"
             inputProps={{ 'aria-label': 'Pin Sidebar Switch' }}
           />
         </Tooltip>

From c8133f31aab4f78195eb6586008f85a096c692ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Mon, 19 Oct 2020 21:39:04 +0200
Subject: [PATCH 6/7] Update PinButton.test.tsx

---
 .../user-settings/src/components/General/PinButton.test.tsx | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/plugins/user-settings/src/components/General/PinButton.test.tsx b/plugins/user-settings/src/components/General/PinButton.test.tsx
index d85fa3ddd9..0db9d6ebf0 100644
--- a/plugins/user-settings/src/components/General/PinButton.test.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.test.tsx
@@ -34,8 +34,14 @@ describe('<PinButton />', () => {
     );
     expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument();
 
+    /*
     const pinButton = rendered.getByTestId('pin');
     fireEvent.change(pinButton);
+    */
+
+    const pinButton = rendered.getByLabelText('Pin Sidebar Switch');
+    fireEvent.click(pinButton);
+
     expect(mockToggleFn).toHaveBeenCalled();
   });
 });

From 4e81187c74e9924437bef460af9ad0d8245aab52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20=C3=85lund?= <alund@spotify.com>
Date: Mon, 19 Oct 2020 21:57:56 +0200
Subject: [PATCH 7/7] Cleanup

---
 .../user-settings/src/components/General/PinButton.test.tsx | 6 ------
 plugins/user-settings/src/components/General/PinButton.tsx  | 1 -
 2 files changed, 7 deletions(-)

diff --git a/plugins/user-settings/src/components/General/PinButton.test.tsx b/plugins/user-settings/src/components/General/PinButton.test.tsx
index 0db9d6ebf0..939588a9ee 100644
--- a/plugins/user-settings/src/components/General/PinButton.test.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.test.tsx
@@ -34,14 +34,8 @@ describe('<PinButton />', () => {
     );
     expect(rendered.getByText('Pin Sidebar')).toBeInTheDocument();
 
-    /*
-    const pinButton = rendered.getByTestId('pin');
-    fireEvent.change(pinButton);
-    */
-
     const pinButton = rendered.getByLabelText('Pin Sidebar Switch');
     fireEvent.click(pinButton);
-
     expect(mockToggleFn).toHaveBeenCalled();
   });
 });
diff --git a/plugins/user-settings/src/components/General/PinButton.tsx b/plugins/user-settings/src/components/General/PinButton.tsx
index d1ac8b2043..d44e5c9a3b 100644
--- a/plugins/user-settings/src/components/General/PinButton.tsx
+++ b/plugins/user-settings/src/components/General/PinButton.tsx
@@ -46,7 +46,6 @@ export const PinButton = () => {
             checked={isPinned}
             onChange={() => toggleSidebarPinState()}
             name="pin"
-            data-testid="pin"
             inputProps={{ 'aria-label': 'Pin Sidebar Switch' }}
           />
         </Tooltip>