From ad11abb382c75e811258c70f2660fcf90f307844 Mon Sep 17 00:00:00 2001
From: Usame Algan <5880855+usame-algan@users.noreply.github.com>
Date: Thu, 5 Jan 2023 11:25:38 +0100
Subject: [PATCH] fix: Keyboard support (#1454)

* fix: Keyboard support

* fix: Restore clickble card on dashboard

* fix: Dashboard e2e test
---
 cypress/e2e/smoke/dashboard.cy.js             |  2 +-
 .../balances/AssetsTable/styles.module.css    |  3 +-
 src/components/common/NavTabs/index.tsx       | 62 +++++++++++++------
 .../common/NetworkSelector/styles.module.css  |  4 --
 .../dashboard/Overview/Overview.tsx           |  8 +--
 src/components/nfts/NftCard/styles.module.css |  3 +-
 .../NotificationCenter/styles.module.css      |  2 +-
 .../safe-apps/SafeAppIcon/index.tsx           |  1 +
 .../sidebar/SidebarHeader/index.tsx           | 34 +++++-----
 .../transactions/TxDetails/Summary/index.tsx  | 32 +++++-----
 .../TxDetails/Summary/styles.module.css       |  9 ---
 11 files changed, 85 insertions(+), 75 deletions(-)

diff --git a/cypress/e2e/smoke/dashboard.cy.js b/cypress/e2e/smoke/dashboard.cy.js
index 45e7dad44f..f5513a2652 100644
--- a/cypress/e2e/smoke/dashboard.cy.js
+++ b/cypress/e2e/smoke/dashboard.cy.js
@@ -18,7 +18,7 @@ describe('Dashboard', () => {
       // Prefix is separated across elements in EthHashInfo
       cy.contains('0xCD4FddB8FfA90012DFE11eD4bf258861204FeEAE').should('exist')
       cy.contains('1/1')
-      cy.get(`a[href="/balances?safe=${encodeURIComponent(SAFE)}"] button`).contains('View assets')
+      cy.get(`a[href="/balances?safe=${encodeURIComponent(SAFE)}"]`).contains('View assets')
       cy.contains('p', 'Tokens').next().contains('3')
       cy.contains('p', 'NFTs').next().contains('0')
     })
diff --git a/src/components/balances/AssetsTable/styles.module.css b/src/components/balances/AssetsTable/styles.module.css
index 74146ec7f8..b9b5882cd5 100644
--- a/src/components/balances/AssetsTable/styles.module.css
+++ b/src/components/balances/AssetsTable/styles.module.css
@@ -3,7 +3,8 @@
   transition: opacity 0.2s;
 }
 
