feat(insights): Support comparison to previous period in Bold Number

[Twixes]

Changes

Follow-up to #11188 adding support for "Compare to previous time period" to the Number display type. Brings the feature fully in line with #6172 (comment).

Query editor

Dashboard

[Twixes]

Now that I think about it, color coding "up" and "down" may not be the best idea. For instance, if we did a "Churned customers this week" insight, the lower the better. Removed the green and red.

[pauldambra]

[pauldambra]

Ah, just one case that should be caught... page load is infinity different for me

[Twixes]

Hmm, this is intentional – I don't have a good idea of a better thing to present when the previous period's value is zero and the current one is non-zero. Infinity seems pretty logical in this case.

[pauldambra]

I'd have expected no comparison, or a note none is possible.

Maybe one to check with @clarkus but no need to block

[clarkus]

We should remove the success color options from the rate of change. As an example, if the value being described is a churn metric or something that improves as it is reduced, then we shouldn't indicate that a negative case. We can't be sure that the status colors will always be applied in a clear way, so I'd recommend removing them and instead go with an unbiased neutral color.

[clarkus]

Hmm, this is intentional – I don't have a good idea of a better thing to present when the previous period's value is zero and the current one is non-zero. Infinity seems pretty logical in this case.

Are we comparing a zero value in the previous range or a null value? If there is data for that range and the values are 0, then showing the actual rate of change with a numerical value is good. If there is no data (say the event didn't exist in that time frame) then no comparison is really available. In either case, the infinity symbol is pretty confusing - not sure how it'd be interpreted by users.

[Twixes]

We should remove the success color options from the rate of change.

Yes, sorry I've not updated the screenshots, but that's exactly what I meant in my previous comment too:
#11225 (comment)

Are we comparing a zero value in the previous range or a null value?

This would be a useful distinction to be made by our API, but unfortunately the result of there being no events and e.g the result of an aggregation on some property are both 0. Mathematically, "Up ∞%" seems like an appropriate result.

[Twixes]

Actually I took one more glance at Paul's screenshot and realized we do return null on aggregation-based series. So, to absolutely nail quality here, I added handling of that case too:

---

[pauldambra]

🦺

Addressed

[Twixes]

I think all feedback should be addressed now, so I'll merge while you're off Chris, but of course let me know if there's anything else.