Fluentui: DetailsList dragDropEvents doesn't update when changed

[goodoldneon]

Environment Information

• __Package version(s)__: 6.117.1
• __Browser and OS versions__: Chrome 71.0.3578.98 and macOS 10.14.2

Please provide a reproduction of the bug in a codepen:

https://codepen.io/goodoldneon/pen/BvJBEq

Actual behavior:

Changing the dragDropEvents prop doesn't change drag/drop behavior. Drag/drop is "stuck" on the initial prop.

Expected behavior:

Changing the dragDropEvents prop should change drag/drop behavior.

Priorities and help requested:

Are you willing to submit a PR to fix? No (don't know the best way to fix this)

Requested priority: Normal

Products/sites affected: None (product is currently just a POC).

Initial Investigation

• In the constructor, this._dragDropHelper = props.dragDropEvents is set. So any future updates to props.dragDropEvents are not used.
• The properties on props.dragDropEvents are methods, so I don't know what diffing logic should be put in componentDidUpdate(). Checking whether the object reference changed isn't a good approach, since most users will probably be assigning an object to props.dragDropEvents every render.

[cliffkoh]

Hi @goodoldneon, I discussed this with @aditima who is the original component author and she agrees with me that we don't think there is an issue with your proposed solution.

I think checking object reference and updating this._dragDropHelper if dragDropEvents has changed is perfectly fine. @aditima stated that the component doesn't do this at the moment because she didn't think anyone would be changing the event - but given that there's a need, your proposed fix is reasonable :)

[goodoldneon]

Thanks!

[cliffkoh]

Hey @goodoldneon ,

I took a look at this and realized that your codepen does not actually illustrate this issue. Additionally, when writing unit tests for this, I can't actually repro the reported issue. I will create a PR that adds a unit test to illustrate. Maybe there is something wrong with the way I am writing my unit test but in the first place, it would be great for an actual repro of the issue.

[goodoldneon]

@cliffkoh

I accidentally overwrote the pen; sorry about that. The pen now demonstrates the issue.

[cliffkoh]

Appreciate the codepen. I will look into this.

[msft-github-bot]

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

Handy links:

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