From 14b62e6ce192e91871598973ae93ff7ad3eaf782 Mon Sep 17 00:00:00 2001
From: Tiffany Wong <7692354+tiffwong@users.noreply.github.com>
Date: Mon, 22 Jun 2020 11:10:33 -0400
Subject: [PATCH] [Tabs] M3-4119: Refactor Tabs - Lish (#6467)

* Refactor to Reach UI

* Add keyboardActivation prop

* Working on focusing on tabs

* Add ref to tabs

* removing redundant a11y attributes

* Switch to SafeTabPanel

* Move styles to Lish and get rid of ref

* Remove TabPanel import

Co-authored-by: Kayla Wilkins <kwilkins@linode.com>
---
 .../components/TabLinkList/TabLinkList.tsx    |  28 ++--
 packages/manager/src/features/Lish/Glish.tsx  |  70 ++++----
 packages/manager/src/features/Lish/Lish.tsx   | 155 +++++++-----------
 .../manager/src/features/Lish/Weblish.tsx     |   2 +-
 4 files changed, 107 insertions(+), 148 deletions(-)

diff --git a/packages/manager/src/components/TabLinkList/TabLinkList.tsx b/packages/manager/src/components/TabLinkList/TabLinkList.tsx
index e815b1a4e79..8e9a0f5fdd2 100644
--- a/packages/manager/src/components/TabLinkList/TabLinkList.tsx
+++ b/packages/manager/src/components/TabLinkList/TabLinkList.tsx
@@ -8,24 +8,24 @@ const useStyles = makeStyles((theme: Theme) => ({
   tab: {
     '&[data-reach-tab]': {
       // This was copied over from our MuiTab styling in themeFactory. Some of this could probably be cleaned up.
-      color: theme.color.tableHeaderText,
-      minWidth: 50,
-      textTransform: 'inherit',
-      fontSize: '0.93rem',
-      padding: '6px 16px',
-      position: 'relative',
-      overflow: 'hidden',
-      maxWidth: 264,
-      boxSizing: 'border-box',
-      borderBottom: '2px solid transparent',
-      minHeight: theme.spacing(1) * 6,
-      flexShrink: 0,
       display: 'inline-flex',
       alignItems: 'center',
+      flexShrink: 0,
       verticalAlign: 'middle',
       justifyContent: 'center',
       appearance: 'none',
+      borderBottom: '2px solid transparent',
+      boxSizing: 'border-box',
+      color: theme.color.tableHeaderText,
+      fontSize: '0.93rem',
       lineHeight: 1.3,
+      maxWidth: 264,
+      minHeight: theme.spacing(1) * 6,
+      minWidth: 50,
+      overflow: 'hidden',
+      padding: '6px 16px',
+      position: 'relative',
+      textTransform: 'inherit',
       [theme.breakpoints.up('md')]: {
         minWidth: 75
       },
@@ -40,6 +40,7 @@ const useStyles = makeStyles((theme: Theme) => ({
     }
   },
   tabList: {
+    color: theme.color.tableHeaderText,
     '&[data-reach-tab-list]': {
       background: 'none !important',
       boxShadow: `inset 0 -1px 0 ${theme.color.border2}`,
@@ -65,9 +66,8 @@ interface Props {
 type CombinedProps = Props;
 
 export const TabLinkList: React.FC<CombinedProps> = props => {
-  const { tabs } = props;
-
   const classes = useStyles();
+  const { tabs } = props;
 
   return (
     <TabList className={classes.tabList}>
diff --git a/packages/manager/src/features/Lish/Glish.tsx b/packages/manager/src/features/Lish/Glish.tsx
index 1907e442140..0bc464936f1 100644
--- a/packages/manager/src/features/Lish/Glish.tsx
+++ b/packages/manager/src/features/Lish/Glish.tsx
@@ -305,43 +305,41 @@ class Glish extends React.Component<CombinedProps, State> {
     }
 
     return (
-      <div id="tabpanel-glish" role="tabpanel" aria-labelledby="tab-glish">
-        <div id="Glish">
-          {!powered && (
-            <div className={classes.errorState}>
-              <ErrorState errorText="Please power on your Linode to use Glish" />
-            </div>
-          )}
-
-          {/*
-           * The loading states have to render with the VncDisplay component
-           * because the messages from the websocket connection have to be send
-           * if you're rendering a loading state, then get a message from websockets,
-           * then render the VncDisplay, you end up with a blank black screen
-           */}
-          {powered && !initialConnect ? (
-            isRetryingConnection ? (
-              this.renderRetryState()
-            ) : (
-              <CircleProgress noInner />
-            )
+      <div id="Glish">
+        {!powered && (
+          <div className={classes.errorState}>
+            <ErrorState errorText="Please power on your Linode to use Glish" />
+          </div>
+        )}
+
+        {/*
+         * The loading states have to render with the VncDisplay component
+         * because the messages from the websocket connection have to be send
+         * if you're rendering a loading state, then get a message from websockets,
+         * then render the VncDisplay, you end up with a blank black screen
+         */}
+        {powered && !initialConnect ? (
+          isRetryingConnection ? (
+            this.renderRetryState()
           ) : (
-            <React.Fragment />
-          )}
-
-          {powered && activeVnc && token && region && (
-            <div
-              className={classes.container}
-              style={!initialConnect ? { display: 'none' } : {}}
-            >
-              <VncDisplay
-                url={`${getLishSchemeAndHostname(region)}:8080/${token}`}
-                onUpdateState={this.onUpdateVNCState}
-                onResize={this.canvasResize}
-              />
-            </div>
-          )}
-        </div>
+            <CircleProgress noInner />
+          )
+        ) : (
+          <React.Fragment />
+        )}
+
+        {powered && activeVnc && token && region && (
+          <div
+            className={classes.container}
+            style={!initialConnect ? { display: 'none' } : {}}
+          >
+            <VncDisplay
+              url={`${getLishSchemeAndHostname(region)}:8080/${token}`}
+              onUpdateState={this.onUpdateVNCState}
+              onResize={this.canvasResize}
+            />
+          </div>
+        )}
       </div>
     );
   }
diff --git a/packages/manager/src/features/Lish/Lish.tsx b/packages/manager/src/features/Lish/Lish.tsx
index 545304fe4a5..4953ce3ee97 100644
--- a/packages/manager/src/features/Lish/Lish.tsx
+++ b/packages/manager/src/features/Lish/Lish.tsx
@@ -4,13 +4,7 @@ import {
   Linode
 } from '@linode/api-v4/lib/linodes';
 import * as React from 'react';
-import {
-  matchPath,
-  Route,
-  RouteComponentProps,
-  Switch,
-  withRouter
-} from 'react-router-dom';
+import { matchPath, RouteComponentProps, withRouter } from 'react-router-dom';
 import CircleProgress from 'src/components/CircleProgress';
 import {
   createStyles,
@@ -18,35 +12,45 @@ import {
   withStyles,
   WithStyles
 } from 'src/components/core/styles';
-import Tab from 'src/components/core/Tab';
-import Tabs from 'src/components/core/Tabs';
+import SafeTabPanel from 'src/components/SafeTabPanel';
+import TabPanels from 'src/components/core/ReachTabPanels';
+import Tabs from 'src/components/core/ReachTabs';
+import TabLinkList from 'src/components/TabLinkList';
 import Typography from 'src/components/core/Typography';
 import ErrorState from 'src/components/ErrorState';
 import NotFound from 'src/components/NotFound';
-import { convertForAria } from 'src/components/TabLink/TabLink';
 import Glish from './Glish';
 import Weblish from './Weblish';
 
-type ClassNames = 'tabs' | 'tabRoot' | 'progress' | 'notFound';
+type ClassNames = 'tabs' | 'progress' | 'notFound' | 'lish';
 
 const AUTH_POLLING_INTERVAL = 2000;
 
 const styles = (theme: Theme) =>
   createStyles({
     tabs: {
-      backgroundColor: theme.bg.offWhite,
-      margin: 0
-    },
-    tabRoot: {
+      backgroundColor: 'black',
       margin: 0,
-      flexBasis: '50%',
-      transition: theme.transitions.create('background-color'),
-      '&[aria-selected="true"]': {
-        backgroundColor: theme.palette.primary.main,
-        color: 'white',
-        '&:hover': {
-          backgroundColor: theme.palette.primary.light,
-          color: 'white'
+      '& [role="tablist"]': {
+        display: 'flex',
+        backgroundColor: theme.bg.offWhite,
+        margin: 0,
+        overflow: 'hidden'
+      },
+      '& [role="tab"]': {
+        backgroundColor: theme.bg.offWhite,
+        color: theme.color.tableHeaderText,
+        flexBasis: '50%',
+        margin: 0,
+        maxWidth: 'none !important',
+        '&[aria-selected="true"]': {
+          backgroundColor: theme.palette.primary.main,
+          borderBottom: 'none !important',
+          color: 'white !important',
+          '&:hover': {
+            backgroundColor: theme.palette.primary.light,
+            color: 'white'
+          }
         }
       }
     },
@@ -188,53 +192,23 @@ class Lish extends React.Component<CombinedProps, State> {
       });
   };
 
-  handleTabChange = (
-    event: React.ChangeEvent<HTMLDivElement>,
-    value: number
-  ) => {
-    const { history } = this.props;
-    const routeName = this.tabs[value].routeName;
-    history.push(`${routeName}`);
-  };
-
   tabs = [
     /* NB: These must correspond to the routes inside the Switch */
-    { routeName: `${this.props.match.url}/weblish`, title: 'Weblish' },
-    { routeName: `${this.props.match.url}/glish`, title: 'Glish' }
+    {
+      title: 'Weblish',
+      routeName: `${this.props.match.url}/weblish`
+    },
+    {
+      title: 'Glish',
+      routeName: `${this.props.match.url}/glish`
+    }
   ];
 
   matches = (p: string) =>
     Boolean(matchPath(p, { path: this.props.location.pathname }));
 
-  renderWeblish = () => {
-    const { linode, token } = this.state;
-    if (linode && token) {
-      return (
-        <Weblish
-          token={token}
-          linode={linode}
-          refreshToken={this.refreshToken}
-        />
-      );
-    }
-    return null;
-  };
-
-  renderGlish = () => {
-    const { linode, token } = this.state;
-    if (linode && token) {
-      return (
-        <Glish token={token} linode={linode} refreshToken={this.refreshToken} />
-      );
-    }
-    return null;
-  };
-
   render() {
-    const {
-      classes,
-      match: { path }
-    } = this.props;
+    const { classes } = this.props;
     const { authenticated, loading, linode, token } = this.state;
 
     // If the window.close() logic above fails, we render an error state as a fallback
@@ -251,49 +225,36 @@ class Lish extends React.Component<CombinedProps, State> {
       );
     }
 
-    const tabA11yProps = (idName: string) => {
-      const ariaVal = convertForAria(idName);
-
-      return {
-        id: `tab-${ariaVal}`,
-        role: 'tab',
-        'aria-controls': `tabpanel-${ariaVal}`
-      };
-    };
-
     return (
       <React.Fragment>
-        <Tabs
-          value={this.tabs.findIndex(tab => this.matches(tab.routeName))}
-          onChange={this.handleTabChange}
-          className={classes.tabs}
-          indicatorColor="primary"
-          textColor="primary"
-          scrollButtons="off"
-        >
-          {this.tabs.map(tab => (
-            <Tab
-              classes={{
-                root: classes.tabRoot
-              }}
-              key={tab.title}
-              label={tab.title}
-              data-qa-tab={tab.title}
-              {...tabA11yProps(tab.title)}
-            />
-          ))}
+        <Tabs className={classes.tabs}>
+          <TabLinkList className={classes.lish} tabs={this.tabs} />
+          <TabPanels>
+            {linode && token && (
+              <SafeTabPanel index={0} data-qa-tab="Weblish">
+                <Weblish
+                  token={token}
+                  linode={linode}
+                  refreshToken={this.refreshToken}
+                />
+              </SafeTabPanel>
+            )}
+            {linode && token && (
+              <SafeTabPanel index={1} data-qa-tab="Glish">
+                <Glish
+                  token={token}
+                  linode={linode}
+                  refreshToken={this.refreshToken}
+                />
+              </SafeTabPanel>
+            )}
+          </TabPanels>
         </Tabs>
         {loading && <CircleProgress noInner className={classes.progress} />}
         {/* Only show 404 component if we are missing _both_ linode and token */}
         {!loading && !linode && !token && (
           <NotFound className={classes.notFound} />
         )}
-        {!loading && token && linode && (
-          <Switch>
-            <Route exact path={`${path}/weblish`} render={this.renderWeblish} />
-            <Route exact path={`${path}/glish`} render={this.renderGlish} />
-          </Switch>
-        )}
       </React.Fragment>
     );
   }
diff --git a/packages/manager/src/features/Lish/Weblish.tsx b/packages/manager/src/features/Lish/Weblish.tsx
index 89c4b5d5048..c503c866210 100644
--- a/packages/manager/src/features/Lish/Weblish.tsx
+++ b/packages/manager/src/features/Lish/Weblish.tsx
@@ -246,7 +246,7 @@ export class Weblish extends React.Component<CombinedProps, State> {
      * then render the terminal div, you end up with a blank black screen
      */
     return (
-      <div id="tabpanel-weblish" role="tabpanel" aria-labelledby="tab-weblish">
+      <div>
         {this.socket && this.socket.readyState === this.socket.OPEN ? (
           <div id="terminal" className="terminal" />
         ) : !retryingConnection ? ( // basically are we switching tabs after the lish token expired?