Skip to content

style(activists): keep showing old results when query changes#322

Merged
alexsapps merged 2 commits intomainfrom
alex/activists-table-flicker
Mar 11, 2026
Merged

style(activists): keep showing old results when query changes#322
alexsapps merged 2 commits intomainfrom
alex/activists-table-flicker

Conversation

@alexsapps
Copy link
Copy Markdown
Collaborator

@alexsapps alexsapps commented Mar 7, 2026

Summary by CodeRabbit

  • Improvements
    • Stabilized table layout during background refreshes to prevent column/sort shifts while placeholder data displays.
    • Improved loading UX: shows a bottom fixed spinner banner "Loading updated results..." and dims content to indicate stale/placeholder data.
    • Disabled "Load more" while placeholder data is active to avoid redundant fetches.
  • Refactor
    • Table component now accepts a flag to indicate stale/placeholder state for consistent visual behavior.

@alexsapps alexsapps requested a review from jakehobbs as a code owner March 7, 2026 04:28
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 7, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Preserves table column/sort state during placeholder refetches on the activists page, surfaces a stale visual state to the table, shows a bottom "Loading updated results..." banner while placeholder data is active, and suppresses load-more behavior until fresh data arrives.

Changes

Cohort / File(s) Summary
Activists page (placeholder handling & load control)
frontend-v2/src/app/(authed)/activists/activists-page.tsx
Adds settledTableState to cache selected columns/sort during placeholderData, reuses previous query results for placeholder rendering, computes tableColumns/tableSort from settled/current state, passes isStale={isPlaceholderData} to the table, shows a fixed bottom loading banner with Loader2, and prevents the infinite LoadMoreTrigger from firing while placeholder data is active.
Activist table (stale styling & prop)
frontend-v2/src/app/(authed)/activists/activists-table.tsx
Introduces optional isStale?: boolean prop (default false) and applies opacity-60 with transition-opacity to the desktop table container and mobile cards when isStale is true; updates component signature and prop interface.

Sequence Diagram(s)

sequenceDiagram
  participant Page as ActivistsPage
  participant Query as useInfiniteQuery
  participant Table as ActivistTable
  participant LoadMore as LoadMoreTrigger
  participant Banner as LoadingBanner

  Page->>Query: fetch / refetch (infinite)
  Query-->>Page: returns placeholderData (isPlaceholderData = true)
  Page->>Page: retain settledTableState (columns, sort)
  Page->>Table: render with tableColumns/tableSort, isStale=true
  Page->>LoadMore: suppress trigger (do not load next page)
  Page->>Banner: show fixed "Loading updated results..." with spinner
  Query-->>Page: returns final fresh data (isPlaceholderData = false)
  Page->>Page: update settledTableState with new selection/sort
  Page->>Table: render fresh data with isStale=false
  Page->>LoadMore: re-enable trigger
  Page->>Banner: hide loading banner
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • jakehobbs

Poem

Hop, hop—I pause and wait,
Columns kept in tidy state,
A tiny spinner hums below,
I dim my fur till fresh rows show,
Then leap! New data, off I go 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: preventing visible flicker by keeping old results displayed while new query results load.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch alex/activists-table-flicker

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend-v2/src/app/`(authed)/activists/activists-page.tsx:
- Around line 290-302: The placeholderData is causing stale cursors to be used
by getNextPageParam while the query is in isPlaceholderData state; fix this by
tagging the placeholder response with a sentinel (e.g., add a boolean property
like __isPlaceholder = true on the object returned from placeholderData) and
then update getNextPageParam to detect lastPage.__isPlaceholder and return
undefined when set (thereby disabling next-page pagination while placeholder
data is displayed); also ensure any UI code calling fetchNextPage or using
isPlaceholderData checks that __isPlaceholder is not true before triggering
load-more.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5b415983-05b7-48f5-a202-7cf1418445ca

📥 Commits

Reviewing files that changed from the base of the PR and between 63c1b4b and 1c697ff.

📒 Files selected for processing (2)
  • frontend-v2/src/app/(authed)/activists/activists-page.tsx
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx

Comment thread frontend-v2/src/app/(authed)/activists/activists-page.tsx
@alexsapps alexsapps force-pushed the alex/activists-table-flicker branch from 1c697ff to 30505c2 Compare March 7, 2026 04:54
@alexsapps alexsapps force-pushed the alex/activists-table-flicker branch from 30505c2 to 060d256 Compare March 11, 2026 04:21
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@frontend-v2/src/app/`(authed)/activists/activists-table.tsx:
- Line 90: When isStale is true the table headers remain interactive and can
change the stale sort/visibleColumns; make header controls inert while isStale
to prevent clicks from mutating stale state. Update the header rendering/handler
logic (the component that reads isStale, visibleColumns, and sort and attaches
the click handler used for header buttons) to short-circuit pointer events and
onClick behavior when isStale is true (e.g., disable the sorting button and
ignore click events), and ensure the same guard is applied in the codepaths
referenced around isStale and lines handling visibleColumns/sort so rapid clicks
cannot compute a new sort from stale state.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6bcbab49-2f29-4024-8dc0-35745ef0a19a

📥 Commits

Reviewing files that changed from the base of the PR and between 30505c2 and 060d256.

📒 Files selected for processing (2)
  • frontend-v2/src/app/(authed)/activists/activists-page.tsx
  • frontend-v2/src/app/(authed)/activists/activists-table.tsx

Comment thread frontend-v2/src/app/(authed)/activists/activists-table.tsx
@alexsapps alexsapps merged commit b737d52 into main Mar 11, 2026
1 of 2 checks passed
@alexsapps alexsapps deleted the alex/activists-table-flicker branch March 11, 2026 04:38
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.

1 participant