Fluentui: Testing DetailsList component

[aslanovsergey]

Bug Report

• __Package version(s)__: ([email protected])
• __Browser and OS versions__: (Windows 10, Chrome: 68.0.3440.84)

Describe the issue:

I'm trying to test DetailsList component and its items but for some reason it's not rendered completely when I mount it in the test I see only

              <WithViewportComponent className="fieldMappingTable" compact={true} items={{...}} setKey="set" columns={{...}} onRenderItemColumn={[Function]} onColumnHeaderClick={[Function]} selection={{...}}>
                <div className="ms-Viewport" style={{...}} />
              </WithViewportComponent>

This is only happens in the test, in browser it's rendered completely fine. Any other office-ui-fabric-react component are rendered fine in tests and in browser, except this one.

Also I created an issue on Enzyme GitHub repo https://github.com/airbnb/enzyme/issues/1745#issuecomment-411919633, and they proposed that DetailsList component mutates the DOM that cases this problem, is that true?

[KevinTCoughlin]

Unfortunately I'm not too familiar with the enzyme framework. Could they recommend a fix with regards to the mutations that are causing the test to fail? Perhaps there's some sort of delay that can be inserted into the test or we arrive at an approach that works for both our tests and yours.

Is there a recommended approach for writing components to avoid this problem for their test suite? That may help us understand the problem better. I'll ask in that issue as well.

EDIT: I see now they recommended: https://github.com/airbnb/enzyme/blob/d65fc69d87e985fcb51ad5cdc8fdec4933519fe6/docs/common-issues.md#testing-third-party-libraries

From what the above suggests, it looks like you can either test the render method for the component or perhaps leverage ref or componentRef.

[KevinTCoughlin]

TIL we use enzyme ourselves after seeing #5887...

[dzearing]

The DetailsList is virtualized and does not render the entire viewport. Could use some more clarity here, like what you're trying to test/validate, and maybe some sample code of what you expected and what happened instead.

[msft-github-bot]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[aslanovsergey]

I'am trying to test my component from UI perspective calling buttons, sorting, selecting rows

[msft-github-bot]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 5 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for your contributions to Fabric React!

[jshaker]

I have the same issue. Also what's up with that bot? No recent activity but no fix..

[lalicari]

@dzearing The thing I'm trying to test is making sure the data I'm getting from our API gets rendered in the DetailsList. I'm using enzyme's mount method, though I think I could get away with using shallow or render. The problem I have with the DetailsList is that it does auto rendering magic and doesn't let us specify a single object to render in each row so we can't unit test the data we're getting back from our API. Instead we give a list of data to DetailsList.items. It's making it difficult to test with. This is a snippet of what I'm seeing from my wrapper.debug statement in my enzyme test. Let me know if there's more info I can provide

[dzearing]

Hmm in your tests, if you disable virtualization, it might work better. Could you try this (for test scenarios)?

          <Fabric.DetailsList 
            onShouldVirtualize={ () => false }
            items={ items } 
            columns={ columns }             
          />     

There is a way to inject custom rows, but I don't think that's your issue. DetailsList will virtualize by default, meaning that it won't render things off screen, but for testing you want everything rendered synchronously.

[KevinTCoughlin]

👍 to disabling virtualization for test unless you need to verify those scenarios. The virtualization logic of List has a setState call in its initial render path that causes _only the container_ to render on initial pass. Because of this the tests see missing rows and typically fail.

That's why for now our tests either disable virtualization or trigger an additional render pass, see:

• https://github.com/OfficeDev/office-ui-fabric-react/blob/50c334ccae034993dccd11d59b55abe7212af0c4/packages/office-ui-fabric-react/src/components/List/List.test.tsx#L104
• https://github.com/OfficeDev/office-ui-fabric-react/blob/50c334ccae034993dccd11d59b55abe7212af0c4/packages/office-ui-fabric-react/src/components/DetailsList/DetailsList.test.tsx#L446

[lalicari]

Is there a cleaner way than passing down a prop down through my component that renders the DetailsList to disable/enable virtualization? Here's the debug from disabling virtualization, still no data being rendered :/

[KevinTCoughlin]

Is there a cleaner way than passing down a prop down through my component that renders the DetailsList to disable/enable virtualization?

Not currently, but your component can accept an optional prop of IDetailsListProps for overrides.

However, I think the problem you are facing is different; I believe it is this LoC:

https://github.com/OfficeDev/office-ui-fabric-react/blob/50c334ccae034993dccd11d59b55abe7212af0c4/packages/office-ui-fabric-react/src/utilities/decorators/withViewport.tsx#L124

To verify, can you modify your DetailsList props to contain the following prop skipViewportMeasures set to true and re-run the test(s):

diff --git a/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx b/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
index 0365d2c11..6c5546d00 100644
--- a/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
+++ b/packages/office-ui-fabric-react/src/components/DetailsList/examples/DetailsList.Basic.Example.tsx
@@ -77,6 +77,7 @@ export class DetailsListBasicExample extends React.Component<{}, IDetailsListBas
             ariaLabelForSelectionColumn="Toggle selection"
             ariaLabelForSelectAllCheckbox="Toggle selection for all items"
             onItemInvoked={this._onItemInvoked}
+            skipViewportMeasures={true}
           />
         </MarqueeSelection>
       </Fabric>

[lalicari]

Perfect, that did it :) thanks for the help!

[lalicari]

I'm assuming the skipViewportMeasures is something that should be used all the time then and not only during testing?

[KevinTCoughlin]

I'm assuming the skipViewportMeasures is something that should be used all the time then and not only during testing?

Given the current code, that prop needs to be present for the DetailsList to render when the containing viewport has height and width of 0, which is the case in your test scenario (no browser viewport).

@ThomasMichon had brought this to my attention as an area that we should revise before I went on vacation. It causes some strange edge-case bugs in SharePoint.

The thought was to:

1. Remove the conditional render in withViewport
2. Do not pass the viewport if both width and height are 0, but pass undefined and have DetailsList handle that case.

I'll revisit that change this week.

[lalicari]

Awesome, thanks!! 😃

[msft-github-bot]

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

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site

[msft-github-bot]

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

Handy links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site