Skip to content

fix(dashboards): Change widget legend series name delimiter#79760

Merged
nikkikapadia merged 1 commit into
masterfrom
fix/dashboards/series-name-delimiter
Oct 25, 2024
Merged

fix(dashboards): Change widget legend series name delimiter#79760
nikkikapadia merged 1 commit into
masterfrom
fix/dashboards/series-name-delimiter

Conversation

@nikkikapadia

@nikkikapadia nikkikapadia commented Oct 25, 2024

Copy link
Copy Markdown
Member

Two things (kind of related)

  • Originally the series name delimiter was : however apparently there are series names that have : in the name 🤩 so the widget legend was not working. I changed it to ;. Also set this as a constant in the code so changing it requires a couple of line changes instead of searching through the file.
  • Also just realized the tooltip was showing the modified timeseries name that we use to control the legend hence I put a name formatter for the tooltip to account for that

Sidenote: if there's any other bugs you notice let me know. Planning to EA on Monday

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Oct 25, 2024
@codecov

codecov Bot commented Oct 25, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 72.72727% with 3 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/widgetCard/chart.tsx 50.00% 2 Missing ⚠️
...pp/views/dashboards/widgetLegendSelectionState.tsx 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #79760      +/-   ##
==========================================
+ Coverage   73.30%   78.44%   +5.13%     
==========================================
  Files        7143     7143              
  Lines      315972   315983      +11     
  Branches    43487    43493       +6     
==========================================
+ Hits       231635   247875   +16240     
+ Misses      77862    61774   -16088     
+ Partials     6475     6334     -141     

@nikkikapadia nikkikapadia marked this pull request as ready for review October 25, 2024 15:55
@nikkikapadia nikkikapadia requested a review from a team October 25, 2024 15:55
const decodedSeriesName = seriesName
? WidgetLegendNameEncoderDecoder.decodeSeriesNameForLegend(seriesName)
: seriesName;
const aggregateName = decodedSeriesName?.split(':').pop()?.trim();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not entirely sure if this line is related to your change or not, but does the ':' also have to get replaced with your delimiter here?

Also, are we using , in any special way? I saw something about a series list delimiter. If you group by multiple values those groupings (e.g. group by organization_slug and project) and you get a legend item like org_slug,project iirc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this : is not related to the change, that has to do with other legend names :)

also , is fine because we're encoding all the series names before using that delimiter so it doesn't get messed up.

@nikkikapadia nikkikapadia merged commit 3469fde into master Oct 25, 2024
@nikkikapadia nikkikapadia deleted the fix/dashboards/series-name-delimiter branch October 25, 2024 16:11
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants