[8419] Improve Change Type dialog/flow#4902
Conversation
|
Warning Review limit reached
More reviews will be available in 23 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe dialog now fetches allowed content types for the current item path, passes them into the container, and changes sizing, loading behavior, empty-state text, and footer actions based on whether compatible types are available. ChangesChangeContentTypeDialog allowed-types flow
Sequence Diagram(s)sequenceDiagram
participant ChangeContentTypeDialog
participant useFetchAllowedTypesForPath
participant ChangeContentTypeDialogContainer
ChangeContentTypeDialog->>useFetchAllowedTypesForPath: fetch allowed types for normalized path
useFetchAllowedTypesForPath-->>ChangeContentTypeDialog: contentTypes and isFetching
ChangeContentTypeDialog->>ChangeContentTypeDialogContainer: render with contentTypes and isFetching
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🧹 Nitpick comments (1)
ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx (1)
67-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTypo:
showSelectTpeView→showSelectTypeView.Minor naming nit for readability; update the declaration and its uses on lines 72 and 74.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx` at line 67, Rename the misspelled boolean `showSelectTpeView` to `showSelectTypeView` in `ChangeContentTypeDialogContainer`, and update every reference to it in the same component so the naming is consistent and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx`:
- Around line 35-41: The loading state in ChangeContentTypeDialog is using the
compact xs width because maxWidth only checks contentTypes?.length, which makes
the SelectTypeView skeleton render cramped and then jump to lg after data loads.
Update the dialog sizing logic in ChangeContentTypeDialog to treat isFetching as
a full-layout state as well, so maxWidth and the subtitle follow isFetching ||
contentTypes?.length and the skeleton gets the larger layout from the start.
In `@ui/app/src/components/ChangeContentTypeDialog/utils.ts`:
- Line 27: Make the ChangeContentTypeDialog props match the hook state by making
contentTypes optional instead of required. Update the type in the utils.ts
definition used by ChangeContentTypeDialog so it accepts ContentType[] |
undefined, since useFetchAllowedTypesForPath returns undefined until data loads.
Keep the existing consumer behavior that already guards with optional chaining,
and ensure any references to contentTypes in ChangeContentTypeDialog or related
props handle the optional value safely.
---
Nitpick comments:
In
`@ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx`:
- Line 67: Rename the misspelled boolean `showSelectTpeView` to
`showSelectTypeView` in `ChangeContentTypeDialogContainer`, and update every
reference to it in the same component so the naming is consistent and readable.
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25c50981-ce50-4e59-9735-09004b97a596
📒 Files selected for processing (3)
ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsxui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsxui/app/src/components/ChangeContentTypeDialog/utils.ts
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Changed files vs develop:"
git diff --name-only origin/develop...HEAD
echo
echo "==== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx ===="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx --view expanded || true
echo
echo "==== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx ===="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "==== File: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx ===="
sed -n '1,260p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "==== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx ===="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx --view expanded || true
echo
echo "==== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx ===="
git diff --unified=100 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "==== File: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx ===="
sed -n '1,320p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "==== File: ui/app/src/components/ChangeContentTypeDialog/utils.ts ===="
sed -n '1,220p' ui/app/src/components/ChangeContentTypeDialog/utils.ts
echo
echo "==== Search: ChangeContentTypeDialogContainer usages ===="
rg -n -C3 'ChangeContentTypeDialogContainer' ui/app/src/components/ChangeContentTypeDialog
echo
echo "==== Search: useFetchAllowedTypesForPath usages ===="
rg -n -C3 'useFetchAllowedTypesForPath' ui/app/srcLength of output: 30361
Blocking issue:
isFetching={isFetching}Cleanup / likely lint issue:
Once those are fixed, the rest of the refactor looks consistent to me: fetching is centralized in the parent, the dialog sizing/header behavior matches the fetched-state intent, and the empty-state copy/button behavior makes sense. ✅ Action performedReview finished.
|
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Changed files vs develop =="
git diff --name-only origin/develop...HEAD
echo
echo "== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx --view expanded || true
echo
echo "== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
sed -n '1,260p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx --view expanded || true
echo
echo "== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
git diff --unified=120 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
sed -n '1,340p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/utils.ts =="
sed -n '1,220p' ui/app/src/components/ChangeContentTypeDialog/utils.ts
echo
echo "== Search: ChangeContentTypeDialogBaseProps usage =="
rg -n -C3 'ChangeContentTypeDialogBaseProps|isFetching|contentTypes' ui/app/src/components/ChangeContentTypeDialog
echo
echo "== Search: useFetchAllowedTypesForPath in ChangeContentTypeDialog subtree =="
rg -n -C3 'useFetchAllowedTypesForPath|getNormalizedFolderPathForApi1GetTypes' ui/app/src/components/ChangeContentTypeDialogLength of output: 32154 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Search: ChangeContentTypeDialog usages across repo =="
rg -n -C3 'ChangeContentTypeDialog' ui/app/src
echo
echo "== Search: ChangeContentTypeDialogProps and ChangeContentTypeDialogStateProps usages across repo =="
rg -n -C3 'ChangeContentTypeDialogProps|ChangeContentTypeDialogStateProps' ui/app/src
echo
echo "== Search: dialog registration for ChangeContentTypeDialog =="
rg -n -C4 'Change Content Type|ChangeContentTypeDialog' ui/app/src/state ui/app/src/components ui/app/src/utils ui/app/src/models
echo
echo "== Candidate file names related to ChangeContentTypeDialog =="
fd -i 'ChangeContentTypeDialog' ui/app/src
echo
echo "== If present, inspect any matching state/connected files =="
for f in $(fd -i 'ChangeContentTypeDialog' ui/app/src); do
echo
echo "---- FILE: $f ----"
sed -n '1,260p' "$f"
doneLength of output: 42076
Good news first: the two issues I flagged before are fixed:
I do still see one remaining issue before I’d call this ready:
That makes the public prop contract inconsistent with the implementation. In other words, I’d recommend splitting the props so only the container/resolved state includes: contentTypes?: ContentType[];
isFetching: boolean;and keeping So: runtime behavior looks good now, but I’d still hold this for that one typing cleanup before handing it to human review. |
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Changed files vs origin/develop =="
git diff --name-only origin/develop...HEAD
echo
echo "== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx --view expanded || true
echo
echo "== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
git diff --unified=120 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx =="
sed -n '1,260p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialog.tsx
echo
echo "== Outline: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
ast-grep outline ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx --view expanded || true
echo
echo "== Diff: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
git diff --unified=160 origin/develop...HEAD -- ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx =="
sed -n '1,340p' ui/app/src/components/ChangeContentTypeDialog/ChangeContentTypeDialogContainer.tsx
echo
echo "== Current file: ui/app/src/components/ChangeContentTypeDialog/utils.ts =="
sed -n '1,260p' ui/app/src/components/ChangeContentTypeDialog/utils.ts
echo
echo "== Search: props types and usages in ChangeContentTypeDialog subtree =="
rg -n -C3 'ChangeContentTypeDialogProps|ChangeContentTypeDialogBaseProps|ChangeContentTypeDialogStateProps|contentTypes\??:|isFetching:' ui/app/src/components/ChangeContentTypeDialog
echo
echo "== Search: ChangeContentTypeDialog public usages across repo =="
rg -n -C3 '\bChangeContentTypeDialog\b' ui/app/src
echo
echo "== Search: direct imports from utils.ts across repo =="
rg -n -C3 "from './utils'|from \"./utils\"|from '.*/ChangeContentTypeDialog/utils'|from \".*/ChangeContentTypeDialog/utils\"" ui/app/srcLength of output: 122348
What I checked:
I didn’t find any remaining issues in the files changed by this PR. From my side, this is ready for human review. ✅ Action performedReview finished.
|
1 participant
craftercms/craftercms#8419
Summary by CodeRabbit
New Features
Bug Fixes