Console error on settings page in teams

[zwass]

Fleet version (Head to the "My account" page in the Fleet UI or run fleetctl version): teams
Operating system (e.g. macOS 11.2.3): Any
Web browser (e.g. Chrome 88.0.4324): Any

---

🧑‍💻  Expected behavior

Navigate to https://localhost:8080/settings/organization. No warnings appear in the console.

💥  Actual behavior

Following error in the console:

Warning: Failed prop type: There should be an equal number of 'Tab' and 'TabPanel' in `Tabs`. Received 3 'Tab' and 0 'TabPanel'.
    in Tabs (created by SettingsWrapper)
    in SettingsWrapper (created by RouterContext)
    in AuthenticatedAdminRoutes (created by ConnectFunction)
    in ConnectFunction (created by RouterContext)
    in div (created by CoreLayout)
    in div (created by CoreLayout)
    in CoreLayout (created by ConnectFunction)
    in ConnectFunction (created by RouterContext)
    in div (created by AuthenticatedRoutes)
    in AuthenticatedRoutes (created by ConnectFunction)
    in ConnectFunction (created by RouterContext)
    in div (created by App)
    in App (created by ConnectFunction)
    in ConnectFunction (created by RouterContext)
    in RouterContext (created by Router)
    in Router
    in Provider

More info

@martavis assigning this to you as another likely easy fix that gets you working in the codebase. Please lmk if you have any questions!

[RachelElysia]

This is a react-tabs issue. It's on stackoverflow that someone submitted a fix to react-tabs:
reactjs/react-tabs#392

Basically, react-tabs expects the TabPanel to render but really we just have it rerouting through react-router

[zwass]

Looks like the next step is to update react-tabs when the fix is merged.

[zwass]

A fix to reactjs/react-tabs#329 would also help resolve this.

[joepvl]

React-tabs collaborator here. I'm genuinely so confused by this. What's the point of using react-tabs if you're not going to use the panels? What's the advantage over simply having a list of buttons? In SettingsWrapper you have the following code:

<Tabs
  selectedIndex={getTabIndex(pathname)}
  onSelect={(i) => navigateToNav(i)}
>
  <TabList>
    {settingsSubNav.map((navItem) => {
      // Bolding text when the tab is active causes a layout shift
      // so we add a hidden pseudo element with the same text string
      return (
        <Tab key={navItem.name} data-text={navItem.name}>
          {navItem.name}
        </Tab>
      );
    })}
  </TabList>
</Tabs>

Which seems like a clear misuse of the component. Why not simplify that to something like

<ul className="NavItems">
  {settingsSubNav.map((navItem, index) => (
    <li className={cx("NavItems-item", { "is-active": index === getTabIndex(pathname) })}>
      <button onClick={navigateToNav}>{navItem.name}</button>
    </li>
  ))}
</ul>

and get rid of the dependency for this SettingsWrapper module altogether? Change the semantics as you see fit of course — I don't know this particular code base.

[martavis]

@joepvl Thank you for bringing this up. This is basically an oversight in our tech debt as we were getting to know the library in the early days of our code. We have other instances of React Tabs that are using it correctly, but we need to clean this up using your idea.