amplify-ui icon indicating copy to clipboard operation
amplify-ui copied to clipboard

TableCell throws warnings on rowspan and colspan usage

Open anandkumarpatel opened this issue 2 years ago • 1 comments

Before creating a new issue, please confirm:

On which framework/platform are you having an issue?

React

Which UI component?

Primitive components

How is your app built?

Create React App

What browsers are you seeing the problem on?

Chrome, Firefox, Microsoft Edge, Safari

Which region are you seeing the problem in?

No response

Please describe your bug.

Getting errors when trying to use Table with rowspan following the example here: https://ui.docs.amplify.aws/react/components/table#spanning-multiple-columns-or-rows

Warning: Invalid DOM property `rowspan`. Did you mean `rowSpan`?

The table renders correctly, but the errors in the console should not be there.

I tried changing to rowSpan but that does not merge the rows as rowspan does.

What's the expected behaviour?

The table renders without errors in the console

Help us reproduce the bug!

run this example: https://ui.docs.amplify.aws/react/components/table#spanning-multiple-columns-or-rows

Code Snippet

import { Table, TableBody, TableCell, TableRow } from '@aws-amplify/ui-react';

export const SpanExample = () => (
  <Table variation="bordered">
    <TableBody>
      <TableRow>
        <TableCell />
        <TableCell />
        <TableCell
          colspan="2"
        />
      </TableRow>
      <TableRow>
        <TableCell
          rowspan="3"
        />
        <TableCell />
        <TableCell />
        <TableCell
          rowspan="3"
        />
      </TableRow>
      <TableRow>
        <TableCell />
        <TableCell />
      </TableRow>
      <TableRow>
        <TableCell
          colspan="3"
        />
      </TableRow>
    </TableBody>
  </Table>
);

Console log output

Warning: Invalid DOM property `rowspan`. Did you mean `rowSpan`?
    at td
    at http://localhost:3006/static/js/bundle.js:244087:11
    at http://localhost:3006/static/js/bundle.js:243601:11
    at tr
    at http://localhost:3006/static/js/bundle.js:244087:11
    at http://localhost:3006/static/js/bundle.js:243678:17
    at tbody
    at http://localhost:3006/static/js/bundle.js:244087:11
    at http://localhost:3006/static/js/bundle.js:243563:17
    at table

Additional information and screenshots

"@aws-amplify/ui-react": 5.0.3

anandkumarpatel avatar Jul 25 '23 20:07 anandkumarpatel

Hi @anandkumarpatel, thanks for reporting this issue! I can reproduce it on dev mode.

The root cause is React treats the props(rowspan and colspan) as camel case while we override them to not use camel case to avoid naming conflicts with our grid item props. As a result, React thinks this could be a misspell and warns on it. The warnings do not affect the functionality and this is a check performed by React on dev mode to highlight potential problems. That being said, they are still annoying. We will discuss this as a team and figure out a solution to fix it soon.

Besides, I also found that we did not type the props correctly:

  colspan?: Pick<React.HTMLProps<HTMLTableCellElement>, 'colSpan'>;
  rowspan?: Pick<React.HTMLProps<HTMLTableCellElement>, 'rowSpan'>;

We mistakenly use the Pick utility type here which does not give us what we we want. It will return an interface type e.g., rowspan?: { rowSpan?: number | undefined } making the prop to accept an object. Instead, we should retrieve the prop type by reading it in the element's interface with the key:

colspan?: React.HTMLProps<HTMLTableCellElement>['colSpan'];
rowspan?: React.HTMLProps<HTMLTableCellElement>['rowSpan'];

That way, the prop will take in a primitive value(number | undefined in this case) instead of an object.

zchenwei avatar Jul 25 '23 22:07 zchenwei