ithaka / pharos

JSTOR's design system
https://pharos.jstor.org
MIT License
123 stars 16 forks source link

Table: Allow for complex data to be displayed in the table component #808

Open brentswisher opened 2 months ago

brentswisher commented 2 months ago

The problem There are several instances where having more complex data than just plain text would be useful in a table component, but that currently isn't possible with the way that the pharos table ingests table rows.

The solution Add a slot-able table body that consumers could pass into the table. They would be responsible for passing <tr> elements into the slot. They can have full control over those rows and what is inside them. This allows maximum flexibility for what could be contained in a table, and allows for things like inline editing or bulk selection.

It would not replace the current RowData property, which is still useful for simple table. Consumer could pass in either the slot, or the RowData property, but not both. The RowData would be relegated to only “plain” data - anything complex would need to use the slotted approach instead.

Alternatives considered There could be two table components in Pharos instead - one for tables with more complex data and one for plain text data. While the separation would make the individual components easier to reason about, there would be too much duplication to justify separating them. A "data table" is really just a simple version of a "table". By continuing to offer the RowData interface, we keep the speed and nice DX of the existing table, and add an escape hatch for users with more complex needs.

Table Example

Screenshot 2024-08-30 at 1 41 07 PM
brentswisher commented 2 months ago

Just a note, we may need to think about accessibility concerns when this is combined with a hasStickyHeader (WCAG 2.4.11 - Focus not obscured)

brentswisher commented 2 months ago

Hmm, so plot twist - the HTML spec for tables don't support slots, and render them outside the table body if you try to add them.

https://stackoverflow.com/questions/63900825/webcomponents-based-on-table-slot-out-of-flow

brentswisher commented 1 month ago

Okay, a long-winded update on this issue after a bit more investigation:

After exploring for quite a while, I don't see any good workarounds to the "no slots in a table" technical problem.

Looking around at other web component based design systems, a common pattern I see is to not use table elements ( e.g. table, tr, td) at all, but to use entirely custom elements, and use CSS to set their display properties to operate like tables.

One example of this is here in the carbon design system, which looks something like this in code:

 <cds-table>
  <cds-table-head>
    <cds-table-header-row>
      <cds-table-header-cell>Name</cds-table-header-cell>
      <cds-table-header-cell>Rule</cds-table-header-cell>
      <cds-table-header-cell>Status</cds-table-header-cell>
      <cds-table-header-cell>Other</cds-table-header-cell>
      <cds-table-header-cell>Example</cds-table-header-cell>
    </cds-table-header-row>
  </cds-table-head>
  <cds-table-body>
    <cds-table-row>
      <cds-table-cell>Load Balancer 1</cds-table-cell>
      <cds-table-cell>Round robin</cds-table-cell>
      <cds-table-cell>Starting</cds-table-cell>
      <cds-table-cell>Test</cds-table-cell>
      <cds-table-cell>22</cds-table-cell>
    </cds-table-row>
    ...
  </cds-table-body>
</cds-table>

If we were to do something like that, we would no longer have the technical limitations we are running into with straight table html, and could do pretty much exactly what we originally wanted.

I'll admit, when I first saw this I thought "No way, no how should we do that" it goes against every web standard and semantic markup bone in my body, and making it accessible seems like a real chore.

However, after looking for better alternatives, I'm just not seeing many.

In additional to that, I know there is a real, current user need for a table that can handle more complex data in Pharos than just plain text. In our current implementation, the table can't even contain links or images.

So, what to do?

As I see it there are three options going forward:

1. Do nothing, continue not supporting complex data in tables. We can probably all agree this is the worst option. These types of components do and will continue to exists in a lot of systems, and not standardizing them in Pharos just means we will have a bunch of different implementation.

2. Look into updating the table component to use all custom elements Despite my initial gut reaction, this is where I am leaning as the best path forward. I do think it could be a lot of work, but working closely with @sirrah-tam to make sure it is accessible, I'm optimistic we could get something that works for everyone.

3. Create a new component, called something else like "data-grid" or "item-grid" (name suggestions welcome). It can be used for complex data, and will look like a table, but isn't a table, we would keep the current table

I see the appeal of this, but I think at the end of the day we will end up with two components which look and feel very similar to consumers, but are very different "under the hood." The biggest downside of this that I see is when new feature request for one come in, the work will often have to be duplicated. Worse yet, the underlying implementations will be so different, there may be edge cases where feature parity wouldn't be possible. At that point, the line would get even blurrier because things like "I only have simple data, but I have to use this other component instead of the table because it has "some feature" the table doesn't have yet."

I'm curious what everyone thinks? Are there other options I haven't considered, or opinions on which would be the best path forward?

eslawski commented 1 month ago

I agree that option (2) seems like the lesser of all evils at this juncture. Assuming we will want to release this in a major version as it will be a breaking change for anyone using the existing implementation of table component.

brentswisher commented 1 month ago

Assuming we will want to release this in a major version as it will be a breaking change for anyone using the existing implementation of table component.

Maybe? I'm hopeful we could support both use cases to start, by marking the current tableRows property as deprecated but keeping it working if there are no child elements in the table. Then at the next major release drop it. Hard to say for sure without getting into the implementation details though.

Could also be a conversation if keeping both inputs as options (data in an object or children elements) is worthwhile, though I lean toward not just for simplicity sake.

eslawski commented 1 month ago

Assuming we will want to release this in a major version as it will be a breaking change for anyone using the existing implementation of table component.

Maybe? I'm hopeful we could support both use cases to start, by marking the current tableRows property as deprecated but keeping it working if there are no child elements in the table. Then at the next major release drop it. Hard to say for sure without getting into the implementation details though.

Could also be a conversation if keeping both inputs as options (data in an object or children elements) is worthwhile, though I lean toward not just for simplicity sake.

Ya that all makes sense. And ya I agree, in the long term i'd prefer a singular way of defining the table for either simple or complex use cases. Otherwise we effectively end up with a watered down version of (3) from above.