nextui-org / nextui

🚀 Beautiful, fast and modern React UI library.
https://nextui.org
MIT License
20.7k stars 1.27k forks source link

[BUG] - Can't use react Fragment in a table body to add multiple rows with one function. #1791

Closed christo9090 closed 8 months ago

christo9090 commented 9 months ago

NextUI Version

2.1.13

Describe the bug

App returns error: TypeError: Cannot convert a Symbol value to a string if you try to return multiple rows with a single map function.

If you try to add multiple rows into the table body using a react fragment. This is a pretty common use case in react when you are mapping rows in.

Using Next 13.5

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

Take the Table example from the next UI site and wrap a fragment around the

Expected behavior

Be able to create many rows and fragment them into the body

Screenshots or Videos

image image

Operating System Version

MacOS

Browser

Chrome

TylerNRobertson commented 8 months ago

I have a similar issue if you take the TableRow components out and make a component called CustomTableRow even with TableRow as the root element within it, it throws an error saying it expects the TableRow element to be the child of the TableBody, which genuinely from a React perspective, makes no sense at all.

I've seen numerous issues on this table component and most of the time the answer is: "that's just how it works, look at react-aria table docs....." without acknowledging the fundamental issue, the current NextUI Table components are not extendable nor re-usable & don't allow developers to follow the usual react component workflow they are used too, making NextUI not ideal for apps that need to use the Table component in multiple different ways, it more or less feels like its a "preview" of an upcoming component rather than an actual full-fledged component

christo9090 commented 8 months ago

Yeah, I ultimately didn't use this table since I couldn't find a workaround. Unfortunate tho because the styling and the features of the Next UI table are really cool.

I have been using this Library extensively for the last month and I really love it, but there are a few small things that aren't as polished as other major UI libraries, but it's newer so I am guessing things will generally get ironed out over time.

sspaeth-r7 commented 8 months ago

Also hitting this same issue. Would love any suggestions or workarounds but if none exist then this library unfortunately wouldn't work for us either.

jrgarciadev commented 8 months ago

@christo9090 You don't need a fragment the TableBody accepts an array of react node

 <TableBody>
        <TableRow key="1">
          <TableCell>Tony Reichert</TableCell>
          <TableCell>CEO</TableCell>
          <TableCell>Active</TableCell>
        </TableRow>
        <TableRow key="2">
          <TableCell>Zoey Lang</TableCell>
          <TableCell>Technical Lead</TableCell>
          <TableCell>Paused</TableCell>
        </TableRow>
        <TableRow key="3">
          <TableCell>Jane Fisher</TableCell>
          <TableCell>Senior Developer</TableCell>
          <TableCell>Active</TableCell>
        </TableRow>
        <TableRow key="4">
          <TableCell>William Howard</TableCell>
          <TableCell>Community Manager</TableCell>
          <TableCell>Vacation</TableCell>
        </TableRow>
      </TableBody>
    </Table>
  );
}

Is important to know that react-aria Collection components don't accept custom components as children https://react-spectrum.adobe.com/react-stately/collections.html

Only specific ones like Select, Item, Row, etc... Because all those components have a function called getCollectionNode which serves to build the entire collection.

@TylerNRobertson a workaround is to inject the getCollectionNode to your custom component from the base one.

Example:

import React from "react";
import {Table, TableHeader, TableColumn, TableBody, TableRow, TableCell, TableRowProps} from "@nextui-org/react";

const MyRow = (props: TableRowProps) => {
  return <TableRow {...props} />
}

// @ts-ignore
MyRow.getCollectionNode = TableRow.getCollectionNode;

export default function App() {
  return (
    <Table aria-label="Example static collection table">
      <TableHeader>
        <TableColumn>NAME</TableColumn>
        <TableColumn>ROLE</TableColumn>
        <TableColumn>STATUS</TableColumn>
      </TableHeader>
      <TableBody>
        <MyRow key="1">
          <TableCell>Tony Reichert</TableCell>
          <TableCell>CEO</TableCell>
          <TableCell>Active</TableCell>
        </MyRow>
        <MyRow key="2">
          <TableCell>Zoey Lang</TableCell>
          <TableCell>Technical Lead</TableCell>
          <TableCell>Paused</TableCell>
        </MyRow>
        <MyRow key="3">
          <TableCell>Jane Fisher</TableCell>
          <TableCell>Senior Developer</TableCell>
          <TableCell>Active</TableCell>
        </MyRow>
        <MyRow key="4">
          <TableCell>William Howard</TableCell>
          <TableCell>Community Manager</TableCell>
          <TableCell>Vacation</TableCell>
        </MyRow>
      </TableBody>
    </Table>
  );
}

