Repository navigation
Unify single and multiple PR hover details - #32284
Conversation
📝 Walkthrough
Priority: ⚪ Pending latest changes Merge Risk: 🟡 Moderate · up to
|
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
234453c0-8df8-4936-85ed-674ce9c7bed3
📒 Files selected for processing: 20
src/renderer/src/assets/main.csssrc/renderer/src/components/activity/activity-thread-hover-card.tsxsrc/renderer/src/components/sidebar/WorktreeCardIssueDetailSection.tsxsrc/renderer/src/components/sidebar/WorktreeCardLinkedItemsSection.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardLinkedItemsSection.tsxsrc/renderer/src/components/sidebar/WorktreeCardMeta.interaction.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardMeta.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardMeta.tsxsrc/renderer/src/components/sidebar/WorktreeCardMetadataControls.tsxsrc/renderer/src/components/sidebar/WorktreeCardMetadataStatusBadges.tsxsrc/renderer/src/components/sidebar/WorktreeCardReferenceRow.tsxsrc/renderer/src/components/sidebar/WorktreeCardReviewActions.tsxsrc/renderer/src/components/sidebar/WorktreeCardReviewDetailSection.tsxsrc/renderer/src/components/sidebar/use-workspace-reference-link-actions.tssrc/renderer/src/components/sidebar/use-worktree-card-secondary-details.store-subscriptions.test.tsxsrc/renderer/src/components/sidebar/use-worktree-card-secondary-details.tssrc/renderer/src/components/sidebar/worktree-card-meta-types.tssrc/renderer/src/components/sidebar/worktree-card-parent-content.tsxsrc/renderer/src/components/sidebar/worktree-card-presentation.tsxsrc/renderer/src/components/sidebar/worktree-review-helpers.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| (e: React.MouseEvent, review: WorktreeCardPrDisplay | null = hoverReview) => { | ||
| e.stopPropagation() | ||
| if (!repo || !hoverReview?.url || hoverReview.provider !== 'github') { | ||
| if (!repo || !review?.url || review.provider !== 'github') { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
-
🏁 Script executed:
#!/bin/bash rg -nP -C4 'openGitHubWorkItem' src/renderer/src --type=ts --type=tsx -g '!**/*.test.*' rg -nP -C3 'repoId' src/shared/worktree/types.ts | head -60
Repository: stablyai/orca
Length of output: 1637
🏁 Script executed:
#!/bin/bash set -u printf '%s\n' '--- changed handler and callers ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'handleOpenReviewInOrca' \ -e 'openGitHubWorkItem' \ -e 'WorktreeCardLinkedReviewActions' \ src/renderer/src || test "$?" -eq 1 printf '%s\n' '--- target file ---' nl -ba src/renderer/src/components/sidebar/use-worktree-card-secondary-details.ts | sed -n '90,175p' printf '%s\n' '--- definitions and imports of openGitHubWorkItem ---' rg -n -F --glob '*.ts' --glob '*.tsx' 'openGitHubWorkItem' src || test "$?" -eq 1 printf '%s\n' '--- relevant work-item and task-page repository resolution ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'repoId' \ -e 'findTaskPageRepoForWorkItem' \ -e 'useGitHubItemDialogDetails' \ -e 'GitHubWorkItem' \ src/renderer/src/components src/renderer/src/lib src/renderer/src/store | head -240
Repository: stablyai/orca
Length of output: 44611
🏁 Script executed:
#!/bin/bash set -u printf '%s\n' '--- task-page navigation action ---' nl -ba src/renderer/src/store/slices/ui/ui-slice-task-actions.ts | sed -n '1,115p' printf '%s\n' '--- GitHub detail hook ---' nl -ba src/renderer/src/components/use-task-page-github-detail.ts | sed -n '80,205p' printf '%s\n' '--- related repository selection and dialog APIs ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'setDialogWorkItem' \ -e 'dialogRepo' \ -e 'preselectedRepoId' \ -e 'repoId: item.repoId' \ src/renderer/src/components src/renderer/src/store | head -180
Repository: stablyai/orca
Length of output: 20734
🏁 Script executed:
#!/bin/bash set -u printf '%s\n' '--- review display type and repository metadata ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'WorktreeCardPrDisplay' \ -e 'type WorktreeCardPrDisplay' \ -e 'interface WorktreeCardPrDisplay' \ -e 'repoId' \ src/renderer/src/components/sidebar | head -220 printf '%s\n' '--- getWorkItemRepo definition ---' rg -n -F --glob '*.ts' --glob '*.tsx' 'function getWorkItemRepo' src/renderer/src src/shared || test "$?" -eq 1 rg -n -F --glob '*.ts' --glob '*.tsx' 'const getWorkItemRepo' src/renderer/src src/shared || test "$?" -eq 1 printf '%s\n' '--- review fixture shapes ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'provider: '\''github'\''' \ -e 'linkedReview' \ -e 'repoId:' \ src/renderer/src/components/sidebar | head -260
Repository: stablyai/orca
Length of output: 41252
🏁 Script executed:
#!/bin/bash set -u printf '%s\n' '--- WorktreeCardPrDisplay declaration ---' nl -ba src/renderer/src/components/sidebar/worktree-card-pr-display.ts | sed -n '1,220p' printf '%s\n' '--- getWorkItemRepo implementation ---' nl -ba src/renderer/src/lib/work-item-repo-row.ts | sed -n '1,120p' printf '%s\n' '--- review display construction and attachment mapping ---' rg -n -F --glob '*.ts' --glob '*.tsx' \ -e 'url:' \ -e 'linkedReview' \ -e 'WorktreeCardPrDisplay' \ -e 'getWorkspaceAttachment' \ src/renderer/src/components/sidebar/worktree-card-pr-display.ts \ src/renderer/src/components/sidebar/use-worktree-card-review-details.ts \ src/renderer/src/components/sidebar/worktree-attachment-editing.ts \ src/renderer/src/components/sidebar/workspace-hover-attachments.ts \ src/renderer/src/components/sidebar/worktree-card-review-display.ts 2>/dev/null || true
Repository: stablyai/orca
Length of output: 12831
Resolve the sibling review repository from its URL.
WorktreeCardPrDisplay has no repoId. When the selected review URL belongs to another GitHub repository, handleOpenReviewInOrca currently assigns the workspace repo.id to both item.repoId and preselectedRepoId. The task page uses that ID for repository lookup, so it can open the wrong pull request. Match the GitHub repository in review.url to the corresponding repository row and use that ID for both values.
| ...buildWorkspaceAttachmentEdits({ initial, draft }), | ||
| ...(item.provider === 'github' && | ||
| hoverReview?.provider === 'github' && | ||
| hoverReview.number === item.number && | ||
| (!item.url || item.url === hoverReview.url) | ||
| ? { suppressedGitHubPR: item.number } | ||
| : {}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
-
🏁 Script executed:
#!/bin/bash rg -nP -C6 'linkedItemsBase' src/main src/shared --type=ts -g '!**/*.test.*' rg -nP -C4 'matchesLegacyWorkspaceAttachment' src/shared --type=ts -g '!**/*.test.*'
Repository: stablyai/orca
Length of output: 14159
Clear the legacy singular field when unlinking the primary attachment.
normalizeWorkspaceAttachmentUpdate merges linkedItems but does not clear the corresponding legacy singular field. getWorkspaceAttachments can therefore re-add the unlinked GitHub PR when linkedPR remains set.
Set the matching singular field to null when the primary attachment is removed.
| <span | ||
| tabIndex={0} | ||
| aria-label={label} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a role to the focusable span that carries aria-label.
aria-label on a generic span has no role, so assistive technology may not announce it. Add role="img" so the checks status gets an accessible name. WorktreeCardReferenceRow.tsx uses the same pattern for the review-state icon at Lines 49-53.
♿ Proposed fix
-
--- "a/src/renderer/src/components/sidebar/WorktreeCardMetadataStatusBadges.tsx" +++ "b/src/renderer/src/components/sidebar/WorktreeCardMetadataStatusBadges.tsx" @@ -242,9 +242,10 @@ return ( <Tooltip> <TooltipTrigger asChild> <span tabIndex={0} + role="img" aria-label={label} className={cn( 'inline-flex shrink-0 rounded-sm outline-none focus-visible:ring-2 focus-visible:ring-ring', getReviewChecksTone(status)
Based on learnings: controls and status elements need semantics and an accessible name from valid sources such as aria-label.
📝 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<span tabIndex={0} aria-label={label} <span tabIndex={0} role="img" aria-label={label}
Source: Learnings
| <span | ||
| tabIndex={0} | ||
| aria-label={reviewStateLabel(review.state, getReviewLabel(review))} | ||
| className="inline-flex shrink-0 rounded-sm outline-none focus-visible:ring-2 focus-visible:ring-ring" | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add a role to the focusable review-state span.
The span has tabIndex={0} and aria-label but no role. Add role="img" so the state label is announced. This is the same issue as in ReviewChecksIndicator.
♿ Proposed fix
-
--- "a/src/renderer/src/components/sidebar/WorktreeCardReferenceRow.tsx" +++ "b/src/renderer/src/components/sidebar/WorktreeCardReferenceRow.tsx" @@ -46,9 +46,10 @@ {review ? ( <Tooltip> <TooltipTrigger asChild> <span tabIndex={0} + role="img" aria-label={reviewStateLabel(review.state, getReviewLabel(review))} className="inline-flex shrink-0 rounded-sm outline-none focus-visible:ring-2 focus-visible:ring-ring" > <ReviewIcon review={review} className="size-3.5" showChecks={false} />
Based on learnings: ARIA controls need an accessible name and appropriate semantics.
📝 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<span tabIndex={0} aria-label={reviewStateLabel(review.state, getReviewLabel(review))} className="inline-flex shrink-0 rounded-sm outline-none focus-visible:ring-2 focus-visible:ring-ring" > <span tabIndex={0} role="img" aria-label={reviewStateLabel(review.state, getReviewLabel(review))} className="inline-flex shrink-0 rounded-sm outline-none focus-visible:ring-2 focus-visible:ring-ring" >
Source: Learnings
| const url = item.url ?? review?.url | ||
| return ( | ||
| <WorktreeCardReviewActions | ||
| label={item.type === 'mr' ? 'MR' : 'PR'} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix the localization coverage failure for the 'MR' and 'PR' JSX literals.
CI fails at bun config/scripts/audit-localization-coverage.mjs --check for these literals in the label attribute. Move the selection out of JSX into a typed constant. The audit then sees an identifier, not a string literal. Alternatively, add a reviewed exclusion to the allowlist.
🐛 Proposed fix
-
--- "a/src/renderer/src/components/sidebar/WorktreeCardReviewActions.tsx" +++ "b/src/renderer/src/components/sidebar/WorktreeCardReviewActions.tsx" @@ -122,11 +122,12 @@ onOpenInOrca?: (event: MouseEvent, review?: WorktreeCardPrDisplay) => void onUnlink?: (item?: WorkspaceAttachment) => void closeHover: () => void }): React.JSX.Element { const url = item.url ?? review?.url + const reviewLabel: 'MR' | 'PR' = item.type === 'mr' ? 'MR' : 'PR' return ( <WorktreeCardReviewActions - label={item.type === 'mr' ? 'MR' : 'PR'} + label={reviewLabel} number={item.number} provider={workspaceAttachmentProviderLabel(item)} onOpenChange={onOpenChange}
Run the audit again to confirm that it passes.
🧰 Tools
-
🪛 GitHub Actions: PR 32284 | source 4001d05c1acbf9eaedaeb2862034f965f44bbcdf | workflow 4001d05c1acbf9eaedaeb2862034f965f44bbcdf | unit full / 14_static analysis and typecheck.txt
-
[error] 129-129: Localization coverage check failed: new unlocalized renderer strings "MR" and "PR" were found in JSX label attributes. Localize them or add a reviewed exclusion to the localization allowlist.
🪛 GitHub Actions: PR 32284 | source 4001d05c1acbf9eaedaeb2862034f965f44bbcdf | workflow 4001d05c1acbf9eaedaeb2862034f965f44bbcdf | unit full / static analysis and typecheck
-
[error] 129-129: Localization coverage check failed for the unlocalized JSX attribute labels "MR" and "PR". Localize these strings or add a reviewed exclusion to the localization allowlist. Failed command: bun config/scripts/audit-localization-coverage.mjs --check.
-
Source: Pipeline failures
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2cd785b4-7a34-49e9-8c4d-0df38d93d405
📒 Files selected for processing: 5
src/renderer/src/assets/main.csssrc/renderer/src/components/file-path-cursor-tooltip.tsxsrc/renderer/src/components/sidebar/WorktreeCardLinkedItemsSection.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardMeta.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardReferenceRow.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Route standalone issue titles through the shared… · WorktreeCardIssueDetailSection.tsx:145-153
src/renderer/src/components/sidebar/WorktreeCardIssueDetailSection.tsx:145-153
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute standalone issue titles through the shared link action.
WorktreeCardIssueDetailSectionrenders the issue title as a non-interactive<div>. Its reachable callers do not passonOpenLink, unlike the review and linked-item paths. Therefore, clicking or pressing Enter on the title cannot open the shared link-action menu, apply modifier-aware browser behavior, or expose its copy action. Users must use the ellipsis menu instead.Add
onOpenLinkto the issue section, pass it from both callers, and render URL-backed titles as anchors that invoke the shared callback.Suggested fix
diff --git a/src/renderer/src/components/sidebar/WorktreeCardIssueDetailSection.tsx b/src/renderer/src/components/sidebar/WorktreeCardIssueDetailSection.tsx @@ +import { FilePathCursorTooltip } from '@/components/file-path-cursor-tooltip' import { Badge } from '@/components/ui/badge' @@ onOpenIssueInBrowser?: (url: string) => void onEditIssue?: (event: React.MouseEvent) => void onOpenGitHubIssueInOrca?: (event: React.MouseEvent) => void + onOpenLink: (url: string, event: React.MouseEvent) => void onManageLinks?: (event: React.MouseEvent) => void @@ onOpenIssueInBrowser, onEditIssue, onOpenGitHubIssueInOrca, + onOpenLink, onManageLinks @@ /> <WorktreeCardDetailSectionContent className="space-y-1.5"> - <div className="text-[13px] font-semibold leading-snug text-foreground break-words"> - {issue.title} - </div> + {issue.url ? ( + <FilePathCursorTooltip path={issue.url}> + <a + href={issue.url} + target="_blank" + rel="noreferrer" + className="block w-full rounded-sm text-[13px] font-semibold leading-snug text-foreground break-words underline decoration-muted-foreground/50 underline-offset-2 outline-none hover:decoration-foreground focus-visible:ring-2 focus-visible:ring-ring" + onClick={(event) => { + event.stopPropagation() + onOpenLink(issue.url!, event) + }} + onAuxClick={(event) => event.stopPropagation()} + onKeyDown={(event) => event.stopPropagation()} + > + {issue.title} + </a> + </FilePathCursorTooltip> + ) : ( + <div className="text-[13px] font-semibold leading-snug text-foreground break-words"> + {issue.title} + </div> + )}diff --git a/src/renderer/src/components/sidebar/WorktreeCardMeta.tsx b/src/renderer/src/components/sidebar/WorktreeCardMeta.tsx @@ <WorktreeCardIssueDetailSection issue={issue} + onOpenLink={onOpenLink} onManageLinks={onManageLinks ? dismissAndRun(onManageLinks) : undefined}diff --git a/src/renderer/src/components/activity/activity-thread-hover-card.tsx b/src/renderer/src/components/activity/activity-thread-hover-card.tsx @@ <WorktreeCardIssueDetailSection issue={secondary.hoverIssue} + onOpenLink={onOpenLink} onManageLinks={dismissAndRun(foundation.handleManageLinks)}
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2aee3a27-977c-45a1-9b25-6a404e63e825
📒 Files selected for processing: 7
src/renderer/src/components/icons/GitHubIcon.tsxsrc/renderer/src/components/sidebar/WorktreeCardLinkedItemsSection.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardMeta.test.tsxsrc/renderer/src/components/sidebar/WorktreeCardReferenceRow.tsxsrc/renderer/src/components/sidebar/WorktreeCardReviewActions.tsxsrc/renderer/src/components/stats/share-card-utils.tsxsrc/renderer/src/i18n/en-runtime-required.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
187 commits landed on main between f045046 and 7cafdb0. All seven catalogs conflicted along with en-runtime-required.json, so they are taken from main and the prune is recomputed in one pass. It is the previous 1,624 keys plus 9: five lost their reader when stablyai#32284 unified the single and multiple PR hover details, two when stablyai#31688 moved skill updates to Orca's own installer, and two when stablyai#28709 reworked where chat agents are offered. None left the set. Since stablyai#32782 filled the missing entries in the other catalogs, all seven now shed the same 1,633 keys. en.json goes from 16,992 entries to 15,359; en-runtime-required.json keeps 615 of them. native-chat-shared-copy-matches-catalog.test.ts merged cleanly this time and still differs from main by exactly the branch's edit.
ELI5
PR details use the same layout for one or several reviews. The left icon shows review state, the GitHub logo precedes the PR number and a small check follows it, and hovering a title shows its full URL. Actions sit in an ellipsis menu at the far right of the metadata line.
What Changed
Why
The previous single-review header differed from the collection layout, repeated status information, and spread actions across several controls. Consistent rows keep status, URLs, and actions easy to find.
Linked Issue
Requested UX improvement; no linked issue.
Visual Proof
Captured via Playwright CDP on macOS using the real hover components. Before uses the pre-change components from commit 0841e09 with identical fixture data. Demo action callbacks are stubbed; attachment targeting and persistence are covered by automated tests.
The full URL appears below the pointer:
Reviews and Issues share the header add control in light mode:
Open, draft, and closed reviews retain distinct state and check tooltips:
Testing
pnpm tc:web— passed.pnpm run check:code-quality:changed— zero new findings.git diff --check— passed; changed files formatted.Adversarial review found and fixed the shared cursor tooltip's invisible positioning wrapper intercepting clicks on the menu below it. No further findings in the rendered cases above.
Rendered verification was on macOS. Linux, Windows, and live SSH were not run locally; URL routing reuses existing platform-aware behavior. Full build and broader suites are left to CI.
Review
Notes
No IPC or wire-format changes. Mobile has no separate surface changed here.