[#398] COMPONENTS - Added reusable issue fetcher pane#429
Open
[#398] COMPONENTS - Added reusable issue fetcher pane#429
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds two client-side React components: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Playground as IssueFetcherPanePlayground
participant Pane as IssueFetcherPane
participant TRPC as TRPC Server
participant DB as Database
User->>Playground: open UI / interact filters
Playground->>Pane: mount / setFilters (props)
Pane->>TRPC: query issues.getAllIssues(filters)
Pane->>TRPC: query roles.getRoleLinks()
TRPC->>DB: fetch issues & roles
DB-->>TRPC: issues & roles
TRPC-->>Pane: issues data, role links
Pane->>Pane: compute blockedParentIds, apply client-side filters, memoize IssueFetcherPaneData
Pane-->>Playground: emit IssueFetcherPaneData (onDataChange) / setIssues
Playground->>User: render issue cards (badges, derived fields)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blade/src/app/_components/issues/issue-fetcher-pane-playground.tsx`:
- Around line 13-15: The header/body currently render as if there are zero
results when fetcherData is null; change the component to treat fetcherData ===
null as "not ready" by gating the header and body rendering on fetcherData !==
null (i.e., only render counts, "No issues match…" messaging, and result UI when
fetcherData is non-null), and ensure any places referencing fetcherData (the
useState [fetcherData, setFetcherData] and the IssueFetcherPaneData-dependent
sections) early-return or show a loading/placeholder until fetcherData is
populated so the pane shows a single coherent loading/error/result state.
In `@apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx`:
- Around line 84-118: The pane currently treats rolesQuery and issuesQuery
independently (rolesQuery, issuesQuery, fetchedIssues, issuesError, isLoading,
refetch, roles, roleNameById, issueErrorMessage) which allows partial renders
when one query fails; instead create a single combined query state: compute
combinedIsLoading = rolesQuery.isLoading || issuesQuery.isLoading, combinedError
= rolesQuery.error ?? issuesQuery.error (or aggregate both), and a
combinedRefetch that calls both refetch functions (e.g.
Promise.all([rolesQuery.refetch(), issuesQuery.refetch()])); use
combinedIsLoading/combinedError/combinedRefetch for all rendering, error
reporting, and the parent refresh API so the component only renders "ready" when
both queries succeed and the Refresh button retries both queries.
- Around line 219-352: Add id/htmlFor associations for each labeled control:
give the Status SelectTrigger an id (e.g., "status-select") and set the
corresponding Label htmlFor to that id; do the same for Team ("team-select"),
Type ("type-select"), Search Input ("search-input"), Date From
("date-from-input"), Date To ("date-to-input"), and the Checkbox
("rootOnly-checkbox"). Update the Label components to include htmlFor and pass
matching id props into SelectTrigger / Input / Checkbox so the Label, Select
(SelectTrigger/SelectValue), Input, and Checkbox components (used in this file)
are properly associated and remain controlled via filters/setFilters (leave
existing onValueChange/onChange/onCheckedChange handlers intact); no logic
changes to DEFAULT_ISSUE_FILTERS required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7ea50fb7-1a8f-4ddc-9675-15fe5eff5fe6
📒 Files selected for processing (2)
apps/blade/src/app/_components/issues/issue-fetcher-pane-playground.tsxapps/blade/src/app/_components/issues/issue-fetcher-pane.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx (2)
85-118:⚠️ Potential issue | 🟠 MajorUnify pane loading/error/refresh across both required queries.
Line 114/117 and Line 173 only track
issuesQuery, so the pane can report “ready” while roles are still loading/failed, and Refresh won’t retry roles. That breaks the single-source contract for parent views.Suggested fix
- const rolesQuery = api.roles.getAllLinks.useQuery(undefined, { + const { + data: rolesData, + error: rolesError, + isLoading: rolesLoading, + refetch: refetchRoles, + } = api.roles.getAllLinks.useQuery(undefined, { refetchOnWindowFocus: false, }); @@ - const { - data: fetchedIssues, - error: issuesError, - isLoading, - refetch, - } = issuesQuery; - const issueErrorMessage = issuesError?.message ?? null; + const { + data: fetchedIssues, + error: issuesError, + isLoading: issuesLoading, + refetch: refetchIssues, + } = issuesQuery; + const isLoading = rolesLoading || issuesLoading; + const errorMessage = rolesError?.message ?? issuesError?.message ?? null; - const roles = useMemo(() => rolesQuery.data ?? [], [rolesQuery.data]); + const roles = useMemo(() => rolesData ?? [], [rolesData]); const refresh = useCallback(() => { - void refetch(); - }, [refetch]); + void Promise.all([refetchRoles(), refetchIssues()]); + }, [refetchIssues, refetchRoles]); @@ - error: issueErrorMessage, + error: errorMessage, @@ - issueErrorMessage, + errorMessage,Based on learnings: "gating rendering should occur only when all required data fetches succeed. Do not render partial success when some queries fail; instead, implement a unified loading/state or error handling that surfaces a single, coherent state once all data is ready or failed."
Also applies to: 172-183, 224-229
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx` around lines 85 - 118, The pane currently only observes issuesQuery (fetchedIssues/issuesError/isLoading/refetch) so rolesQuery can be loading/failed while the UI reports "ready"; update the component to derive a unified state from both rolesQuery and issuesQuery (e.g., combine rolesQuery.data && issuesQuery.data for ready, rolesQuery.isLoading || issuesQuery.isLoading for loading, and prefer a merged error message from rolesQuery.error or issuesQuery.error), and change the refresh handling to call both rolesQuery.refetch() and issuesQuery.refetch() so Refresh retries both; locate the rolesQuery and issuesQuery variables in issue-fetcher-pane.tsx to implement this unified loading/error/refresh behavior.
234-366:⚠️ Potential issue | 🟠 MajorAdd explicit
Label↔ control associations (htmlFor/id).The filter labels are visually present but not programmatically bound to inputs/select triggers/checkbox. This hurts screen-reader labeling and label click behavior.
Suggested fix
- <Label>Status</Label> + <Label htmlFor="issue-filter-status">Status</Label> @@ - <SelectTrigger> + <SelectTrigger id="issue-filter-status"> @@ - <Label>Team</Label> + <Label htmlFor="issue-filter-team">Team</Label> @@ - <SelectTrigger> + <SelectTrigger id="issue-filter-team"> @@ - <Label>Type</Label> + <Label htmlFor="issue-filter-type">Type</Label> @@ - <SelectTrigger> + <SelectTrigger id="issue-filter-type"> @@ - <Label>Search</Label> + <Label htmlFor="issue-filter-search">Search</Label> <Input + id="issue-filter-search" placeholder="Search name/description/id..." @@ - <Label>Date From</Label> + <Label htmlFor="issue-filter-date-from">Date From</Label> <Input + id="issue-filter-date-from" type="date" @@ - <Label>Date To</Label> + <Label htmlFor="issue-filter-date-to">Date To</Label> <Input + id="issue-filter-date-to" type="date" @@ <Checkbox + id="issue-filter-root-only" checked={filters.rootOnly} @@ - <Label>Show root issues only (hide subtasks)</Label> + <Label htmlFor="issue-filter-root-only"> + Show root issues only (hide subtasks) + </Label>As per coding guidelines: "apps/blade/** ... Accessibility (alt text, ARIA, semantic HTML)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx` around lines 234 - 366, Labels in the filter pane are not associated with their form controls (Label vs Input/Select/Checkbox), breaking accessibility; add explicit id attributes to each control and set the corresponding Label htmlFor to match. Specifically, assign unique ids for the Status Select (tie to Select or SelectTrigger depending on the Select implementation), Team Select, Type Select, Search/Input, Date From, Date To, and the rootOnly Checkbox, and update each Label's htmlFor to those ids; keep ids unique (e.g., status-filter, team-filter, type-filter, search-filter, date-from-filter, date-to-filter, root-only-filter) and ensure any custom Select component uses the id in the actionable trigger element so clicking the Label focuses the control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx`:
- Around line 85-118: The pane currently only observes issuesQuery
(fetchedIssues/issuesError/isLoading/refetch) so rolesQuery can be
loading/failed while the UI reports "ready"; update the component to derive a
unified state from both rolesQuery and issuesQuery (e.g., combine
rolesQuery.data && issuesQuery.data for ready, rolesQuery.isLoading ||
issuesQuery.isLoading for loading, and prefer a merged error message from
rolesQuery.error or issuesQuery.error), and change the refresh handling to call
both rolesQuery.refetch() and issuesQuery.refetch() so Refresh retries both;
locate the rolesQuery and issuesQuery variables in issue-fetcher-pane.tsx to
implement this unified loading/error/refresh behavior.
- Around line 234-366: Labels in the filter pane are not associated with their
form controls (Label vs Input/Select/Checkbox), breaking accessibility; add
explicit id attributes to each control and set the corresponding Label htmlFor
to match. Specifically, assign unique ids for the Status Select (tie to Select
or SelectTrigger depending on the Select implementation), Team Select, Type
Select, Search/Input, Date From, Date To, and the rootOnly Checkbox, and update
each Label's htmlFor to those ids; keep ids unique (e.g., status-filter,
team-filter, type-filter, search-filter, date-from-filter, date-to-filter,
root-only-filter) and ensure any custom Select component uses the id in the
actionable trigger element so clicking the Label focuses the control.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 89eb541b-0f9e-4a80-8a50-93db5a3a4812
📒 Files selected for processing (1)
apps/blade/src/app/_components/issues/issue-fetcher-pane.tsx
Jomak-x
changed the title
Jomak-x
changed the title
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.
Why
One shared issue filter/fetch component that can be reused across different Blade views later.
What
Issue(s): #398
Added a reusable IssueFetcherPane component that handles shared issue filtering and fetching.
Also added a small IssueFetcherPanePlayground example to show how a parent view can receive the filtered issues and use them for later views like list, kanban, or calendar.
Test Plan
Checklist
db:pushbefore mergingSummary by CodeRabbit