wave icon indicating copy to clipboard operation
wave copied to clipboard

`TableCell` and `TableHeaderCell` don't apply `display` prop although both accept `LayoutProps`

Open arturmiglio opened this issue 2 years ago • 1 comments

  • @freenow/wave version: v1.2.0

Relevant code

<TableCell (...) display='none' />

Probably missing layout in the compose function:

https://github.com/freenowtech/wave/blob/517c13628a2b494702704c570e2327ad81421a84/src/components/Table/components/TableCell.tsx#L26 https://github.com/freenowtech/wave/blob/517c13628a2b494702704c570e2327ad81421a84/src/components/Table/components/TableHeaderCell.tsx#L32

What was expected to happen?

The prop is applied and the display (at least) can be controlled. The main use-case is responsive tables, where we want to hide certain columns to fit narrower screens. Then we could pass responsive to hide elements in certain breakpoints values like:

<TableCell (...) display={['none', 'table-cell']} />

And if display prop is finally not allowed, typescript should trigger an error.

Reproduction

https://codesandbox.io/s/wave-playground-forked-bnn69g?file=/src/App.tsx

arturmiglio avatar May 10 '22 17:05 arturmiglio

So I dug around a bit, and for those who didn't know (I didn't), the display related props don't exist on tr, td elements. I fixed it as per the suggestion in this commit but I am not sure that this is the best approach.

Additionally, should we add layout prop styling for TableRow elements as well? probably not related to responsiveness but wondering if there can be usecase to extend this prop to show/hide a row

lloydaf avatar May 30 '22 17:05 lloydaf