[8762] Git Management view flow and dialog issues

[jvega190]

craftercms/craftercms#8762

Summary by CodeRabbit

• New Features

  • The conflicted-path diff view now opens in the app’s shared dialog system, improving consistency with other dialogs.
  • The diff dialog now supports better fullscreen behavior and resizes more smoothly.
  • The split-view diff editor now adjusts automatically when the window or layout changes.

• Bug Fixes

  • Improved pull error handling so failures close the dialog more reliably.
  • Conflict resolution dialogs now close correctly after resolving a conflict.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2e3ddce9-2828-4ca3-9c0b-9d963ca49611

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8ec52 and 84c6fa2.

📒 Files selected for processing (5)

• ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialog.tsx
• ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialogSplitView.tsx
• ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialogUI.tsx
• ui/app/src/components/GitManagement/RepoGrid/RepoGrid.tsx
• ui/app/src/components/GitManagement/RepoStatus/RepoStatus.tsx

---

Walkthrough

ConflictedPathDiffDialog was migrated from MUI Dialog to EnhancedDialog with a renamed props interface. Split-view editor and dialog UI layout were adjusted for automatic layout and fullscreen height. RepoGrid pull error handling was refactored, and RepoStatus now opens/closes the diff dialog via the Redux dialog stack.

Changes

Conflicted Path Diff Dialog and Git Management updates

Layer / File(s)	Summary
EnhancedDialog migration ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialog.tsx	Replaces MUI Dialog/DialogHeader with EnhancedDialog, renames RemoteRepositoriesDiffDialogProps to ConflictedPathDiffDialogProps extending EnhancedDialogProps, moves title/tabs into dialogHeaderProps, and updates the close handler and closing tag.
Split-view editor and dialog layout ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialogSplitView.tsx, ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialogUI.tsx	Types the Monaco editor ref, adds automaticLayout: true, and updates ConflictedPathDiffDialogUI to size height based on fullscreen state via useEnhancedDialogContext, changing tabsHeight to '60vh'.
RepoGrid pull error handling ui/app/src/components/GitManagement/RepoGrid/RepoGrid.tsx	Adds handlePullError to forward errors and close the pull dialog, wiring it into PullDialog's onPullError prop.
RepoStatus dialog-stack integration ui/app/src/components/GitManagement/RepoStatus/RepoStatus.tsx	Replaces local open/path state with Redux dialog-stack pushDialog/popDialog and a nanoid-generated diffDialogId, removing the inline dialog rendering.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RepoStatus
  participant DialogStackRedux
  participant ConflictedPathDiffDialog

  User->>RepoStatus: click conflicted path
  RepoStatus->>DialogStackRedux: dispatch(pushDialog(ConflictedPathDiffDialog, path, onResolveConflict))
  DialogStackRedux->>ConflictedPathDiffDialog: render with props
  User->>ConflictedPathDiffDialog: resolve conflict
  ConflictedPathDiffDialog->>RepoStatus: onResolveConflict callback
  RepoStatus->>DialogStackRedux: dispatch(popDialog({ id: diffDialogId }))
  DialogStackRedux->>ConflictedPathDiffDialog: unmount

Loading

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description only links the issue and lacks the required template heading and a summary of the changes.	Add the template heading and summarize the implemented changes, affected components, and any validation performed.
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 (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the PR's main theme: Git Management flow and dialog fixes for issue 8762.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jvega190]

@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review

[coderabbitai]

Oops, something went wrong! Please try again later. 🐰 💔

[jvega190]

@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh pr view 4906 --repo craftercms/studio-ui --json title,body,state,isDraft,files,additions,deletions,commits -q '.'
echo "----DIFF----"
gh pr diff 4906 --repo craftercms/studio-ui

Length of output: 13130

---

🏁 Script executed:

#!/bin/bash
echo "---- EnhancedDialog index/types ----"
fd -t f . ui/app/src/components/EnhancedDialog | head -50
echo "---- EnhancedDialogProps onClose type ----"
rg -n "onClose" ui/app/src/components/EnhancedDialog/*.ts* -A2 -B2

echo "---- ConflictedPathDiffDialogSplitView full file ----"
cat -n ui/app/src/components/ConflictedPathDiffDialog/ConflictedPathDiffDialogSplitView.tsx

echo "---- createComponentId usage ----"
rg -n "createComponentId" ui/app/src -A5 -B2 | head -80

echo "---- other pushDialog examples for reference ----"
rg -n "pushDialog\(" ui/app/src -B2 -A15 | head -150

Length of output: 24591

---

🏁 Script executed:

#!/bin/bash
rg -n "ConflictedPathDiffDialog" ui/app/src --type=ts --type=tsx -g '!*.test.*' 
echo "---- dialog component registry ----"
rg -n "craftercms.components" ui/app/src -l
rg -n "'ConflictedPathDiffDialog'|\"ConflictedPathDiffDialog\"" ui/app/src

Length of output: 1954

---

🏁 Script executed:

#!/bin/bash
rg -n "ConflictedPathDiffDialog|registerComponents|PublishPackageReviewDialog" ui/app/src/env/registerComponents.ts
echo "---full file head---"
sed -n '1,80p' ui/app/src/env/registerComponents.ts

Length of output: 1885

---

🏁 Script executed:

#!/bin/bash
fd studioUI.ts ui/app/src/env
rg -n "ConflictedPathDiffDialog|PublishPackageReviewDialog" ui/app/src/env/studioUI.ts

Length of output: 483

---

Good — that confirms ConflictedPathDiffDialog is already registered in ui/app/src/env/studioUI.ts (Line 91), so the dialog-stack lookup via createComponentId('ConflictedPathDiffDialog') in RepoStatus.tsx will resolve correctly. That was my main open concern, and it checks out.

Overall the diff looks solid and follows established patterns in the codebase (e.g., packageActions.ts, itemActions.ts use the same nanoid + pushDialog/popDialog pattern). A couple of minor, non-blocking notes:

1. ConflictedPathDiffDialogSplitView.tsx (Line 33) — useRef<HTMLDivElement>(undefined) works, but the more idiomatic React/TS pattern is useRef<HTMLDivElement>(null). Not a functional bug since Box ref={ref} will still populate it, just a minor style nit.
2. ConflictedPathDiffDialog.tsx — maxWidth changed from "md" to "lg"; confirm this is an intentional UX tweak and not an incidental change while migrating to EnhancedDialog.
3. RepoGrid.tsx — handlePullError always calls pullFromRemoteDialogState.onClose() even if onPullError is undefined; this matches the intended bug fix (closing the dialog reliably on pull error), so looks correct.
4. The onClose?.(e, null) pattern in ConflictedPathDiffDialog.tsx matches the internal convention already used inside EnhancedDialog.tsx (onClose(e, null) for the close button), so it's consistent.

No blocking issues found — the migration to EnhancedDialog, the dialog-stack-based open/close flow in RepoStatus.tsx, and the pull-error handling in RepoGrid.tsx are all wired correctly. Nice work, @jvega190! This looks ready for human review from my end, though the PR is still marked draft — let me know if you'd like me to do anything else before handoff.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.