Convert table cell header into undefined if it's been provided to EuiBasicTable as a non-string node.#1312
Conversation
…to EuiBasicTable as a non-string node. - Change name prop-type of action columns to node.
cchaos
left a comment
cchaos
left a comment
There was a problem hiding this comment.
Thanks for taking this on to fix it. It think this is an ok fix for now, but may be odd for consumers who don't see their column headers in responsive view. I have a note to myself to fix some of the responsive table stuff so I'll just add handling of non-string headers to that.
I noticed that I always display a space for the mobile header even if the data attribute doesn't exist, which is why there's so much spacing in your screenshot. Can you change this line:
eui/src/components/table/_responsive.scss
Line 117 in 62aec19
to &[data-header]::before { ?
| // Name can also be an array or an element, so we need to convert it into a string. We can't | ||
| // stringify the value, because this value is rendered directly in the mobile layout. So the | ||
| // best thing we can do is render nothing. | ||
| const header = typeof name === 'string' ? name : ''; |
There was a problem hiding this comment.
I think it's best to let the EuiTableRowCell handle this since it's the one that places the prop in a string-only data attribute. It will also be there to catch for anyone who builds their own custom tables.
|
Thanks @cchaos I made the change to hide the header:
Per our convo I'll defer changing the prop type until we make the change to render the header using React instead of CSS. |
cchaos
left a comment
cchaos
left a comment
There was a problem hiding this comment.
Thanks for adding in the CSS fix!
cjcenizal
changed the title
2 participants
This fixes a prop-type warning and poor UI when you provide a non-string node. This PR also changes the
nameprop-type of action columns to node, leftover from #1234.Before
After