refactor: Consolidate charts with BarChart component#7222
Closed
talissoncosta wants to merge 1 commit intofeat/feature-analytics-6067from
Closed
refactor: Consolidate charts with BarChart component#7222talissoncosta wants to merge 1 commit intofeat/feature-analytics-6067from
talissoncosta wants to merge 1 commit intofeat/feature-analytics-6067from
Conversation
…rts SCSS Replaces the two raw-recharts consumers with the shared <BarChart /> component introduced in #7215. Both pages keep their behaviour and visual contract; the JSX collapses substantially. BarChart API additions to support these consumers: - `barSize?: number` — fixed bar width in pixels (OrganisationUsage uses 14px to fit four series per day comfortably) - `verticalGrid?: boolean` (default true) — toggles CartesianGrid's vertical lines (legacy charts hide them) Migrations: - OrganisationUsage.container.tsx (4 metric series stacked, custom display labels via seriesLabels, selection-driven series filter, pre-formatted day axis) - SingleSDKLabelsChart.tsx (per-SDK stacked, palette colour map passed in from parent, MultiSelect-driven SDK filter) - OrganisationUsageMetrics.container.tsx — pre-formats day to 'D MMM' (matches the new chart-side day-as-display-string contract), switches userAgentColorMap from Map to Record (consistent with BarChart's prop after #7215) Cleanup: - Delete web/styles/3rdParty/_recharts.scss entirely — its rules existed solely to style the two legacy charts' tooltips. With both consumers migrated, no consumer of recharts global classNames remains; the new BarChart's ChartTooltip uses Bootstrap utilities + semantic token classes directly. Drop the @import too. - SingleSDKLabelsChart loses unused props (`metricKey`, `colours`) that the migration made redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
bot
added
the
front-end
talissoncosta
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/if required so people know about the feature.Changes
Follow-up to #7215 — migrates the two raw-recharts consumers in the org-level Usage page to the shared
<BarChart />component, retiring the legacy recharts SCSS overrides.What's migrated
OrganisationUsage(chart section)web/components/organisation-settings/usage/OrganisationUsage.container.tsxenvironment_document→ "Environment Document"), selection-driven series, pre-formatted day axisSingleSDKLabelsChartweb/components/organisation-settings/usage/components/SingleSDKLabelsChart.tsxOrganisationUsageMetrics(parent)web/components/organisation-settings/usage/OrganisationUsageMetrics.container.tsxdayto'D MMM'(matches BarChart's day-as-display-string contract);userAgentColorMapswitched fromMap→Record<string, string>for consistency with the BarChart propBarChartAPI additionsBoth consumers needed two props the existing
BarChartdidn't expose:barSize?: number— fixed bar width in pixels (OrganisationUsageuses 14px to fit four series per day comfortably)verticalGrid?: boolean(defaulttrue) — togglesCartesianGrid's vertical lines (legacy charts hide them)seriesLabels(added in #7215) handles the display-name mapping forOrganisationUsage(environment_document→ "Environment Document").Cleanup
web/styles/3rdParty/_recharts.scssentirely — its rules existed solely to style the two legacy charts' tooltips. With both consumers migrated, no recharts global-className consumer remains; the newBarChart'sChartTooltipuses Bootstrap utilities + semantic token classes directly. Drop the@importtoo.SingleSDKLabelsChartloses unused props (metricKey,colours) that the migration made redundant.Net diff
+101 / -312— six files changed, one deleted.Type strictness
OrganisationUsage'scolorMapandseriesLabelsare typed againstRecord<MetricDataKey, string>whereMetricDataKeyis derived from theMETRICSarray ((typeof METRICS)[number]['dataKey']). Adding a new metric toMETRICSforces both maps to be updated — TS errors on missing keys.How did you test this code?
In the app
ENV=local npm run devsdk_usage_chartsflag enabled) → per-SDK stack renders, MultiSelect filter narrows visible series, tooltip shows SDK namesAutomated
npx eslint— 0 errors on touched filesnpm run typecheck— 0 new errors (pre-existinganycluster inuseFeatureAnalytics.tsunchanged; pre-existing errors inOrganisationUsageSideBar/OrganisationUsagePageare unrelated)🤖 Generated with Claude Code