Repository navigation
Conversation
github-actions
Bot
added
vouch:unvouched
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe default-branch confirmation dialog now tracks its open state separately from its pending action. Abort and continuation close the dialog without immediately clearing pending data. The pending action is cleared when the exit transition completes. ChangesDefault-branch confirmation
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue remains in the changed confirmation flow. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The confirmation remains required before the affected Git actions proceed, and no new action path or authority was identified. Some confidence depends on how the dialog handles reopening during its close transition. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai Two notes on the review:
|
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, self-contained UI bug fix that preserves the pending Git action and its copy through the dialog exit animation while retaining existing continuation behavior. It touches no sensitive, schema, deployment, or static-analysis configuration areas. You can add or adjust custom eligibility rules. Learn more. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline apps/web/src/components/GitActionsControl.tsx --match 'GitActionsControl|defaultBranch|continue' --view expanded | head -140
rg -n -C4 'onOpenChangeComplete|defaultBranch|featureBranch|handleContinue|@base-ui/react' apps/web/src/components/GitActionsControl.tsx | tail -220
rg -n -C2 '"@base-ui/react"|@base-ui/react@1\.5\.0' -g 'package.json' -g '*lock*' | head -90
gh api repos/pingdotgg/t3code/pulls/13921/comments --jq '.[] | select(.user.login=="coderabbitai" or (.body|test("docstring|reopen|Reopen|close|completion";"i"))) | {id,body,path,line,html_url}' | head -180Length of output: 5677 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -i 'dialog*' apps/web/src/components | head -35
rg -n -C5 'onOpenChangeComplete|DialogRoot|Root as Dialog' apps/web/src/components/ui -g '*.tsx' | head -130
sed -n '1170,1195p;1240,1265p;1455,1480p;2010,2045p' apps/web/src/components/GitActionsControl.tsx
printf '\nUpstream Base UI source:\n'
curl -fsSL --max-time 15 'https://unpkg.com/@base-ui/react@1.5.0/esm/internals/useOpenChangeComplete.js' | head -160Length of output: 2351 🏁 Script executed: #!/bin/bash
ast-grep outline apps/web/src/components/ui/dialog.tsx --view expanded | head -100
sed -n '1170,1195p;1240,1265p;1455,1480p;2010,2045p' apps/web/src/components/GitActionsControl.tsx
sed -n '1,130p' apps/web/src/components/ui/dialog.tsx
printf '\nUpstream Base UI source:\n'
curl -fsSL --max-time 15 'https://unpkg.com/@base-ui/react@1.5.0/esm/internals/useOpenChangeComplete.js' | head -160Length of output: 9904
I also withdraw the docstring request. The changed function is a component-local event handler, and your clarification about this codebase’s docstring convention applies here. ✏️ Learnings added
You are interacting with an AI system. |
afa266d to
58a93f2
Compare
"Continue" and "Check out feature branch & continue" duplicated the same body and differed only in the featureBranch flag. Fold them into one handler that takes the flag, so the confirmation has a single exit path for running the pending action.
58a93f2 to
43c9a8a
Compare
The dialog's open state was derived from the pending action payload, so
dismissing it nulled the payload in the same render. The popup stayed
mounted for its 200ms exit transition with fallback copy ("Run action on
default branch?", no description, "Continue"), visibly shrinking before
it faded.
Track the open flag separately and clear the payload in
onOpenChangeComplete(false), matching RelayClientInstallDialog and
ConfirmDialogHost. The continue handler now checks the open flag so a
click during the exit transition cannot run the action twice.
Fixes pingdotgg#13831
43c9a8a to
b35127f
Compare
|
Note This comment is posted by Julius' dot The PR still promises a before/after recording, but none is supplied. A jsdom check doesn't demonstrate the visible 200ms closing transition. Closing under the verification rule. Add current-client before/after evidence showing dismissal and both continue actions keeping their copy until the dialog disappears, then request reconsideration. |
|
@juliusmarminge Thanks. I added current-client before/after evidence to the description. It was recorded in the web client against current
Before, all three paths drop to "Run action on default branch?" with no description and a "Continue" label, and the dialog shrinks as it fades. After, the original title, description, and labels stay until the dialog is gone. The real-speed frame strips (33ms apart) in the description show the same thing without the slowdown. Could you take another look and reopen if it holds up? |
2 participants
What Changed
The default-branch confirmation ("Commit & push to default ref?") now tracks its open state separately from the pending action. The pending action is cleared in
onOpenChangeComplete(false), after the exit transition, instead of in the same render that closes the dialog.A preceding refactor commit merges the "Continue" and "Check out feature branch & continue" handlers, which differed only by the
featureBranchflag, so the dialog has a single close path. That handler now checks the open flag, so a click during the exit transition cannot run the action twice.Why
Fixes #13831.
openwas derived frompendingDefaultBranchAction !== null, so Abort, the close button, Escape, and both continue buttons nulled the payload while the popup was still mounted for its 200ms exit transition. The title, description, and primary label fell back to "Run action on default branch?", nothing, and "Continue", and the dialog visibly shrank before it faded.This matches the pattern
RelayClientInstallDialogandConfirmDialogHostalready use. Base UI aborts the pending completion callback whenopenflips back, so reopening mid-exit cannot clear a fresh payload.Reproduced in a throwaway jsdom harness with the real
Dialogand a held exit animation: before the fix the mounted popup showed the fallback title during exit; after it, the original copy stays until unmount. Web and desktop share this UI; the mobile confirm sheet is a separate route and is unaffected.UI Changes
Recorded in the web client against current
main(5cc99e1); the fix rebases onto it cleanly. Synthetic repo onmainwith a remote. Each video shows Abort at real speed, then Abort, "Commit & push to main", and "Check out feature branch & continue" with animations slowed to 10% through DevTools (Animation.setPlaybackRate) so the 200ms exit is visible.Before: every close path swaps the copy to "Run action on default branch?" with no description and a "Continue" label, and the dialog shrinks while it fades.
After: the title, description, and both continue labels stay until the dialog is gone.
Real-speed Abort, 33ms per frame from the click:
MP4: before · after
Checklist
Made with Claude Opus 5.5 in Claude Code (via T3 Code).
Summary by CodeRabbit