[8398] Default title/comment generation updates#4904
Conversation
…r and use it as default message when submitting
WalkthroughPublishOnDemandWidget now derives a mode-specific submission comment placeholder, no longer requires comment for form validity, and substitutes the placeholder when submitting blank comments. PublishOnDemandForm receives the placeholder and uses it for the Submission Comment field. ChangesPublish-on-demand comment placeholder
Sequence Diagram(s)sequenceDiagram
participant User
participant PublishOnDemandWidget
participant PublishOnDemandForm
PublishOnDemandWidget->>PublishOnDemandWidget: derive submissionCommentPlaceholder from selectedMode and publishingTarget
PublishOnDemandWidget->>PublishOnDemandForm: pass submissionCommentPlaceholder
PublishOnDemandForm-->>User: render Submission Comment placeholder
User->>PublishOnDemandWidget: submit with blank comment
PublishOnDemandWidget->>PublishOnDemandWidget: replace blank comment with submissionCommentPlaceholder
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/app/src/components/PublishOnDemandWidget/PublishOnDemandWidget.tsx (1)
203-225: 🎯 Functional Correctness | 🔵 TrivialSimplify dependency by using
currentFormDatadirectly.The use of
useUpdateRefshere is functionally safe because it updatesref.currentsynchronously during render, so there is no risk of stale data. However, depending onrefs.current.currentFormDatain theuseMemodependency array is less explicit than depending on the value variable itself. IfcurrentFormDatais available in the component scope and holds the same value, use it directly to improve clarity without changing behavior.♻️ Suggested simplification
const submissionCommentPlaceholder = useMemo(() => { let comment = ''; - const formData = refs.current.currentFormData; + const formData = currentFormData; if (selectedMode === 'everything' && formData.publishingTarget) { @@ - }, [selectedMode, refs.current.currentFormData, formatMessage]); + }, [selectedMode, currentFormData, formatMessage]);🤖 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/PublishOnDemandWidget/PublishOnDemandWidget.tsx` around lines 203 - 225, The dependency array for the `useMemo` that builds `submissionCommentPlaceholder` should use `currentFormData` directly instead of `refs.current.currentFormData`, since `useUpdateRefs` already keeps the ref in sync and the scoped value is clearer. Update the `submissionCommentPlaceholder` memo in `PublishOnDemandWidget` to reference the existing `currentFormData` variable for dependencies while keeping the same formatting logic for the `selectedMode` branches.
🤖 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.
Nitpick comments:
In `@ui/app/src/components/PublishOnDemandWidget/PublishOnDemandWidget.tsx`:
- Around line 203-225: The dependency array for the `useMemo` that builds
`submissionCommentPlaceholder` should use `currentFormData` directly instead of
`refs.current.currentFormData`, since `useUpdateRefs` already keeps the ref in
sync and the scoped value is clearer. Update the `submissionCommentPlaceholder`
memo in `PublishOnDemandWidget` to reference the existing `currentFormData`
variable for dependencies while keeping the same formatting logic for the
`selectedMode` branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fae2f6e0-32b4-4fd8-acfe-9db60378c817
📒 Files selected for processing (2)
ui/app/src/components/PublishOnDemandForm/PublishOnDemandForm.tsxui/app/src/components/PublishOnDemandWidget/PublishOnDemandWidget.tsx
|
@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 base =="
git diff --name-only origin/develop...HEAD || git diff --name-only HEAD~1...HEAD
echo
echo "== Diffstat =="
git diff --stat origin/develop...HEAD || git diff --stat HEAD~1...HEAD
echo
echo "== Outline: PublishOnDemandForm.tsx =="
FORM_FILE=$(fd 'PublishOnDemandForm.tsx' . | head -n1)
echo "$FORM_FILE"
ast-grep outline "$FORM_FILE" --view expanded || true
echo
echo "== Outline: PublishOnDemandWidget.tsx =="
WIDGET_FILE=$(fd 'PublishOnDemandWidget.tsx' . | head -n1)
echo "$WIDGET_FILE"
ast-grep outline "$WIDGET_FILE" --view expanded || true
echo
echo "== Relevant diff: PublishOnDemandForm.tsx =="
git diff origin/develop...HEAD -- "$FORM_FILE" || git diff HEAD~1...HEAD -- "$FORM_FILE"
echo
echo "== Relevant diff: PublishOnDemandWidget.tsx =="
git diff origin/develop...HEAD -- "$WIDGET_FILE" || git diff HEAD~1...HEAD -- "$WIDGET_FILE"
echo
echo "== Current file slice: PublishOnDemandForm.tsx =="
sed -n '1,260p' "$FORM_FILE"
echo
echo "== Current file slice: PublishOnDemandWidget.tsx =="
sed -n '1,340p' "$WIDGET_FILE"
echo
echo "== Search: submissionCommentPlaceholder usages =="
rg -n -C2 'submissionCommentPlaceholder|isBlank\\(comment\\)|comment:' ui/app/src || true
echo
echo "== Search: PublishOnDemandForm callsites =="
rg -n -C2 '<PublishOnDemandForm|PublishOnDemandForm\\(' ui/app/src || true
echo
echo "== Search: comment validation / required markers near widget =="
rg -n -C3 'required|setValidity|isBlank\\(|comment\\s*[:=]' ui/app/src/components/PublishOnDemandWidget ui/app/src/components/PublishOnDemandForm || trueLength of output: 50283
What I checked:
Overall this looks aligned with the PR objective and ready for human review. One quick manual sanity check I’d still recommend before handoff: leave the comment empty, switch modes / publishing targets, and verify the visible placeholder and submitted payload both reflect the latest target. ✅ Action performedReview finished.
|
1 participant
Updates in the PublishOnDemand component:
craftercms/craftercms#8398
Summary by CodeRabbit
New Features
Bug Fixes