Skip to content

Conversation

@elizabetdev
Copy link
Contributor

Closes HDX-1954.

This PR improves the visibility of the search bar chart loading state by graying out the chart and adding a "Loading" text overlay.

search-results-loading.mov

@changeset-bot
Copy link

changeset-bot bot commented Sep 18, 2025

🦋 Changeset detected

Latest commit: 8af74ce

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 Minor
@hyperdx/api Minor

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 Sep 18, 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 Nov 4, 2025 5:46pm

@elizabetdev elizabetdev changed the title fix: Enhance pulse animation effect for loading state fix: Make search bar chart loading state more obvious Sep 18, 2025
@MikeShi42 MikeShi42 requested review from a team and dhable and removed request for a team September 18, 2025 22:50
@teeohhem teeohhem self-requested a review September 20, 2025 01:46
@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2025

E2E Test Results

All tests passed • 39 passed • 3 skipped • 287s

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

View full report →

@MikeShi42
Copy link
Contributor

I think the loading indicator is pretty aggressive in live tail since it'll continually refresh/flash - is there a way we can make it more subtle during live tail? (ex. we disable loading indicators in the search results table for live tail for this same reason)

@elizabetdev
Copy link
Contributor Author

@MikeShi42 I disabled the bar chart loading state during auto-refresh in live tail, but kept it when a user adds a filter. That added a bit of code complexity, though I left comments in the code. It also got me wondering if this is really the right solution, or if we should rethink loading states overall. What do you think? Should we stick with this approach or explore a better pattern?

@claude
Copy link

claude bot commented Nov 4, 2025

PR Review - Search Bar Chart Loading State

Critical Issues:

DBSearchPage.tsx - Memory Leak Risk:

  • ❌ Line 873: Previous timeout not cleared before setting new one in debouncedSubmit → Add clearTimeout(filteringTimeoutRef.current) before line 873 (you clear it at line 870-872 but then immediately set without checking again)

DBSearchPage.tsx - Race Condition:

  • ⚠️ Lines 880, 873: setIsFiltering(true) cleared after 1500ms timeout regardless of actual query completion → If query takes >1.5s, loading state disappears prematurely. Should tie to actual query completion state.

Code Quality Issues:

  • ⚠️ Line 873: Hardcoded magic number 1500 → Extract to named constant FILTERING_TIMEOUT_MS
  • ⚠️ HDXMultiSeriesTimeChart.tsx line 548: "Loading..." text may lack contrast on light backgrounds → Consider adding semi-transparent background or stronger text styling

Suggestion:

Consider using the actual query loading state from TanStack Query instead of timeout-based approach for more reliable UX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants