Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(dashboards): Add releases overlay on dashboard charts #77947

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

nikkikapadia
Copy link
Member

@nikkikapadia nikkikapadia commented Sep 23, 2024

Added the releases overlay onto dashboard widget charts (area and line charts). The releases are initially toggled off and can be toggled on from the legend. Closes #54460

image

image

image

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Sep 23, 2024
Copy link

codecov bot commented Sep 23, 2024

Codecov Report

Attention: Patch coverage is 39.28571% with 17 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
static/app/views/dashboards/detail.tsx 25.00% 12 Missing ⚠️
...rds/widgetBuilder/buildSteps/visualizationStep.tsx 50.00% 4 Missing ⚠️
static/app/views/dashboards/widgetCard/chart.tsx 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #77947       +/-   ##
===========================================
+ Coverage   64.78%   78.09%   +13.31%     
===========================================
  Files        6992     6994        +2     
  Lines      310032   309746      -286     
  Branches    50743    50722       -21     
===========================================
+ Hits       200852   241911    +41059     
+ Misses      97204    56123    -41081     
+ Partials    11976    11712      -264     

Copy link
Member

@gggritso gggritso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! Two questions:

  1. There are some notes scattered about loading release early and passing them through the Dashboard component to avoid separate calls from inside components. The code suggests that calls to fetch releases are memoized and shared between components. Is that true? If yes, I think it'd be easier and better to wrap the line and area charts directly in ReleaseQueries and avoid the brouhaha of passing everything through, due to point 2
  2. Does loading the releases take a while? If so, I think it'd be better to lazy load releases: only load them if someone selects the releases series on a chart. Unless releases are auto-selected in which case ignore me?

@nikkikapadia
Copy link
Member Author

nikkikapadia commented Sep 23, 2024

Very cool! Two questions:

  1. There are some notes scattered about loading release early and passing them through the Dashboard component to avoid separate calls from inside components. The code suggests that calls to fetch releases are memoized and shared between components. Is that true? If yes, I think it'd be easier and better to wrap the line and area charts directly in ReleaseQueries and avoid the brouhaha of passing everything through, due to point 2
  2. Does loading the releases take a while? If so, I think it'd be better to lazy load releases: only load them if someone selects the releases series on a chart. Unless releases are auto-selected in which case ignore me?

So I thought about wrapping those charts directly as well but I think if there's a lot of line and area charts it will take a while to load them all. Loading releases doesn't take that long but many simultaneous calls does. I like the idea of lazy loading the releases though (releases are not auto-selected so this could work). I'll look more into that and we can discuss more tomorrow :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboards - Ability to show releases in Line & Area Charts
2 participants