react-gauge-chart
react-gauge-chart copied to clipboard
Gauge re-initialized whenever parent re-renders (needle animates from 0)
I don't know why this is a problem only I appear to be seeing, but when the parent of a GaugeChart component re-renders due to prop changes, the GaugeChart needle resets to 0 and then animates to its current value, causing it to oscillate wildly when there are frequent prop changes. This happens even if the props passed to GaugeChart don't change (the parent renders other components as well). I'm using useMemo
to ensure the props to GaugeChart like arcsLength
and colors
don't change.
I've verified (using an onEffect
hook) that the parent is only re-rendering, and not being unmounted and re-mounted.
I did a bit of debugging in the GaugeChart code, and I was able to see that this call to initChart()
with no parameters inside the useLayoutEffect
hook was being called every time the parent's props changed. The dependencies listed for the useLayoutEffect
hook include the whole props
object for the GaugeChart component - doesn't that mean that the callback passed to useLayoutEffect
will be re-invoked every time the GaugeChart props change?
I was able to stop the behaviour in my application by rolling back to [email protected]. The introduction of the useLayoutEffect
hook with props
in its dependencies seems to have happened with merge request #74.
@stevenhankin
I noticed the same and other issues. I'll be pushing a merge request that cleans this up a lot. The simple fix for this is to memoise the gauge props used by the useLayoutEffect like
const memoProps = React.useMemo(() => props, [JSON.stringify(props)])
And use memoProps instead of props. I would recommend pulling out just the create gauge props and memoising that instead of all the props though.
Also, the useDeepCompareMemo fn seems broken too. I believe line 10 should be if (!isDeepEquals(...
. Currently it's updating the memoised deps if equal?
https://github.com/Martin36/react-gauge-chart/blob/master/src/lib/GaugeChart/customHooks.js#L10
Did you ever get round to doing a PR for this @bjbrewster? 🤞
Hello,
I didn't create a PR but I did finish cleaning it up for the project I am on. I will commit to creating a PR within a few days. Please pester me again if I don't, lol!.
Cheers
On Thu, Feb 17, 2022 at 8:19 PM Ross @.***> wrote:
Did you ever get round to doing a PR for this @bjbrewster https://github.com/bjbrewster? 🤞
— Reply to this email directly, view it on GitHub https://github.com/Martin36/react-gauge-chart/issues/81#issuecomment-1042789852, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANJAYZYWIS25EEKEKOP5X3U3TDTXANCNFSM5CPV7TNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID: @.***>
@bjbrewster Is this merged ?
@bjbrewster any (good) news regarding the PR?
I'm actively working on this and have been testing it lots on our website. I will push this in the next few days. Please bare with me. I've added some tests to make sure my refactoring doesn't break anything. I would appreciate a few people trying it out after I've pushed it. I will aim to push by the end of this weekend.
I'm actively working on this and have been testing it lots on our website. I will push this in the next few days. Please bare with me. I've added some tests to make sure my refactoring doesn't break anything. I would appreciate a few people trying it out after I've pushed it. I will aim to push by the end of this weekend.
@bjbrewster let me know once you are done, I will do some tests.
I'm guessing you're still doing tests and haven't made the PR? This single feature underwent more testing than Cyberpunk 2077 😂😂
@bjbrewster Any chance you can create a pull request for your fixes? Is there anything we can do to assist?
Hello. I'm very sorry life got in the way this last year. I have a week off next week for Xmas so I will push what I've done. It's almost a full rewrite. I tried to redo it in smaller commits but it took too long.
On Fri, 9 Dec 2022, 6:31 am Michael Curry, @.***> wrote:
@bjbrewster https://github.com/bjbrewster Any chance you can create a pull request for your fixes??
— Reply to this email directly, view it on GitHub https://github.com/Martin36/react-gauge-chart/issues/81#issuecomment-1343316995, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANJAY276RTWEPV46VDPPS3WMJAQNANCNFSM5CPV7TNQ . You are receiving this because you were mentioned.Message ID: @.***>
Hello everyone, I have grown a great interest in this exceptional project and have made several modifications on my own. One of them includes a complete overhaul of the rendering process, which potentially resolves the issues you mentioned (and also #126, #44 (I'm planning to make some new types), #31, #27, #100, #57 and #122). Instead of submitting a pull request, I decided to create a new repository due to the disruptive changes and crazy experiments I'm still doing, feel free to check it out and test the modifications, let me know what u find.
You can find the repository here: https://github.com/antoniolago/react-gauge-component
Cheers to Martin36 and all collaborators for creating this great project, not easy to find a free and nice gauge component.