Performance issue: inactive tabs should not be rendered

[bradenmacdonald]

Feedback summary

As far as I can tell, Paragon currently processes and renders the entire React component tree for all tabs in a <Tabs> component. This can cause poor performance when tabs are used for major parts of the application, not only because of unnecessary rendering but can also cause unnecessary data loading and API calls if the inactive tabs have those.

Here is a simple example showing the issue. No messages should be logged to the console, but two messages are logged from the contents of the inactive tab:

Improved API

Generally, the way to fix this is to make each <Tab> accept the child component as a prop, and not as JSX children. Then it can be conditionally rendered.

i.e. <Tab><Child /></Tab> becomes <Tab component={Child} />

Simple Workaround

Alternately, the docs could be updated to say that <Tabs> should only be used for "minor" parts of an application, like two different views within a modal, and that larger chunks of the application should strictly use <Nav variant="tabs"> (which is available in Paragon) together with a router to control the "tab pane" content. At the moment, we don't seem to document nor encourage this pattern. The only openedx example I could find of using <Nav variant="tabs'> is not using a client-side router.

So, I don't have a working Paragon example handy (even though I think it's possible), but here is a working example from another application I made, using Catalyst Navbar as the Tabs component and wouter as the routing framework. In this example, only the active tab is rendered.

[adamstankiewicz]

This can cause poor performance when tabs are used for major parts of the application, not only because of unnecessary rendering but can also cause unnecessary data loading and API calls if the inactive tabs have those.

While I agree there can be performance implications where each tab's children are rendered, regardless of whether it's the active tab or not, it may be desired on a case-by-case basis. For example, perhaps a consumer wants to render an inactive tab to "preload" its data before the user actually clicks on the tab to make it active in certain circumstances, e.g. to prevent loading states when clicking into the tab. I'm not sure we should assume all consumers want to conditionally render the tabs' children based on the active tab.

For what it's worth, this behavior is largely attributed to extending react-bootstrap; the Paragon component itself does not control when the tab contents are shown currently. Typically, consumers have gotten around these concerns by making the children of each tab only render when active as needed (the conditional rendering you're proposing), e.g.

() => {
  const [key, setKey] = useState('home');

  return (
    <Tabs
      id="controlled-tab-example"
      activeKey={key}
      onSelect={(k) => setKey(k)}
    >
      <Tab eventKey="home" title="Home">
        {key === 'home' && (
          <>
            Hello I am the first panel.
            {console.log('first panel')}
          </>
        )}
      </Tab>
      <Tab eventKey="profile" title="Profile">
        {key === 'profile' && (
          <>
            Hello I am the second panel.
            {console.log('second panel')}
          </>
        )}
      </Tab>
      <Tab eventKey="contact" title="Contact" disabled>
        {key === 'contact' && (
          <>
            Hello I am the third panel.
            {console.log('third panel')}
          </>
        )}
      </Tab>
    </Tabs>
  );
}

That said, I agree there's some risk this approach is not implemented in cases where it really should be. I wonder if we'd be able to implement something like the proposed conditional rendering (e.g., based on an opt-in prop like renderActiveTabOnly) given the existing behavior comes from react-bootstrap's internals, not the Paragon component itself. We might need to opt out from using the parent Tabs from react-bootstrap in order to modify the rendering behavior of the tab contents (source), if we wanted to pursue this conditional rendering behavior.

Alternately, the docs could be updated to say that should only be used for "minor" parts of an application, like two different views within a modal, and that larger chunks of the application should strictly use <Nav variant="tabs"> (which is available in Paragon) together with a router to control the "tab pane" content. At the moment, we don't seem to document nor encourage this pattern.

I'm not sure <Nav variant="tabs"> should be encouraged for tabs navigation as it doesn't have the same HTML semantics nor a11y attributes as Tabs, e.g.:

• The Tabs component relies on <Nav role="tablist" as="nav"> (source) whereas seemingly no nav element nor role="tablist" a11y attribute are used with <Nav variant="tabs">
• The Tabs component includes role="tab", aria-controls, aria-hidden, and aria-selected on its provided tabs, whereas <Nav variant="tabs"> does not, which feels like it'd introduce a11y issues or otherwise leave a11y up to the consumer to implement.

See below Paragon Playground example of the resulting DOM structure for <Nav variant="tabs"> by default:

Regarding the interaction with a router, though, I definitely 100% agree Tabs should be coupled with routes, where relevant, to make it straightforward for users to deep link / bookmark specific tabs, improving the UX. The docs site could probably expand its documentation to make this pattern more strongly suggested, though given Paragon has no knowledge of consumers' routers, it likely should be exemplary only.

As for prior art, there are a few instances where we integrate React Router with Tabs from Paragon to have a route-per-tab implementation, e.g. within frontend-app-admin-portal (an Enterprise MFE):

• example 1: includes conditional rendering of children based on the active tab.
• example 2 (see handleTabSelect), also including conditional rendering based on the active tab based on the active tab (source).