marmelab / react-admin

A frontend Framework for building data-driven applications running on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.55k stars 5.2k forks source link

Multiple expandable datagrids: expanding a row triggers all datagrids to expand #9688

Open roxeteer opened 5 months ago

roxeteer commented 5 months ago

What you were expecting: When I click to expand a row in a datagrid, only that row should expand.

What happened instead: If I have multiple expandable datagrids on the same page, the same row in all of them expand (i.e. if I click the first row, the first row of all datagrids expand).

Steps to reproduce:

  1. Create a view with multiple expandable datagrids.
  2. Expand a row.
  3. See that more than one row expands.
  4. Collapse a row.
  5. See that all the previous rows collapse, too.

Related code:

<SimpleShowLayout>
  <ArrayField<Product> source="descriptionSource">
    <Datagrid bulkActionButtons={false} expand={<DescriptionPanel />} expandSingle>
      <TextField<ProductAttribute> source="language" />
      <TextField<ProductAttribute> source="value" />
    </Datagrid>
  </ArrayField>

  <ArrayField<Product> source="descriptionHtmlSource">
    <Datagrid bulkActionButtons={false} expand={<DescriptionPanel />} expandSingle>
      <TextField<ProductAttribute> source="language" />
      <TextField<ProductAttribute> source="value" />
    </Datagrid>
  </ArrayField>
</SimpleShowLayout>

The issue arises because all the datagrids use the parent resource name as the localStorage key. In this case expanding the first row of either of the datagrids stores ["row0"] with the key RaStore.Product.datagrid.expanded.

Other information: Since expand uses <Datagrid>'s resource prop value in creating the localStorage key, setting the prop to a unique string makes the localStorage key unique as well. However, I'm not entirely sure what else resource prop is used for here and if it creates side-effects. Like this:

<Datagrid bulkActionButtons={false} expand={<DescriptionPanel />} expandSingle resource="descriptionSource">

// ...

<Datagrid bulkActionButtons={false} expand={<DescriptionPanel />} expandSingle resource="descriptionHtmlSource">

Environment

slax57 commented 5 months ago

Thank you for opening this issue.

Indeed, currently the expand controller uses a Store key built only with the resource: `${resource}.datagrid.expanded`.

There is currently no way to override that store key (except passing a different resource).

We could introduce the ability to customize this store key (using e.g. the storeKey prop from the ListContext), but this would be a new feature.

Would you like to open a PR (against the next branch) to implement it?

roxeteer commented 5 months ago

Would you like to open a PR (against the next branch) to implement it?

I will take a look.

davidhenley commented 4 months ago

@slax57 I just came to ask this same question. Very annoying you can't pass storeKey down to the list context inside ArrayField. I would absolutely love this.

Is there any work around or should we just create a new implementation of ArrayField for now?

roxeteer commented 4 months ago

@davidhenley I was about to implement this, but I wasn't sure what would be the best approach. I asked about it on Discord but never got a reply so I forgot the whole thing 🙈

davidhenley commented 4 months ago

@roxeteer I'm digging through the ListContext source code right now, and am trying the same. It should be as simple as adding a prop and passing it down unless I'm missing something as the List just passes it to ListBase which passes it to the ListContext.

roxeteer commented 4 months ago

@davidhenley I think the more React Admin-like approach would be to create yet another context for it. ListContext is quite huge already and it's probably not needed to be exposed. preferenceKey is passed around in similar manner, using its own context provider.

davidhenley commented 4 months ago

I don't understand this logic, it would be the exact same as it is now, just pass it to ListContext under ArrayField. What am I missing?

roxeteer commented 4 months ago

Maybe I was thinking it too complicated. It's been a while already so can't remember how it was exactly.