feat(insights): Allow picking chart layouts in Insights > Session Health #88502
Conversation
github-actions
bot
added
the
Scope: Frontend
…spots within the same view
static/app/views/insights/common/components/insightsTimeSeriesWidget.tsx
Show resolved
Hide resolved
static/app/views/insights/sessions/charts/newAndResolvedIssueChart.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/chartPlacementContext.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/chartSelectionTitle.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/insightLayoutContext.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/insightLayoutContext.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/insightLayoutContext.tsx
Outdated
Show resolved
Hide resolved
static/app/views/insights/sessions/components/insightLayoutContext.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
could the contents of this file be moved into static/app/views/insights/sessions/utils/sessions.tsx ? or in static/app/views/insights/sessions/settings.ts if that makes more sense
There was a problem hiding this comment.
i can go for views/insights/sessions/settings.ts
There was a problem hiding this comment.
ow :(
so i had to move move of the stuff back out of the main insights/sessions/settings.ts file because it was creating a circular dependency that couldn't be resolved. What's left is CHART_TITLES. I also flipped the types to depend on that, so there's no more circular deps anymore.
Basically insights/sessions/settings.ts defines MODULE_TITLE which is needed by the charts, which are imported by settings.ts. So in the end it's best to keep settings.ts simple, with simple values, and the new file has heavy dependencies which eventually also depend on settings itself.
static/app/views/insights/sessions/charts/newAndResolvedIssueChart.tsx
Outdated
Show resolved
Hide resolved
| NewAndResolvedFeedbackChart: t('User Feedback'), | ||
| ReleaseNewIssuesChart: t('New Issues by Release'), | ||
| ReleaseSessionCountChart: t('Total Sessions by Release'), | ||
| ReleaseSessionPercentageChart: t('Release Adoption'), |
There was a problem hiding this comment.
should we just rename this chart to ReleaseAdoptionChart?
There was a problem hiding this comment.
i took all the component names for the keys in here, so everything is 1:1 except NewAndResolvedFeedbackChart which is going away.
ryan953
force-pushed
the
ryan953/insights-session-change-charts
branch
from
87a5844 to
a528cba
Compare
This PR implements a dropdown to replace the standard title within each chart on the Session Health page. Using the dropdown users can pick which charts appear in which spots inside the page.
Each page (Frontend, Mobile) has 5 slots (i removed some extra ones) where charts can go, therefore we need to have at least 6 options in the dropdown for there to be one 'unused' option that's a valid choice.
Currently the sort-order of the items in the dropdown is alphabetical by component name (not by the label that appears on the screen). So we need to revisit that and be intentional about what order we group/suggest things.
It's all similar but different to how charts were picked, rendered and saved within the framework of the 'old' insights charts. It should be extensible to support other types of insights pages and charts. But we should strive to async load more of the chart types instead of importing everything into settings.tsx