perf: show loading spinner during navigation#312
Merged
Conversation
📝 WalkthroughWalkthroughAdds a Loading component and a server-side ActivistsSection that prefetches activist names; replaces HydrationBoundary/dehydrate SSR flow with a client Suspense boundary in the page; updates Navbar chapter switching to fetch unconditionally and handle loading/error states. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client Page"
participant Suspense as "React Suspense"
participant Activists as "ActivistsSection (Server)"
participant ApiClient as "ApiClient"
participant API as "External API"
Client->>Suspense: Render ActivistsSection (client)
Suspense->>Activists: Request server component render
Activists->>ApiClient: instantiate (with cookies) & prefetch names
ApiClient->>API: GET /activist-names
API-->>ApiClient: activist names response
ApiClient-->>Activists: return data
Activists-->>Suspense: rendered output (dehydrated state)
Suspense-->>Client: display ActivistsSection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/loading.tsx`:
- Around line 1-12: The Loading component's formatting is not compliant with the
project's code style; run the formatter (make fmt) or apply the project's
Prettier/ESLint formatting rules to this component so the JSX and imports match
the repo style, then commit the updated file; locate the Loading function and
its use of Loader2 to ensure only formatting changes are made (no functional
edits) before pushing.
alexsapps
force-pushed
the
alex/frontend-v2-nav-loading-indicator
branch
from
98283c6 to
5f60deb
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/example-server-component/page.tsx`:
- Around line 26-31: ActivistsFallback duplicates the spinner/loading UI;
replace it by reusing the shared Loading component from loading.tsx: import the
existing Loading (or similarly named) component and render it with a label or
children "Loading activists..." instead of a new JSX fallback; if the shared
Loading component doesn't accept custom text, update its signature (e.g., add a
label or children prop) to accept and render the per-page message, preserve
existing accessibility props (aria-hidden, role) and the Loader2 usage inside
the shared component, then remove the ActivistsFallback declaration and any
local references to it.
In `@frontend-v2/src/app/loading.tsx`:
- Around line 5-13: The loader isn't announced to screen readers; update the
outer container (the div that wraps Loader2 and the "Loading page..." span) to
be a polite live region by adding role="status" and aria-live="polite" (and
aria-busy="true" while loading) so assistive tech will announce the change;
ensure the visible <span>Loading page...</span> remains available to screen
readers (or add a visually-hidden copy if you prefer) and keep the Loader2 icon
purely decorative (aria-hidden="true") to avoid duplicate announcements.
In `@frontend-v2/src/components/nav.tsx`:
- Around line 163-166: ChapterSwitcherFallback doesn’t announce the loading
state to assistive tech; update the root element rendered by the
ChapterSwitcherFallback component to include live-region semantics (e.g., add
aria-live="polite" and role="status", and optionally aria-atomic="true") so
screen readers will announce "Loading chapters..." when this transient UI
appears; locate the ChapterSwitcherFallback functional component and add those
attributes to the outer div (the one using buefyStyles['navbar-item']).
- Around line 134-137: The ChapterSwitcher can throw via useSuspenseQuery
(queryFn: apiClient.getChapterList), so wrap the component in a local error
boundary to prevent errors bubbling to the route; either add a React
ErrorBoundary component around <ChapterSwitcher> or use React Query's
<QueryErrorResetBoundary> with an error fallback UI and a reset handler that
calls reset() to retry the query and clears error state; ensure the Suspense
that currently wraps ChapterSwitcher remains for loading, but place the
ErrorBoundary/QueryErrorResetBoundary outside Suspense so fetch errors are
caught and a compact fallback (e.g., retry button or collapsed nav) is rendered
instead of blowing up the navbar.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
frontend-v2/src/app/example-server-component/activists-section.tsxfrontend-v2/src/app/example-server-component/page.tsxfrontend-v2/src/app/loading.tsxfrontend-v2/src/components/nav.tsx
There was a problem hiding this comment.
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/components/nav.tsx`:
- Around line 134-157: The error UI for the chapter list disables
recovery—modify the useQuery call in this component to destructure refetch
(e.g., const { data, isLoading, isError, refetch } = useQuery({ queryKey:
[API_PATH.CHAPTER_LIST], queryFn: apiClient.getChapterList })) and replace the
terminal "Chapters unavailable" block with a retry UI that calls refetch when
clicked (a button labeled "Retry" or "Reload chapters"); keep the existing
role="alert" and provide accessible text/hints, optionally disable the button
while isLoading or show a small spinner using the same buefyStyles navbar-item
container to preserve layout.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
frontend-v2/src/app/example-server-component/page.tsxfrontend-v2/src/app/loading.tsxfrontend-v2/src/components/nav.tsx
Comment on lines
+134
to
157
| const { data, isLoading, isError } = useQuery({ | ||
| queryKey: [API_PATH.CHAPTER_LIST], | ||
| queryFn: apiClient.getChapterList, | ||
| enabled: user.role === 'admin', | ||
| }) | ||
|
|
||
| if (user.role !== 'admin') { | ||
| return null | ||
| if (isLoading) { | ||
| return ( | ||
| <div | ||
| className={buefyStyles['navbar-item']} | ||
| role="status" | ||
| aria-live="polite" | ||
| > | ||
| <span className="text-sm text-neutral-500">Loading chapters...</span> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| if (isError) { | ||
| return ( | ||
| <div className={buefyStyles['navbar-item']} role="alert"> | ||
| <span className="text-sm text-neutral-500">Chapters unavailable</span> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Add a retry path for chapter-list fetch failures.
Line 151 returns a terminal error state (Chapters unavailable) with no recovery action. This blocks chapter switching for admins until a full page reload.
Suggested fix
- const { data, isLoading, isError } = useQuery({
+ const { data, isLoading, isError, refetch } = useQuery({
queryKey: [API_PATH.CHAPTER_LIST],
queryFn: apiClient.getChapterList,
})
@@
if (isError) {
return (
- <div className={buefyStyles['navbar-item']} role="alert">
- <span className="text-sm text-neutral-500">Chapters unavailable</span>
+ <div className={buefyStyles['navbar-item']} role="alert">
+ <span className="text-sm text-neutral-500">Chapters unavailable.</span>
+ <button
+ type="button"
+ onClick={() => void refetch()}
+ className="ml-2 text-sm text-primary underline"
+ >
+ Retry
+ </button>
</div>
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { data, isLoading, isError } = useQuery({ | |
| queryKey: [API_PATH.CHAPTER_LIST], | |
| queryFn: apiClient.getChapterList, | |
| enabled: user.role === 'admin', | |
| }) | |
| if (user.role !== 'admin') { | |
| return null | |
| if (isLoading) { | |
| return ( | |
| <div | |
| className={buefyStyles['navbar-item']} | |
| role="status" | |
| aria-live="polite" | |
| > | |
| <span className="text-sm text-neutral-500">Loading chapters...</span> | |
| </div> | |
| ) | |
| } | |
| if (isError) { | |
| return ( | |
| <div className={buefyStyles['navbar-item']} role="alert"> | |
| <span className="text-sm text-neutral-500">Chapters unavailable</span> | |
| </div> | |
| ) | |
| } | |
| const { data, isLoading, isError, refetch } = useQuery({ | |
| queryKey: [API_PATH.CHAPTER_LIST], | |
| queryFn: apiClient.getChapterList, | |
| }) | |
| if (isLoading) { | |
| return ( | |
| <div | |
| className={buefyStyles['navbar-item']} | |
| role="status" | |
| aria-live="polite" | |
| > | |
| <span className="text-sm text-neutral-500">Loading chapters...</span> | |
| </div> | |
| ) | |
| } | |
| if (isError) { | |
| return ( | |
| <div className={buefyStyles['navbar-item']} role="alert"> | |
| <span className="text-sm text-neutral-500">Chapters unavailable.</span> | |
| <button | |
| type="button" | |
| onClick={() => void refetch()} | |
| className="ml-2 text-sm text-primary underline" | |
| > | |
| Retry | |
| </button> | |
| </div> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend-v2/src/components/nav.tsx` around lines 134 - 157, The error UI for
the chapter list disables recovery—modify the useQuery call in this component to
destructure refetch (e.g., const { data, isLoading, isError, refetch } =
useQuery({ queryKey: [API_PATH.CHAPTER_LIST], queryFn: apiClient.getChapterList
})) and replace the terminal "Chapters unavailable" block with a retry UI that
calls refetch when clicked (a button labeled "Retry" or "Reload chapters"); keep
the existing role="alert" and provide accessible text/hints, optionally disable
the button while isLoading or show a small spinner using the same buefyStyles
navbar-item container to preserve layout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit