Make tabs components better (#691)

* Make better tabs components, apply to user page * Remove fishy unused href property from tabs * Remove tab ID property * Clean up crufty markup in tabs component * Fix naming to be right (thanks James!)
2022-07-25 13:27:09 -07:00 · 2022-07-25 13:27:09 -07:00 · 64462d6ab4
commit 64462d6ab4
parent e4f8c14fab
3 changed files with 102 additions and 82 deletions
--- a/web/components/layout/tabs.tsx
+++ b/web/components/layout/tabs.tsx
@ -1,82 +1,121 @@
 import clsx from 'clsx'
-import Link from 'next/link'
+import { useRouter, NextRouter } from 'next/router'
 import { ReactNode, useState } from 'react'
-import { Row } from './row'
 import { track } from '@amplitude/analytics-browser'

 type Tab = {
  title: string
  tabIcon?: ReactNode
  content: ReactNode
-  // If set, change the url to this href when the tab is selected
-  href?: string
  // If set, show a badge with this content
  badge?: string
 }

-export function Tabs(props: {
+type TabProps = {
  tabs: Tab[]
-  defaultIndex?: number
  labelClassName?: string
  onClick?: (tabTitle: string, index: number) => void
  className?: string
  currentPageForAnalytics?: string
-}) {
+}
+
+export function ControlledTabs(props: TabProps & { activeIndex: number }) {
  const {
    tabs,
-    defaultIndex,
+    activeIndex,
    labelClassName,
    onClick,
    className,
    currentPageForAnalytics,
  } = props
-  const [activeIndex, setActiveIndex] = useState(defaultIndex ?? 0)
  const activeTab = tabs[activeIndex] as Tab | undefined // can be undefined in weird case
-
  return (
    <>
-      <div className={clsx('mb-4 border-b border-gray-200', className)}>
-        <nav className="-mb-px flex space-x-8" aria-label="Tabs">
-          {tabs.map((tab, i) => (
-            <Link href={tab.href ?? '#'} key={tab.title} shallow={!!tab.href}>
-              <a
-                id={`tab-${i}`}
-                key={tab.title}
-                onClick={(e) => {
-                  track('Clicked Tab', {
-                    title: tab.title,
-                    href: tab.href,
-                    currentPage: currentPageForAnalytics,
-                  })
-                  if (!tab.href) {
-                    e.preventDefault()
-                  }
-                  setActiveIndex(i)
-                  onClick?.(tab.title, i)
-                }}
-                className={clsx(
-                  activeIndex === i
-                    ? 'border-indigo-500 text-indigo-600'
-                    : 'border-transparent text-gray-500 hover:border-gray-300 hover:text-gray-700',
-                  'cursor-pointer whitespace-nowrap border-b-2 py-3 px-1 text-sm font-medium',
-                  labelClassName
-                )}
-                aria-current={activeIndex === i ? 'page' : undefined}
-              >
-                <Row className={'items-center justify-center gap-1'}>
-                  {tab.tabIcon && <span> {tab.tabIcon}</span>}
-                  {tab.badge ? (
-                    <div className="px-0.5 font-bold">{tab.badge}</div>
-                  ) : null}
-                  {tab.title}
-                </Row>
-              </a>
-            </Link>
-          ))}
-        </nav>
-      </div>
-
+      <nav
+        className={clsx('mb-4 space-x-8 border-b border-gray-200', className)}
+        aria-label="Tabs"
+      >
+        {tabs.map((tab, i) => (
+          <a
+            href="#"
+            key={tab.title}
+            onClick={(e) => {
+              e.preventDefault()
+              track('Clicked Tab', {
+                title: tab.title,
+                currentPage: currentPageForAnalytics,
+              })
+              onClick?.(tab.title, i)
+            }}
+            className={clsx(
+              activeIndex === i
+                ? 'border-indigo-500 text-indigo-600'
+                : 'border-transparent text-gray-500 hover:border-gray-300 hover:text-gray-700',
+              'inline-flex cursor-pointer flex-row gap-1 whitespace-nowrap border-b-2 px-1 py-3 text-sm font-medium',
+              labelClassName
+            )}
+            aria-current={activeIndex === i ? 'page' : undefined}
+          >
+            {tab.tabIcon && <span>{tab.tabIcon}</span>}
+            {tab.badge ? (
+              <span className="px-0.5 font-bold">{tab.badge}</span>
+            ) : null}
+            {tab.title}
+          </a>
+        ))}
+      </nav>
      {activeTab?.content}
    </>
  )
 }
+
+export function UncontrolledTabs(props: TabProps & { defaultIndex?: number }) {
+  const { defaultIndex, onClick, ...rest } = props
+  const [activeIndex, setActiveIndex] = useState(defaultIndex ?? 0)
+  return (
+    <ControlledTabs
+      {...rest}
+      activeIndex={activeIndex}
+      onClick={(title, i) => {
+        setActiveIndex(i)
+        onClick?.(title, i)
+      }}
+    />
+  )
+}
+
+const isTabSelected = (router: NextRouter, queryParam: string, tab: Tab) => {
+  const selected = router.query[queryParam]
+  if (typeof selected === 'string') {
+    return tab.title.toLowerCase() === selected
+  } else {
+    return false
+  }
+}
+
+export function QueryUncontrolledTabs(
+  props: TabProps & { defaultIndex?: number }
+) {
+  const { tabs, defaultIndex, onClick, ...rest } = props
+  const router = useRouter()
+  const selectedIdx = tabs.findIndex((t) => isTabSelected(router, 'tab', t))
+  const activeIndex = selectedIdx !== -1 ? selectedIdx : defaultIndex ?? 0
+  return (
+    <ControlledTabs
+      {...rest}
+      tabs={tabs}
+      activeIndex={activeIndex}
+      onClick={(title, i) => {
+        router.replace(
+          { query: { ...router.query, tab: title.toLowerCase() } },
+          undefined,
+          { shallow: true }
+        )
+        onClick?.(title, i)
+      }}
+    />
+  )
+}
+
+// legacy code that didn't know about any other kind of tabs imports this
+export const Tabs = UncontrolledTabs
--- a/web/components/user-page.tsx
+++ b/web/components/user-page.tsx
@ -22,7 +22,7 @@ import { Linkify } from './linkify'
 import { Spacer } from './layout/spacer'
 import { Row } from './layout/row'
 import { genHash } from 'common/util/random'
-import { Tabs } from './layout/tabs'
+import { QueryUncontrolledTabs } from './layout/tabs'
 import { UserCommentsList } from './comments-list'
 import { useWindowSize } from 'web/hooks/use-window-size'
 import { Comment, getUsersComments } from 'web/lib/firebase/comments'
@ -64,12 +64,8 @@ export function UserLink(props: {
 export const TAB_IDS = ['markets', 'comments', 'bets', 'groups']
 const JUNE_1_2022 = new Date('2022-06-01T00:00:00.000Z').valueOf()

-export function UserPage(props: {
-  user: User
-  currentUser?: User
-  defaultTabTitle?: string | undefined
-}) {
-  const { user, currentUser, defaultTabTitle } = props
+export function UserPage(props: { user: User; currentUser?: User }) {
+  const { user, currentUser } = props
  const router = useRouter()
  const isCurrentUser = user.id === currentUser?.id
  const bannerUrl = user.bannerUrl ?? defaultBannerUrl(user.id)
@ -276,29 +272,17 @@ export function UserPage(props: {
        <Spacer h={10} />

        {usersContracts !== 'loading' && contractsById && usersComments ? (
-          <Tabs
+          <QueryUncontrolledTabs
            currentPageForAnalytics={'profile'}
            labelClassName={'pb-2 pt-1 '}
-            defaultIndex={
-              defaultTabTitle ? TAB_IDS.indexOf(defaultTabTitle) : 0
-            }
-            onClick={(tabName) => {
-              const tabId = tabName.toLowerCase()
-              const subpath = tabId === 'markets' ? '' : '?tab=' + tabId
-              // BUG: if you start on `/Bob/bets`, then click on Markets, use-query-and-sort-params
-              // rewrites the url incorrectly to `/Bob/bets` instead of `/Bob`
-              router.push(`/${user.username}${subpath}`, undefined, {
-                shallow: true,
-              })
-            }}
            tabs={[
              {
                title: 'Markets',
                content: <CreatorContractsList creator={user} />,
                tabIcon: (
-                  <div className="px-0.5 font-bold">
+                  <span className="px-0.5 font-bold">
                    {usersContracts.length}
-                  </div>
+                  </span>
                ),
              },
              {
@ -311,7 +295,9 @@ export function UserPage(props: {
                  />
                ),
                tabIcon: (
-                  <div className="px-0.5 font-bold">{usersComments.length}</div>
+                  <span className="px-0.5 font-bold">
+                    {usersComments.length}
+                  </span>
                ),
              },
              {
@ -329,7 +315,7 @@ export function UserPage(props: {
                    />
                  </div>
                ),
-                tabIcon: <div className="px-0.5 font-bold">{betCount}</div>,
+                tabIcon: <span className="px-0.5 font-bold">{betCount}</span>,
              },
            ]}
          />
--- a/web/pages/[username]/index.tsx
+++ b/web/pages/[username]/index.tsx
@ -31,9 +31,8 @@ export default function UserProfile(props: { user: User | null }) {
  const { user } = props

  const router = useRouter()
-  const { username, tab } = router.query as {
+  const { username } = router.query as {
    username: string
-    tab?: string | undefined
  }
  const currentUser = useUser()

@ -42,11 +41,7 @@ export default function UserProfile(props: { user: User | null }) {
  if (user === undefined) return <div />

  return user ? (
-    <UserPage
-      user={user}
-      currentUser={currentUser || undefined}
-      defaultTabTitle={tab}
-    />
+    <UserPage user={user} currentUser={currentUser || undefined} />
  ) : (
    <Custom404 />
  )