Skip to content

Conversation

@pulpdrew
Copy link
Contributor

Closes HDX-2581

Dashboard filter options are now sorted alphabetically.

@changeset-bot
Copy link

changeset-bot bot commented Oct 15, 2025

🦋 Changeset detected

Latest commit: 973d0fe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Oct 15, 2025 7:14pm

@pulpdrew pulpdrew force-pushed the drew/dashboard-filter-fixes branch from 84f238d to 1eed266 Compare October 15, 2025 17:15
@github-actions
Copy link
Contributor

github-actions bot commented Oct 15, 2025

E2E Test Results

All tests passed • 25 passed • 3 skipped • 234s

Status Count
✅ Passed 25
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

View full report →

@claude
Copy link

claude bot commented Oct 15, 2025

PR Review: Sort dashboard filter values

Summary

This PR adds alphabetical sorting to dashboard filter dropdown options, improving UX by making values easier to find. The implementation is clean and straightforward.

✅ Strengths

  1. Clean implementation: The refactoring improves code clarity by extracting the data transformation into a named variable
  2. Proper sorting: Using JavaScript's native .sort() for alphabetical ordering
  3. Changeset included: Follows the project's versioning workflow correctly
  4. Small, focused change: Clear single responsibility

🔍 Code Quality Observations

Good:

  • Follows functional programming patterns with method chaining
  • Maintains proper null handling with selectValues || []
  • Type safety preserved with String(value) conversion

⚠️ Potential Issues & Considerations

1. Sorting Behavior with Mixed Data Types

The current implementation converts all values to strings before sorting:

keys?.[0]?.value
  .map(value => String(value))
  .sort()

Potential issue: For numeric values (e.g., status codes, port numbers), this produces lexicographic ordering rather than numeric ordering:

  • Current: ['1', '10', '100', '2', '20']
  • Expected (for numbers): ['1', '2', '10', '20', '100']

Recommendation: Consider locale-aware or numeric sorting:

const selectValues = keys?.[0]?.value
  .map(value => String(value))
  .sort((a, b) => a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' }))
  .map(value => ({
    value,
    label: value,
  }));

This provides:

  • Numeric sorting for numeric strings
  • Case-insensitive sorting
  • Locale-aware ordering

2. Performance Consideration

For filters with large value sets (100+ options), the dropdown has limit={20} but sorting happens on the full dataset before limiting. This is fine for most cases, but worth noting.

3. Case Sensitivity

Current .sort() is case-sensitive ('Z' < 'a'). If filters contain mixed-case values, consider case-insensitive sorting.

📝 Test Coverage

No tests were added for this change. While the existing useDashboardFilters.test.tsx is currently skipped (waiting on nuqs upgrade), consider:

  • Adding a comment or ticket for future test coverage
  • Manual testing with various filter value types (strings, numbers, mixed case)

🔒 Security

No security concerns identified.

🎯 Suggestions

Optional enhancement:

const selectValues = keys?.[0]?.value
  .map(value => String(value))
  .sort((a, b) => 
    a.localeCompare(b, undefined, { 
      numeric: true,      // Handle numeric strings naturally
      sensitivity: 'base' // Case-insensitive
    })
  )
  .map(value => ({
    value,
    label: value,
  }));

✅ Overall Assessment

Approve with minor suggestions. The change successfully implements the requested feature and improves code readability. The sorting enhancement suggestion is optional but would provide better UX for numeric filters.

Files reviewed:

  • .changeset/happy-buses-occur.md
  • packages/app/src/DashboardFilters.tsx

@pulpdrew pulpdrew requested review from a team and brandon-pereira and removed request for a team October 15, 2025 19:09
@kodiakhq kodiakhq bot merged commit 2dc0079 into main Oct 15, 2025
8 of 9 checks passed
@kodiakhq kodiakhq bot deleted the drew/dashboard-filter-fixes branch October 15, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants