wave icon indicating copy to clipboard operation
wave copied to clipboard

Strategy to pass the property `whiteSpace` to cells

Open div-Leo opened this issue 3 years ago • 4 comments

What The table by default renders with the property whiteSpace set to nowrap.

By default cells has whiteSpace set to nowrap which will make cells expand depending on the content instead of the given width. If you really want to fix a width make sure to override the whiteSpace property too. On top of that make sure to check your table a11y. An in-depth guide of accessible tables can be found at https://a11y-101.com/development/tables

In order to change this behavior, as mentioned in the comment above, we need to overwrite whiteSpace with normal. Which of the two following options should be the conventional way to implement this case?

Option 1:

const WrappedTableCell = styled(TableCell)`
    white-space: normal;
`

<WrappedTableCell>
  {...}
</WrappedTableCell>
<WrappedTableCell>
  {...}
</WrappedTableCell>
<WrappedTableCell>
  {...}
</WrappedTableCell>

Option 2:

<TableCell whiteSpace="normal">
  {...}
</TableCell>
<TableCell whiteSpace="normal">
  {...}
</TableCell>
<TableCell whiteSpace="normal">
  {...}
</TableCell>

As an alternative solution I'd like to propose a change that could make this option more accessible and easyer to use. Proposal: Add the option whiteSpace to the TableRow to be applied to each cell.

<TableRow whiteSpace="normal">
  <TableCell>
    {...}
  </TableCell>
  <TableCell>
    {...}
  </TableCell>
  <TableCell>
    {...}
  </TableCell>
</TableRow>

div-Leo avatar May 21 '21 12:05 div-Leo

I may be wrong but currently TableCell doesn't support a whiteSpace prop. In that sense, Option 2 in your description is not an option. Moving forward, I can't yet wrap my head around why the decision on using whiteSpace: nowrap instead normal @snapsnapturtle can you recall why this decision?

Also my concern here @div-Leo is that whiteSpace is a prop that comes to play because people need fixed width on cells, therefore seems to be dependent on devs to set a fixed width, isn't it?

Brainstorming... I would prefer something like "given a fixed width will set the whiteSpace to normal" (but I am not sure if this is worthwhile now)

@div-Leo why do you need the whiteSpace?

duranmla avatar May 25 '21 11:05 duranmla

@duranmla this is what happens if I don't use the whiteSpace: normal gczdg6zv8c

If I use it then it looks right Screenshot 2021-05-25 at 15 52 39

Tbh, I'm not even using fixed width, it auto adapts and wraps the text in 2 lines.

div-Leo avatar May 25 '21 14:05 div-Leo

And what is the current obstacle that prevents you to use explicit lines? Because for me it feels that here I will expect the cells to be like:

<TableCell>
    <a href="mailto:[email protected]">[email protected]</a>
    <a href="tel:+44123456789">+44123456789</a>
    <p>I just want another line</p>
</TableCell>

It is way more accessible and also it is explicitly asking for new lines.

duranmla avatar May 26 '21 09:05 duranmla

@duranmla Unfortunately I do not recall why we have specified this explicitly. Keep in mind though, that in tables, we should stick to one (and a maximum of two lines for medium and large rowSpaces) row of content in the table row.

snapsnapturtle avatar May 26 '21 11:05 snapsnapturtle