Fix: Prevent NaN error in Campaign Overview Stat Widget #7698
Conversation
| @@ -93,7 +93,7 @@ const StatWidget = ({label, values, description, formatter = null}) => { | |||
| </header> | |||
| <div className={styles.statWidgetAmount}> | |||
| <DisplayText> | |||
| {formatter?.format(values[0]) ?? values[0]} | |||
| {!isNaN(values[0]) ? formatter?.format(values[0]) ?? values[0] : (<span> </span>)} | |||
There was a problem hiding this comment.
@kjohnson can you help us understand what could cause one of these values to be considered NaN? Is it an issue with data still loading or is it possible for a value to resolve as a non number?
There was a problem hiding this comment.
The isssue here is that the data is loading. The endpoint would only return a numeric value.
There was a problem hiding this comment.
Okay, If that's the case then I feel it would be more sensible for the stat widget to have a loading state (or conditionally rendered all together). You could then add a css class for the loading state to be styled.
Without the loading state we are making the assumption the value is always resolved when passed to the component, which is not true here - raising questions of why it could be a non number.
There was a problem hiding this comment.
This passes a non-breaking space as a placeholder if the value is not provided, which is during loading. While not a proper loading state, this does cut down on the amount of visual change while loading. I think it would be more distracting for a row of widgets to flash loading spinners or something.
Note that I did update the check from a NaN check to an undefined check, see d3077e7.
There was a problem hiding this comment.
Regardless of the design, what is the reason for not having a loading state in the logic?
Loading states tell us when our data is resolved and ready to be used. They are useful in the application's logic for conditionally rendering data. In this scenario we have data that is not resolved yet but we are attempting to render it regardless. There's lots of ways we can go about this visually but to me the important part is we are being intentional about our data loading and rendering.
There was a problem hiding this comment.
About the design for the "loading state", maybe we can use something similar to the give-skeleton class.
Here are a couple of links for reference:
And here is the screenshot with the class in action:
There was a problem hiding this comment.
Or even better, we can use a package like this one: https://www.npmjs.com/package/react-loading-skeleton
There was a problem hiding this comment.
I really don't think this needs a special loading state. It is a simple widget which shows a number returned by the server. We aren't running exspensive queries and we aren't loading tables of data. There isn't a fancy display to be stubbed by a skeleton.
This PR prevents a flashed NaN and additionally prevents a content shift when the data is loaded.
Resolves GIVE-2031
Description
This PR updates the
<StatWidget>component to prevent aNaNerror while fetching data.Additionally, a non-breaking space is used to prevent content shifting.
Visuals
Testing Instructions
Pre-review Checklist
@unreleasedtags included in DocBlocks