feat(widget-viewer): Render with new visualization widget #105687
Conversation
github-actions
bot
added
the
Scope: Frontend
| (() => { | ||
| const plottables = transformLegacySeriesToPlottables( | ||
| seriesData, | ||
| seriesResultsType, | ||
| widget | ||
| ); | ||
|
|
||
| if (plottables.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <TimeSeriesWidgetVisualization | ||
| plottables={plottables} | ||
| releases={releases} | ||
| showReleaseAs="bubble" | ||
| onZoom={(_event, chart) => { | ||
| onZoom(_event, chart); | ||
| setChartUnmodified(false); | ||
| }} | ||
| onLegendSelectionChange={selected => { | ||
| widgetLegendState.setWidgetSelectionState(selected, widget); | ||
| trackAnalytics('dashboards_views.widget_viewer.toggle_legend', { | ||
| organization, | ||
| widget_type: widget.widgetType ?? WidgetType.DISCOVER, | ||
| display_type: widget.displayType, | ||
| }); | ||
| }} | ||
| legendSelection={widgetLegendState.getWidgetSelectionState(widget)} | ||
| pageFilters={modalSelection} | ||
| /> | ||
| ); | ||
| })() |
There was a problem hiding this comment.
This IIFE is kind of gross, should I pull it out into an actual component? 😅
The widget viewer has this pattern of:
A: I have memoized data, I'll just render the chart without the querying components
B: I don't have data, so I'll render "higher" up the component tree to get the querying
|
Drafting this for now, I think there might be regressions in the confidence footers for spans widgets if I do this. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| renderErrorMessage={errorMessage => | ||
| errorMessage ? <div>{errorMessage}</div> : null | ||
| } | ||
| /> |
There was a problem hiding this comment.
Zoom in fresh data path causes data-axis mismatch
When using the fresh data path with VisualizationWidget, zooming causes a mismatch between the chart's x-axis and the displayed data. The VisualizationWidget doesn't pass pageFilters or onZoom to the inner TimeSeriesWidgetVisualization, so when the user zooms, the default behavior updates the URL with the new date range. However, VisualizationWidget continues to fetch data using modalSelection (which isn't updated by URL changes - only by the onZoom callback or browser back navigation). The result is that after zooming, TimeSeriesWidgetVisualization reads the new date range from usePageFilters() for its x-axis, but displays data that was fetched for the old date range.
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
1 participant
Converts the widget viewer modal to render with the new visualization timeseries widgets. I just prompted cursor for this and it passes all of the existing tests (minus where I had to update some mock values because we handle empty data differently) and I think the implementation is pretty minimal to get the widget viewer modal to work.