sentry icon indicating copy to clipboard operation
sentry copied to clipboard

ref(dashboards): Use `BigNumberWidgetVisualization` in Dashboards

Open gggritso opened this issue 1 year ago • 1 comments

This is Step 1 of using BigNumberWidget inside Dashboards. This change uses the innards of BigNumberWidget as the innards of WidgetCardChart.

A few notes:

  • in BigNumberWidget, the thresholds indicator is next to the value, not next the heading. In this PR, I make that change in Dashboards, too
  • in BigNumberWidget "expanding" numbers isn't supported, but isn't needed because all numbers always show the expanded value on hover. In this new version, modal, regular, and mobile numbers all show up the same. Let me know if this causes a problem!
  • I added a bit more error checking in a few places, and some small layout improvements

gggritso avatar Oct 16 '24 22:10 gggritso

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ds/widgets/bigNumberWidget/thresholdsIndicator.tsx 0.00% 2 Missing :warning:
static/app/views/dashboards/widgetCard/chart.tsx 83.33% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #79246      +/-   ##
==========================================
+ Coverage   70.30%   78.33%   +8.03%     
==========================================
  Files        7137     7123      -14     
  Lines      314594   314338     -256     
  Branches    51362    51332      -30     
==========================================
+ Hits       221167   246249   +25082     
+ Misses      86521    61642   -24879     
+ Partials     6906     6447     -459     

codecov[bot] avatar Oct 16 '24 22:10 codecov[bot]

@edwardgou-sentry ah fair points all around. The reason for Thresholds is that it's a facade over the existing type. I'm intending to change/update the existing one but that requires updating the serializer which I haven't had a chance to do yet. For that reason I'm not re-using the enum or any other part of ThresholdsConfig for now, it's hidden behind Thresholds while I work on things

I take your point though, I'll come back and clean this stuff up sometime soon (probably when we start supporting thresholds in other widget types)

gggritso avatar Oct 23 '24 18:10 gggritso