https://codesandbox.io/p/sandbox/beautiful-breeze-6hfn3n?file=%2FApp.jsx%3A1%2C1-43%2C1&utm_medium=sandpack

christo9090 commented 8 months ago

@jrgarciadev Thank you for the response. I appreciate you looking into this.

However, I need the fragment to map in data properly, the example above was just to show how to recreate the error that was thrown.

Let's say you have data like this:

const data = [
  { name: 'example', age: 1, sublist: [ { name: 'subName', } ]  }
]

Then you wanted to map it into the table and conditionally add in the sublist below the row. Standard react patterns would have you do something like this.

<TableBody>
   { data.map((row) => (
     <>
       <TableRow>
          <TableCell>{row.name}</TableCell>
          <TableCell>{row.age}</TableCell>
        </TableRow>

       {showSublist && data.sublist.map(subRow =>  <TableRow>...</TableRow>)}
    </>
   )) }
  </TableBody>

However, because you cant fragment the 2 rows together, you either have to flat map the data together in an external function, or use a different table. In my case, it was simpler to use a different table since the data is complex.

So hopefully you can see the issue there and how your response doesn't address it. But if this is a limitation of react-aria and this pattern will never be addressed, it's not a big deal.

jrgarciadev commented 8 months ago

Hey @christo9090 you can use the dynamic render for that https://nextui.org/docs/components/table#dynamic

Example:

const mainRows = [
  {
    key: "1",
    name: "Tony Reichert",
    role: "CEO",
    status: "Active",
  },
  {
    key: "2",
    name: "Zoey Lang",
    role: "Technical Lead",
    status: "Paused",
  },
];

const subRows = [
  {
    key: "3",
    name: "Jane Fisher",
    role: "Senior Developer",
    status: "Active",
  },
  {
    key: "4",
    name: "William Howard",
    role: "Community Manager",
    status: "Vacation",
  },
];

 <Table aria-label="Example table with dynamic content">
      <TableHeader columns={columns}>
        {(column) => <TableColumn key={column.key}>{column.label}</TableColumn>}
      </TableHeader>
      <TableBody items={rows}>
        {(item) => (
          <TableRow key={item.key}>
            {(columnKey) => (
              <TableCell>{getKeyValue(item, columnKey)}</TableCell>
            )}
          </TableRow>
        )}
      </TableBody>
    </Table>

https://codesandbox.io/p/sandbox/prod-paper-tz6sv4?file=%2FApp.jsx%3A61%2C4-74%2C13&utm_medium=sandpack

christo9090 commented 8 months ago

@jrgarciadev Interesting. Thanks much for the quick response. I will test that now.

maniteja-emmadi commented 1 week ago

Any fix for this? I have a similar use case.

tableData is a list of Users, and on selection of a user, I have to render another row below the selected row, and NextUI Table does not support Fragment

