Table refactor discovery

[zarahzachz]

(Based on discovery work in #3028)

But first, a lil' background on this...

While completing WNMGDS-2673, I found we were including an Alert component as the scrollableNotice default prop on Table. This discovery prompted a few observations:

• If scrollableNotice exists to inform users that a table requires scrolling, why does the prop allow for any React code as a child? This implementation implies teams can pass anything to scrollableNotice - not necessarily something warning users a table is scrollable.
• Why does the Alert passed to scrollableNotice have its own table-specific CSS class? It has the class ds-c-table__scroll-alert, but this isn't documented anywhere and overwrites Alert styles by specifying normal font weight for all text inside (Alert has bold headings).
• The Alert itself isn't entirely visible on smaller breakpoints, requiring scrolling to read the instructions that say scrolling is required. That makes this Alert redundant imo.
• The instructions inside the Alert only specify how keyboard users can interact with the table (specifying arrow keys) when other modalities could also be used to view the table.

I followed this discovery up with a brief meeting with accessibility who informed us that scrollable tables violates SC 1.4.10 Reflow and if possible, we should modify our Tables to enable the stackable prop/style by default for all tables on smaller viewports.

#3028 digs into how our Table solution is currently implemented, how feasible it is to implement the stackable layout as a default, and what that effort might look like.

What does our current Table API give folks?

Before I get into my findings, I want to highlight what our current API offers. And this might be better illustrated by showing how our API compares to writing a native HTML table.

Our API, based on the default table example:

<Table>
    <TableCaption>Foo Table Caption</TableCaption>
    <TableHead>
        <TableRow>
            <TableCell key="Foo heading" id="fooHeading">Foo Heading</TableCell>
        </TableRow>
    </TableHead>
    <TableBody>
        <TableRow key="Foo cell content">
            <TableCell
                key="Foo heading-Foo cell content"
                headers="fooHeading"
                stackedTitle="Foo heading"
            >
                Foo cell content
            </TableCell>
        </TableRow>
    </TableBody>
</Table>

Native HTML, using the same example:

<table class="foo-table">
    <caption class="foo-caption">Foo Table Caption</caption>
    <thead>
        <tr>
            <th role="columnheader" scope="col" id="fooHeading" class="foo-cell">Foo Heading</th>
        </tr>
    </thead>
    <tbody>
        <tr>
            <td headers="fooHeading" class="foo-cell">Foo cell content</td>
        </tr>
    </tbody>
</table>

What does our API do that native HTML can't?

• Encapsulates styles
• Applies the scope attribute
  • scope isn't correctly applied in more complex examples
• Assigns the correct role to <th>
  • In more complex examples, this role needs to be defined in the data itself via bodyComponent
• Can use a stackedTitle prop that allows developers to implement the stacked layout at a given breakpoint

If you compare the structures, developers are still writing the same kind of tags (e.g., <TableRow> instead of <tr>, or <TableCaption> instead of <caption>). Developers also need to correctly apply headers and id to the <TableCell> component, which is the same expectation for writing <th> or <td>.

Findings

In short, it's kind of possible to implement the stackable layout by default. #3028 goes into what specific code changes would be needed to do this, but it's not a perfect solution. I'm going to call issues out here, but if you wanna gain a deeper understanding of what's happening, read that PR.

• Automatically applying the stackable layout to tables won't always fix the reflow violation.
• The stackable layout style only works for one specific type of table (a table with a single header row, where each column has its own header, like this example) and breaks for more complex table layouts (like multi-header table).
• The current API is complex and inaccessible.

The stackable layout won't fix the reflow violation

This is probably a minor issue, but I think it's worth calling out all the same.

To satisfy the reflow success criterion, we need to prevent side-scrolling on tables. To prevent side-scrolling, it was decided that the stackable layout should be baked into the table CSS, rendering the stacked layout at smaller breakpoints and the normal table layout at larger breakpoints.

Code at this point looks like this:

<Table>
  [...additional Table sub-components]
</Table>

At a glance, this seems totally doable; you just add a media query dictating when each layout kicks in and you're off to the races! But the big complication here is in the size of the dataset a developer's passing to the table is entirely unpredictable. If a developer passes a large enough dataset, the stackable layout would need to turn on at a larger breakpoint to avoid the side-scrolling behavior.

So if we can't predict the breakpoint an application needs to switch between between table layouts, what can we do? Well, the current implementation does have a stackableBreakpoint prop that accepts sm, md, and lg options that tie to the breakpoints established by the design system. We could keep this prop in place, set a default value of that prop to sm so all tables use the stackable layout on mobile viewports and allow teams to override this prop if they find their table requires a larger breakpoint.

Code at this point likes like this:

<Table stackableBreakpoint="md">
  [...additional Table sub-components]
</Table>

If we keep the stackableBreakpoint prop around, it just means developers will need to manually test their table implementations to make sure they use the correct layouts on the right viewports. We would need to be very clear in our documentation that side-scrolling tables is not allowed at any breakpoint and how developers can work around that guidance.

The caveat with stackableBreakpoint is what if the dataset is so large it surpasses the lg value of that prop? Playing around with tables locally, I was able to create a side-scrolling table on desktop with 15 columns of data consisting of cells with a single word in them. If I zoom in, the scroll gets a little worse before jumping to the stackable layout. I'm not sure what to make of this discovery since we don't have a lot of real-world examples to know how our tables are used.

If we're talking in pure theoreticals, I don't know how possible it is creating a table component that intelligently switches between two different layouts if a horizontal scrollbar appears. But if we're only dealing with small datasets this probably isn't an issue. If supporting large datasets is a concern, then maybe the right solution is to repurpose the stackable prop so it renders the stackable layout across all viewports?

Code would look like this:

<Table stackable>
  [...additional Table sub-components]
</Table>

Note

To summarize this issue:

1. Implementing the stackable layout by default can be beneficial for tables with small datasets, but can't intelligently swap layouts for all sizes of datasets.
2. Teams still need to test their tables to make sure they don't have any side-scrolling, we can provide a prop to assist with any override they'd need.
3. Tables with very large datasets may need to opt-out of the default table layout entirely and only use the stackable layout, possibly using a repurposed stackable prop.

The stackable layout isn't flexible enough for complex datasets

The previous issue dealt with limitations our tables have with different dataset sizes, this one's going to get into the limitations our tables have with different dataset complexities.

Our default table example demonstrates what I mean when I refer to simple datasets; within the thead you have one row of th tags, and each th tag is assigned to one column of data. In simple datasets like this, the stackable layout removes the thead and adds a label within each cell using the stackedTitle prop indicating which cell was assigned to which column header. (Check out this example at a medium breakpoint to see what I'm talking about.)

When teams have simple datasets that "work" on small breakpoints, we still have to be aware that giving tables a stackable layout by default will result in pretty sizable issues for teams downstream. This is because teams originally had to opt-in to the stackable layout, which meant implementing the right props/data to work with both stackable and default table layouts. If a team didn't opt-in to this layout, they'll be missing the stackedTitle prop needed to display headings correctly on the stacked layout.

Going even further into more complex datasets, like you see in our multi-header table example, the stackable layout design can't account for the various horizontal and vertical headers that exist. The unaccounted for headings just disappear, resulting in loss of information users may need. The stackable layout design is just too limited.

For teams using complex datasets, implementing the stackable layout by default could be pretty disastrous. We could create a prop allowing teams to opt-out of the stackable layout, but that defeats the purpose of doing all this work to address the side-scrolling/reflow issue.

Note

To summarize this issue:

1. Implementing the stackable layout by default will require teams to ensure the props and data they're passing to tables is correct.
2. The stackable layout design isn't robust enough to handle complex tables with multiple headings.

The current API is complex and (kinda) inaccessible

Writing accessible tables is difficult; each tag requires its own attribute to ensure users can understand how a given cell relates to its header/footer content. It looks like our current Table API is meant to automate building accessible tables and it does a good job of getting most of the way there, but there are footguns in this implementation worth mentioning.

If you look at the code, you'll notice it's checking the value and context for a lot of the tags used to create a table. TableCell is a good place to go to see what I'm talking about. When you look at the TableCell props, you'll notice they're all labelled as "optional," but because a TableCell can render either a <td> or a <th> each of these tags requires certain props to be applied to be accessible. In an effort to encourage developers writing accessible code, console.warn was added to check if a certain tag is being rendered and if the correct props are being applied. And that's OK... until you look at the Table code where the prop warningDisabled allows developers to turn off these warnings.

The way I see it, having an option to disable accessibility warnings essentially undoes all the work done to encourage writing an accessible table. That's why I say the current table API is kinda inaccessible.

The other issue I see with the API is that developers still need to understand what gaps may exist in the API so they can implement any missing attributes on their own (again, to make the table accessible). You can see a good example of how the API falls short by checking out the multi-header example.

Because the dataset requires multiple headings, some of which being nested, developers have to understand where our components don't work and how to use native HTML to fill in those gaps. And once developers are pulling in native HTML, it's on them to understand where attributes like scope, id, etc. are needed. In this scenario, if developers are resorting to writing native HTML to make the tables they need, then what is our API giving them? It just seems like an added layer of complexity that gets in the way.

In an ideal world, developers would only need to learn one API to make the table they want to make. Our API can't be as flexible as native HTML without us ending up with a solution that looks identical to native HTML (sans the niceties of using native HTML like smaller bundle sizes and removing complex code). Additionally, our API isn't as robust as other third party libraries that offer features like sorting, searching, pagination, etc.

If our API isn't flexible enough to tackle any dataset while also not providing additional features (search, sorting, etc.), then I think it's wise for the design system to stop competing in the table API space and focus its energies on styling, accessibility and content guidance instead. This move would allow teams to build the table they need using the API they prefer while having the tools to make it accessible and consistent with brand guidelines.

Note

To summarize this issue:

1. Our code allows developers to misuse the table API and write inaccessible code.
2. Our API isn't as flexible as native HTML. This means developers have to understand the limitations of our API in addition to how to write native HTML accessibly.
3. Our API isn't as robust as other third party libraries. This means we're lacking features like sorting, pagination, search, etc.

Conclusion and next steps

Our table API isn't providing a whole bunch given the amount of work that went into creating it. I think our users would be better served with more well-written HTML examples, documenting how and why certain attributes are needed for accessibility and crafting more solid content guidelines instead of the API we're providing.

Next steps could include the following:

1. Develop solid and varied examples using real-world examples of tables from the teams we support as inspiration.
   1. We've been told the teams we support don't have a need for complex tables, so there's a chance we just need to pare down our implementation to be less complex.
   2. MCT made alterations to our table in the wild and it'd be interesting to compare what changes they made.
2. Consult with accessibility to determine what we need to improve on and what markup can be deleted.
3. Consult with design to figure out the stackable layout for multi-headings situation (the MCT example could be helpful here, too).
4. Develop thorough guidance for creating tables, especially accessibility testing.
5. Update our CSS API for tables to make it flexible enough to implement stackable designs on varied table structures (complex datasets).