Fluentui: ShimmerDetailsList - onRenderMissingItem doesn't fire.

Created on 25 Jun 2019  路  13Comments  路  Source: microsoft/fluentui

  • __Package version(s)__: 7.5.0

Hi

I recently updated my project to use Office React package 7.5.0 from 6. One thing we had is we used a DetailsList component with infinite scrolling and enabling of the shimmer when data was being loaded. This was working fine.

However since version 7.0, Office React doesn't contain the shimmer properties on the DetailsList anymore and so I tried to update using the ShimmerDetailsList to counteract that.

My issue though is that the onRenderMissingItem is not fired anymore despite my data accessor being the same - it returns all data plus a "null" record to trigger the onRenderMissingItem method.

I looked around but didn't fine anyone else describing the issue for the ShimmerDetailsList - has the implementation changed ?

Actual behavior:

Does not fire the onRenderMissingItem

Expected behavior:

That it fires onRenderMissingItem ;o)

Priorities and help requested:

Requested priority: (Blocking, High, Normal, Low)
Normal

Products/sites affected: (if applicable)

DetailsList Shimmer Needs ASAP Fixed

Most helpful comment

@vmary2014 with the current fix submitted you should be able to do same things as before with a slight API change on your side. You will basically have to provide the onRenderCustomPlaceholder callback that now passes you the default shimmer row render so you can still have them render properly and not build from scratch and also execute any custom logic before the render. It should look something like this:

<ShimmeredDetailsList
  onRenderCustomPlaceholder={this._onRenderCustomPlaceholder}
/>

private _onRenderCustomPlaceholder = (
  rowProps: IDetailsRowProps,
  index: number,
  defaultRender: (props: IDetailsRowProps) => React.ReactNode
): React.ReactNode => {
  // custom logic execution
  return defaultRender(rowProps);
}

All 13 comments

Looking further and taking the source code of the office react project, I looked at the tests for the DetailsList and ported one test to the ShimmerDetailsList as follow :

it('shimmerDetailsList invokes optional onRenderMissingItem prop once per missing item rendered', () => {
    const onRenderMissingItem = jest.fn();
    const items = [...mockData(5), null, null];
    mount(<ShimmeredDetailsList items={items} skipViewportMeasures={true} onRenderMissingItem={onRenderMissingItem} />);
    expect(onRenderMissingItem).toHaveBeenCalledTimes(2);
  });

in the file DetailsList.test.tsx

It seems to be definitely not triggered as the result of the test is :

Expected mock function to have been called two times, but it was called zero times

So i'm thinking this is possibly a bug as it looks like the source code implements onRenderMissingItem?

Any input appreciated, thanks.

Thanks for your detailed report @vmary2014. We'll take a look and get back to you soonest!

@vmary2014, ShimmeredDetailsList overrides onRenderMissingItem to know to render the "shimmer" rows, relevant LoC:

https://github.com/OfficeDev/office-ui-fabric-react/blob/044e13c2c8c391885693bbaa493e975fab35f3ea/packages/office-ui-fabric-react/src/components/DetailsList/ShimmeredDetailsList.base.tsx#L66

I'm not sure what behavior we should support if we allow for overriding via prop in ShimmeredDetailsList.

However, DetailsList still invokes onRenderMissingItem as you expect, LoC:

https://github.com/OfficeDev/office-ui-fabric-react/blob/044e13c2c8c391885693bbaa493e975fab35f3ea/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.base.tsx#L583

Would using DetailsList be a viable option for you?

https://github.com/OfficeDev/office-ui-fabric-react/pull/9462 reverted a number of ShimmeredDetailsList changes that might have impacted you. Behaviors should be closer to that of 6.x now.

cc: @Vitalius1

Hi. Thanks for taking the time to look into this.

So we initially were using the DetailsList with the Shimmer options. The onRenderMissingItem was used to fetch additional data when the last records were displayed when a user scrolls down. This is the technique that is shown in the advanced example of the DetailsList component.

However In recent updates of Office Fabric (I think 7.0) the Shimmer options have been removed from the DetailsList to the benefit of the ShimmeredDetailsList.

The issue I am facing is that as far as I can see there is no way for me to fetch more data on the ShimmerDetailsList since as you say onRenderMissingItem is overridden and I cannot see another event that would replace it to indicate that more data is required.

The end goal is to have an infinite scroll for our Web UI lists. We don鈥檛 use pages but instead want to load data when the user needs it. The shimmer effect is a nice way to indicate to the user that something is happening in the background.

I looked at #9462 and that was merged in 7.3.0 but the issue I describe is in version 7.5.0.

Hope that clarifies more what it is we are trying to achieve.

Thanks.

Going to add @Vitalius1 as an assignee who is most familiar with this area of code. We will work towards a solution when he is back in the office next week.

Any changes I provide would benefit from his review so it is best he be included.

@Vitalius1, is this solvable with the current code? If not, what workaround should we propose and document?

Ping @Vitalius1 and @KevinTCoughlin can you come up with a solution here?

@vmary2014 with the current fix submitted you should be able to do same things as before with a slight API change on your side. You will basically have to provide the onRenderCustomPlaceholder callback that now passes you the default shimmer row render so you can still have them render properly and not build from scratch and also execute any custom logic before the render. It should look something like this:

<ShimmeredDetailsList
  onRenderCustomPlaceholder={this._onRenderCustomPlaceholder}
/>

private _onRenderCustomPlaceholder = (
  rowProps: IDetailsRowProps,
  index: number,
  defaultRender: (props: IDetailsRowProps) => React.ReactNode
): React.ReactNode => {
  // custom logic execution
  return defaultRender(rowProps);
}

FYI @soettl ^

Thanks very much guys for sorting this issue. Really appreciate your efforts and how you followed up. I will test those changes as soon as they are in the released package.

Hi
Just to confirm that the issue is fixed for me in version 7.17.0 of the office-ui-fabric-react package.
Thanks again for your help, it is much appreciated !
Vincent

@vmary2014 Yes, according to the release notes 7.17.0 is the release containing the fix. Let us know in the issue thread if it solves your scenario.

:tada:This issue was addressed in #9930, which has now been successfully released as [email protected].:tada:

Handy links:

Was this page helpful?
0 / 5 - 0 ratings