tableData.map((item) => (
  <Fragment key={item.id}>
    <TableRow key={`table-row-${item.id}`}>
      <TableCell>{item.name}</TableCell>
      <TableCell>{item.role}</TableCell>
    </TableRow>
    {selectedRow === item.id && (
      <TableRow>
        <TableCell></TableCell>
        <TableCell><Button>Edit</Button></TableCell>
      </TableRow>
    )}
  </Fragment

)

Please fix this as the UI requirements can be endless

maniteja-emmadi commented 1 week ago

I just fixed this using a workaround, not a beautiful workaround though, but here is the fix

{tableData.map((item) =>
  Array.from({length: item.id === selectedRow ? 2 : 1}).map((_, index) => {
    return index === 0 ? TableRow with Data : TableRow with Edit Button
  }

And this fix is working like a charm, thanks for starting the thread @christo9090 , I wouldn't have thought of this fix, if I hadn't read all these comments

TarasVasylivskyy commented 13 hours ago

I just fixed this using a workaround, not a beautiful workaround though, but here is the fix

{tableData.map((item) =>
  Array.from({length: item.id === selectedRow ? 2 : 1}).map((_, index) => {
    return index === 0 ? TableRow with Data : TableRow with Edit Button
  }

And this fix is working like a charm, thanks for starting the thread @christo9090 , I wouldn't have thought of this fix, if I hadn't read all these comments

Sorry, didn't get how to use this fix. Can you add some example please?

maniteja-emmadi commented 12 hours ago

I just fixed this using a workaround, not a beautiful workaround though, but here is the fix

{tableData.map((item) =>
  Array.from({length: item.id === selectedRow ? 2 : 1}).map((_, index) => {
    return index === 0 ? TableRow with Data : TableRow with Edit Button
  }

And this fix is working like a charm, thanks for starting the thread @christo9090 , I wouldn't have thought of this fix, if I hadn't read all these comments

Sorry, didn't get how to use this fix. Can you add some example please?

Okay, let me give one more example, we have a requirement to show only one column in mobile breakpoint and 3 columns in desktop breakpoint, with additional functionality that does not concern the problem.

Here is the way that I cannot write

<TableBody>
...beginning of the jsx
   {isMobile ?
        <TableColumn {...whateverProps}>
            {content}
        </TableColumn>
    :
    //! Now here I cannot use Fragment to show 3 columns 
         <Fragment>
              <TableColumn>
              </TableColumn>
             <TableColumn>
             </TableColumn>
             <TableColumn>
             </TableColumn>
         </Fragment>
   }
...rest of the jsx
</TableBody>

I can not write like above because TableBody does not accept Fragment as a child, so I have to write it like the following.

<TableBody>
...beginning of the jsx
   {isMobile ?
        <TableColumn {...whateverProps}>
            {content}
        </TableColumn>
    :
    //! Now here I cannot use Fragment to show 3 columns 
          ['column1', 'column2', 'column3'].map(column => (
              <TableColumn key={column}>
                  {data?.[column]}
              </TableColumn>
          )
   }
...rest of the jsx
</TableBody>

You can write it like above since there's no fragment involved and you are just returning TableColumns with a map function.

I hope you understand it now.

TarasVasylivskyy commented 11 hours ago

I just fixed this using a workaround, not a beautiful workaround though, but here is the fix

{tableData.map((item) =>
  Array.from({length: item.id === selectedRow ? 2 : 1}).map((_, index) => {
    return index === 0 ? TableRow with Data : TableRow with Edit Button
  }

And this fix is working like a charm, thanks for starting the thread @christo9090 , I wouldn't have thought of this fix, if I hadn't read all these comments

Sorry, didn't get how to use this fix. Can you add some example please?

Okay, let me give one more example, we have a requirement to show only one column in mobile breakpoint and 3 columns in desktop breakpoint, with additional functionality that does not concern the problem.

Here is the way that I cannot write

<TableBody>
...beginning of the jsx
   {isMobile ?
        <TableColumn {...whateverProps}>
            {content}
        </TableColumn>
    :
    //! Now here I cannot use Fragment to show 3 columns 
         <Fragment>
              <TableColumn>
              </TableColumn>
             <TableColumn>
             </TableColumn>
             <TableColumn>
             </TableColumn>
         </Fragment>
   }
...rest of the jsx
</TableBody>

I can not write like above because TableBody does not accept Fragment as a child, so I have to write it like the following.

<TableBody>
...beginning of the jsx
   {isMobile ?
        <TableColumn {...whateverProps}>
            {content}
        </TableColumn>
    :
    //! Now here I cannot use Fragment to show 3 columns 
          ['column1', 'column2', 'column3'].map(column => (
              <TableColumn key={column}>
                  {data?.[column]}
              </TableColumn>
          )
   }
...rest of the jsx
</TableBody>

You can write it like above since there's no fragment involved and you are just returning TableColumns with a map function.

I hope you understand it now.

Didn't get how it works with tableRows then