Skip to content

fix(tracemetrics): Remove sorting styles from embedded table#114588

Merged
nsdeschenes merged 2 commits intomasterfrom
nd/LOGS-726/fix-trace-metrics-remove-table-header-styles-from-waterfall-table
May 1, 2026
Merged

fix(tracemetrics): Remove sorting styles from embedded table#114588
nsdeschenes merged 2 commits intomasterfrom
nd/LOGS-726/fix-trace-metrics-remove-table-header-styles-from-waterfall-table

Conversation

@nsdeschenes
Copy link
Copy Markdown
Contributor

This PR removes the sorting styles from the embedded application metrics table. As well I added in a header value for metric type so now it'll render "Type" for that column rather than being just blank.

Closes LOGS-726

@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 1, 2026

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 1, 2026
nsdeschenes and others added 2 commits May 1, 2026 14:58
When the metrics samples table is embedded in the trace waterfall view,
header columns are not interactive. Disable sorting in embedded mode so
headers render as plain divs without hover effects or button semantics.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
Add 'Type' as the header label for the METRIC_TYPE column in the
embedded metrics samples table.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
@nsdeschenes nsdeschenes force-pushed the nd/LOGS-726/fix-trace-metrics-remove-table-header-styles-from-waterfall-table branch from f6bfd5a to 43bd3e8 Compare May 1, 2026 17:58
@nsdeschenes nsdeschenes marked this pull request as ready for review May 1, 2026 17:59
@nsdeschenes nsdeschenes requested a review from a team as a code owner May 1, 2026 17:59
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 43bd3e8. Configure here.

const label = getFieldLabel(field);
const hasPadding = field !== VirtualTableSampleColumnKey.EXPAND_ROW;
const canSort = SORTABLE_SAMPLE_COLUMNS.has(field);
const canSort = !embedded && SORTABLE_SAMPLE_COLUMNS.has(field);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sort indicator still visible in embedded table

Medium Severity

The sort prop is still passed unconditionally to StyledSimpleTableHeaderCell even when embedded is true. The underlying SimpleTable.HeaderCell uses sort to derive isSorted, which renders a sort arrow icon and changes the header text color. So while the click handler is correctly suppressed for embedded mode, the visual sort indicators (arrow + text color) can still appear if query params contain matching sort fields like METRIC_VALUE or TIMESTAMP. The sort prop also needs to be conditioned on !embedded (or on canSort) to fully remove sorting styles.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 43bd3e8. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is fine? just so that users know how they are sorted ...

Copy link
Copy Markdown
Member

@narsaynorath narsaynorath left a comment

Choose a reason for hiding this comment

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

🎉

@nsdeschenes nsdeschenes merged commit ba8761e into master May 1, 2026
71 checks passed
@nsdeschenes nsdeschenes deleted the nd/LOGS-726/fix-trace-metrics-remove-table-header-styles-from-waterfall-table branch May 1, 2026 18:07
cleptric pushed a commit that referenced this pull request May 5, 2026
This PR removes the sorting styles from the embedded application metrics
table. As well I added in a header value for metric type so now it'll
render "Type" for that column rather than being just blank.

Closes LOGS-726

---------

Co-authored-by: Claude Opus 4 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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