-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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(ui): Add release bubbles support to TimeSeriesWidgetVisualization
#85270
base: master
Are you sure you want to change the base?
Conversation
❌ 13 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
9f72f97
to
3a6a222
Compare
45793f5
to
1af0aea
Compare
id
property to eCharts bar/line seriesTimeSeriesWidgetVisualization
static/app/views/dashboards/widgets/timeSeriesWidget/timeSeriesWidgetVisualization.tsx
Outdated
Show resolved
Hide resolved
Bundle ReportChanges will increase total bundle size by 51.37kB (0.12%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: app-webpack-bundle-array-pushAssets Changed:
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
Files in
|
1d3784f
to
05548de
Compare
e4c13a3
to
77d8a89
Compare
<ReleaseSeries | ||
start={start} | ||
end={end} | ||
queryExtra={undefined} | ||
period={period} | ||
utc={utc} | ||
projects={projects} | ||
environments={environments} | ||
> | ||
{({releases}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need releases to show on the "mini" charts instead of only fullscreen. This sucks because it will make a request for releases per widget.
TODO: Change to react-query + hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sucks because it will make a request for releases per widget
Will it? I updated ReleaseSeries
a while back to share data between every instance of the component, so they shouldn't be making any extra requests. If that's not working, I can fix it! The hook is better though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have been misinterpreting the requests I was seeing
static/app/views/insights/common/components/insightsTimeSeriesWidget.tsx
Outdated
Show resolved
Hide resolved
04f60f2
to
eeb62f9
Compare
eeb62f9
to
a39025f
Compare
apologies ahead of time @gggritso |
Replace the old `<ReleaseSeries>` renderer and use a simple hook to fetch Release Stats ref #85779
This is a draft PR to ensure we can manipulate echarts the way we want to. What we are trying to do instead of drawing a single line per release is to group releases up into time buckets and show them as a bubble below the main chart. This demonstrates the ability to do this as well as: * shading the relevant area on chart when hovering over bubble * highlighting relevant bubble when moving mouse around the chart * click handler for the bubble * tooltips function as expected between release bubbles and main chart
a39025f
to
7af5fcc
Compare
Adds release bubbles to

<TimeSeriesWidgetVisualization
Clicking on a bubble opens up a drawer.

requires #86076
ref: #85775