react-admin icon indicating copy to clipboard operation
react-admin copied to clipboard

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

Open roxeteer opened this issue 11 months ago • 8 comments

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

  • React-admin version: 4.16.11
  • React version: 18.2.0

roxeteer avatar Feb 29 '24 07:02 roxeteer

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?

slax57 avatar Feb 29 '24 15:02 slax57

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

I will take a look.

roxeteer avatar Mar 01 '24 18:03 roxeteer

@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?

davidhenley avatar Mar 29 '24 14:03 davidhenley

@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 🙈

roxeteer avatar Mar 29 '24 14:03 roxeteer

@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.

davidhenley avatar Mar 29 '24 14:03 davidhenley

@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.

roxeteer avatar Mar 29 '24 14:03 roxeteer

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?

davidhenley avatar Mar 29 '24 15:03 davidhenley

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

roxeteer avatar Mar 29 '24 16:03 roxeteer

This proposal didn't receive enough contributions from the community so we'll close the issue.

We'll accept any PR that implements this change.

fzaninotto avatar Sep 04 '24 19:09 fzaninotto