-.container tr:hover td:last-of-type button {
+.container tr:hover td:last-of-type button,
+.container td:last-of-type button:focus-visible {
   opacity: 1;
 }
 
diff --git a/src/components/common/NavTabs/index.tsx b/src/components/common/NavTabs/index.tsx
index cfdf0cfbbd..cb37d36613 100644
--- a/src/components/common/NavTabs/index.tsx
+++ b/src/components/common/NavTabs/index.tsx
@@ -1,10 +1,35 @@
-import Link from 'next/link'
-import { Tab, Tabs, Typography } from '@mui/material'
+import React, { forwardRef } from 'react'
+import NextLink, { type LinkProps as NextLinkProps } from 'next/link'
+import { Tab, Tabs, Typography, type TabProps } from '@mui/material'
 import { useRouter } from 'next/router'
 import type { NavItem } from '@/components/sidebar/SidebarNavigation/config'
-
 import css from './styles.module.css'
 
+type Props = TabProps & NextLinkProps
+
+// This is needed in order for the tabs to work properly with Next Link e.g. tabbing with the keyboard
+// Based on https://github.com/mui/material-ui/blob/master/examples/nextjs-with-typescript/src/Link.tsx
+const NextLinkComposed = forwardRef<HTMLAnchorElement, Props>(function NextComposedLink(props: Props, ref) {
+  const { href, as, replace, scroll, shallow, prefetch, legacyBehavior = true, locale, ...other } = props
+
+  return (
+    <NextLink
+      href={href}
+      prefetch={prefetch}
+      as={as}
+      replace={replace}
+      scroll={scroll}
+      shallow={shallow}
+      passHref
+      locale={locale}
+      legacyBehavior={legacyBehavior}
+    >
+      {/* @ts-ignore */}
+      <a ref={ref} {...other} />
+    </NextLink>
+  )
+})
+
 const NavTabs = ({ tabs }: { tabs: NavItem[] }) => {
   const router = useRouter()
   const activeTab = tabs.map((tab) => tab.href).indexOf(router.pathname)
@@ -13,21 +38,22 @@ const NavTabs = ({ tabs }: { tabs: NavItem[] }) => {
     <Tabs value={activeTab} variant="scrollable" allowScrollButtonsMobile className={css.tabs}>
       {tabs.map((tab, idx) => {
         return (
-          <Link key={tab.href} href={{ pathname: tab.href, query: { safe: router.query.safe } }} passHref>
-            <Tab
-              className={css.tab}
-              label={
-                <Typography
-                  variant="body2"
-                  fontWeight={700}
-                  color={activeTab === idx ? 'primary' : 'primary.light'}
-                  className={css.label}
-                >
-                  {tab.label}
-                </Typography>
-              }
-            />
-          </Link>
+          <Tab
+            component={NextLinkComposed}
+            key={tab.href}
+            href={{ pathname: tab.href, query: { safe: router.query.safe } }}
+            className={css.tab}
+            label={
+              <Typography
+                variant="body2"
+                fontWeight={700}
+                color={activeTab === idx ? 'primary' : 'primary.light'}
+                className={css.label}
+              >
+                {tab.label}
+              </Typography>
+            }
+          />
         )
       })}
     </Tabs>
diff --git a/src/components/common/NetworkSelector/styles.module.css b/src/components/common/NetworkSelector/styles.module.css
index 55c2fdf198..76f06608f7 100644
--- a/src/components/common/NetworkSelector/styles.module.css
+++ b/src/components/common/NetworkSelector/styles.module.css
@@ -7,10 +7,6 @@
   display: none;
 }
 
-.select *:focus {
-  background: inherit;
-}
-
 .select *:focus-visible {
   outline: 5px auto Highlight;
   outline: 5px auto -webkit-focus-ring-color;
diff --git a/src/components/dashboard/Overview/Overview.tsx b/src/components/dashboard/Overview/Overview.tsx
index c28e3ac31a..5087b8f015 100644
--- a/src/components/dashboard/Overview/Overview.tsx
+++ b/src/components/dashboard/Overview/Overview.tsx
@@ -153,11 +153,9 @@ const Overview = (): ReactElement => {
               <Grid item>
                 <Box display="flex" height={1} alignItems="flex-end" justifyContent="flex-end">
                   <Link href={assetsLink} passHref>
-                    <a>
-                      <Button size="medium" variant="contained" color="primary">
-                        View assets
-                      </Button>
-                    </a>
+                    <Button size="medium" variant="contained" color="primary">
+                      View assets
+                    </Button>
                   </Link>
                 </Box>
               </Grid>
diff --git a/src/components/nfts/NftCard/styles.module.css b/src/components/nfts/NftCard/styles.module.css
index 114846ae8f..c14c438f54 100644
--- a/src/components/nfts/NftCard/styles.module.css
+++ b/src/components/nfts/NftCard/styles.module.css
@@ -13,7 +13,8 @@
   pointer-events: none;
 }
 
-.card:hover .sendButton {
+.card:hover .sendButton,
+.sendButton:focus-visible {
   pointer-events: all;
   opacity: 1;
 }
diff --git a/src/components/notification-center/NotificationCenter/styles.module.css b/src/components/notification-center/NotificationCenter/styles.module.css
index 989e46be85..82d3f95627 100644
--- a/src/components/notification-center/NotificationCenter/styles.module.css
+++ b/src/components/notification-center/NotificationCenter/styles.module.css
@@ -1,7 +1,7 @@
 .bell {
   display: flex;
   justify-content: center;
-  height: 100%;
+  padding: 4px;
 }
 
 .bell svg path {
diff --git a/src/components/safe-apps/SafeAppIcon/index.tsx b/src/components/safe-apps/SafeAppIcon/index.tsx
index 45f1577cd4..9b470e769f 100644
--- a/src/components/safe-apps/SafeAppIcon/index.tsx
+++ b/src/components/safe-apps/SafeAppIcon/index.tsx
@@ -35,6 +35,7 @@ const SafeAppIcon = ({
       width={width}
       height={height}
       style={{ pointerEvents: 'none' }}
+      tabIndex={-1}
     />
   )
 }
diff --git a/src/components/sidebar/SidebarHeader/index.tsx b/src/components/sidebar/SidebarHeader/index.tsx
index f15c3179f8..a84952d881 100644
--- a/src/components/sidebar/SidebarHeader/index.tsx
+++ b/src/components/sidebar/SidebarHeader/index.tsx
@@ -1,7 +1,6 @@
 import type { ReactElement } from 'react'
 import { useEffect, useState } from 'react'
 import Typography from '@mui/material/Typography'
-import type { IconButtonProps } from '@mui/material/IconButton'
 import IconButton from '@mui/material/IconButton'
 import Skeleton from '@mui/material/Skeleton'
 import Tooltip from '@mui/material/Tooltip'
@@ -29,18 +28,6 @@ import Track from '@/components/common/Track'
 import { OVERVIEW_EVENTS } from '@/services/analytics/events/overview'
 import { SvgIcon } from '@mui/material'
 
-const HeaderIconButton = ({
-  title,
-  children,
-  ...props
-}: { title: string } & Omit<IconButtonProps, 'className' | 'disableRipple' | 'sx'>) => (
-  <Tooltip title={title} placement="top">
-    <IconButton className={css.iconButton} {...props}>
-      {children}
-    </IconButton>
-  </Tooltip>
-)
-
 const SafeHeader = (): ReactElement => {
   const currency = useAppSelector(selectCurrency)
   const { balances, loading: balancesLoading } = useBalances()
@@ -88,9 +75,11 @@ const SafeHeader = (): ReactElement => {
         <div className={css.iconButtons}>
           <Track {...OVERVIEW_EVENTS.SHOW_QR}>
             <QrCodeButton>
-              <HeaderIconButton title="Open QR code">
-                <SvgIcon component={QrIconBold} inheritViewBox color="primary" fontSize="small" />
-              </HeaderIconButton>
+              <Tooltip title="Open QR code" placement="top">
+                <IconButton className={css.iconButton}>
+                  <SvgIcon component={QrIconBold} inheritViewBox color="primary" fontSize="small" />
+                </IconButton>
+              </Tooltip>
             </QrCodeButton>
           </Track>
 
@@ -101,11 +90,16 @@ const SafeHeader = (): ReactElement => {
           </Track>
 
           <Track {...OVERVIEW_EVENTS.OPEN_EXPLORER}>
-            <a target="_blank" rel="noreferrer" href={blockExplorerLink?.href || ''}>
-              <HeaderIconButton title={blockExplorerLink?.title || ''}>
+            <Tooltip title={blockExplorerLink?.title || ''} placement="top">
+              <IconButton
+                className={css.iconButton}
+                target="_blank"
+                rel="noreferrer"
+                href={blockExplorerLink?.href || ''}
+              >
                 <SvgIcon component={LinkIconBold} inheritViewBox fontSize="small" color="primary" />
-              </HeaderIconButton>
-            </a>
+              </IconButton>
+            </Tooltip>
           </Track>
         </div>
       </div>
diff --git a/src/components/transactions/TxDetails/Summary/index.tsx b/src/components/transactions/TxDetails/Summary/index.tsx
index 1efd0f15f7..ff39c72872 100644
--- a/src/components/transactions/TxDetails/Summary/index.tsx
+++ b/src/components/transactions/TxDetails/Summary/index.tsx
@@ -44,22 +44,24 @@ const Summary = ({ txDetails, defaultExpanded = false }: Props): ReactElement =>
             </Link>
           )}
 
-          <div className={`${css.collapsibleSection}${expanded ? 'Expanded' : ''}`}>
-            <TxDataRow title="Operation:">
-              {`${txData.operation} (${Operation[txData.operation].toLowerCase()})`}
-            </TxDataRow>
-            <TxDataRow title="safeTxGas:">{safeTxGas}</TxDataRow>
-            <TxDataRow title="baseGas:">{baseGas}</TxDataRow>
-            <TxDataRow title="gasPrice:">{gasPrice}</TxDataRow>
-            <TxDataRow title="gasToken:">{generateDataRowValue(gasToken, 'hash', true)}</TxDataRow>
-            <TxDataRow title="refundReceiver:">{generateDataRowValue(refundReceiver, 'hash', true)}</TxDataRow>
-            {confirmations?.map(({ signature }, index) => (
-              <TxDataRow title={`Signature ${index + 1}:`} key={`signature-${index}:`}>
-                {generateDataRowValue(signature, 'rawData')}
+          {expanded && (
+            <div>
+              <TxDataRow title="Operation:">
+                {`${txData.operation} (${Operation[txData.operation].toLowerCase()})`}
               </TxDataRow>
-            ))}
-            <TxDataRow title="Raw data:">{generateDataRowValue(txData.hexData, 'rawData')}</TxDataRow>
-          </div>
+              <TxDataRow title="safeTxGas:">{safeTxGas}</TxDataRow>
+              <TxDataRow title="baseGas:">{baseGas}</TxDataRow>
+              <TxDataRow title="gasPrice:">{gasPrice}</TxDataRow>
+              <TxDataRow title="gasToken:">{generateDataRowValue(gasToken, 'hash', true)}</TxDataRow>
+              <TxDataRow title="refundReceiver:">{generateDataRowValue(refundReceiver, 'hash', true)}</TxDataRow>
+              {confirmations?.map(({ signature }, index) => (
+                <TxDataRow title={`Signature ${index + 1}:`} key={`signature-${index}:`}>
+                  {generateDataRowValue(signature, 'rawData')}
+                </TxDataRow>
+              ))}
+              <TxDataRow title="Raw data:">{generateDataRowValue(txData.hexData, 'rawData')}</TxDataRow>
+            </div>
+          )}
         </>
       )}
     </>
diff --git a/src/components/transactions/TxDetails/Summary/styles.module.css b/src/components/transactions/TxDetails/Summary/styles.module.css
index b52f448846..f6dd3cc651 100644
--- a/src/components/transactions/TxDetails/Summary/styles.module.css
+++ b/src/components/transactions/TxDetails/Summary/styles.module.css
@@ -2,12 +2,3 @@
   margin-top: 8px;
   padding: 0;
 }
-
-.collapsibleSection {
-  max-height: 0px;
-  overflow: hidden;
-}
-
-.collapsibleSectionExpanded {
-  max-height: '500px';
-}