feat(FormBuilder): morph action button label with TextMorph#118
Conversation
…lish Consumers need to await the publish toggle to freeze UI state until the server response settles. isSaving surfaces setValue.loading so components don't reach into formResource internals.
frappe-ui's setValue.submit mutates doc optimistically, making isDirty briefly true before originalDoc syncs — causing two reactive renders and two TextMorph transitions per click. Freeze the button config on click and release only after the settled state is stable. Also integrates TextMorph for animated label transitions and fixes the :loading binding to reflect setValue state (not just fetch state).
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughFrontend dependency management updated with addition of Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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. Review rate limit: 0/1 reviews remaining, refill in 50 minutes and 54 seconds.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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/stores/editForm.ts`:
- Around line 172-186: Compute the intended new publish state before calling
formResource.value.setValue.submit (e.g. const newIsPublished =
!formResource.value.doc.is_published) and use that flag in both onSuccess and
onError handlers instead of reading formResource.value.doc inside those
callbacks; update the success toast to show "Form published successfully" or
"Form unpublished successfully" based on newIsPublished and update the error
toast to "Failed to publish form" or "Failed to unpublish form" accordingly
(refer to setValue.submit and formResource.value.doc.is_published to locate the
code).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32f67d7f-b06c-46d3-81be-a5bb3a5a9ab8
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
frontend/package.jsonfrontend/src/components/FormBuilderHeader.vuefrontend/src/stores/editForm.ts
| return formResource.value.setValue.submit( | ||
| { | ||
| is_published: !formResource.value.doc.is_published, | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
| if (formResource.value.doc.is_published) { | ||
| toast.success("Form published successfully"); | ||
| } else { | ||
| toast.info("Form unpublished successfully"); | ||
| } | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
| if (formResource.value.doc.is_published) { | ||
| toast.success("Form published successfully"); | ||
| } else { | ||
| toast.info("Form unpublished successfully"); | ||
| } | ||
| }, | ||
| onError: () => { | ||
| toast.error("Failed to publish form"); | ||
| }, | ||
| } | ||
| ); | ||
| } | ||
| onError: () => { | ||
| toast.error("Failed to publish form"); | ||
| }, |
There was a problem hiding this comment.
Use action-specific failure text in togglePublish.
On Line 185, the error toast always says “Failed to publish form”, even when the attempted action was unpublish.
Suggested fix
function togglePublish() {
if (!formResource.value?.doc) return Promise.resolve();
+ const willPublish = !formResource.value.doc.is_published;
return formResource.value.setValue.submit(
{
- is_published: !formResource.value.doc.is_published,
+ is_published: willPublish,
},
{
onSuccess: () => {
- if (formResource.value.doc.is_published) {
+ if (willPublish) {
toast.success("Form published successfully");
} else {
toast.info("Form unpublished successfully");
}
},
onError: () => {
- toast.error("Failed to publish form");
+ toast.error(
+ willPublish ? "Failed to publish form" : "Failed to unpublish form"
+ );
},
}
);
}📝 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.
| return formResource.value.setValue.submit( | |
| { | |
| is_published: !formResource.value.doc.is_published, | |
| }, | |
| { | |
| onSuccess: () => { | |
| if (formResource.value.doc.is_published) { | |
| toast.success("Form published successfully"); | |
| } else { | |
| toast.info("Form unpublished successfully"); | |
| } | |
| }, | |
| { | |
| onSuccess: () => { | |
| if (formResource.value.doc.is_published) { | |
| toast.success("Form published successfully"); | |
| } else { | |
| toast.info("Form unpublished successfully"); | |
| } | |
| }, | |
| onError: () => { | |
| toast.error("Failed to publish form"); | |
| }, | |
| } | |
| ); | |
| } | |
| onError: () => { | |
| toast.error("Failed to publish form"); | |
| }, | |
| function togglePublish() { | |
| if (!formResource.value?.doc) return Promise.resolve(); | |
| const willPublish = !formResource.value.doc.is_published; | |
| return formResource.value.setValue.submit( | |
| { | |
| is_published: willPublish, | |
| }, | |
| { | |
| onSuccess: () => { | |
| if (willPublish) { | |
| toast.success("Form published successfully"); | |
| } else { | |
| toast.info("Form unpublished successfully"); | |
| } | |
| }, | |
| onError: () => { | |
| toast.error( | |
| willPublish ? "Failed to publish form" : "Failed to unpublish form" | |
| ); | |
| }, | |
| } | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/stores/editForm.ts` around lines 172 - 186, Compute the intended
new publish state before calling formResource.value.setValue.submit (e.g. const
newIsPublished = !formResource.value.doc.is_published) and use that flag in both
onSuccess and onError handlers instead of reading formResource.value.doc inside
those callbacks; update the success toast to show "Form published successfully"
or "Form unpublished successfully" based on newIsPublished and update the error
toast to "Failed to publish form" or "Failed to unpublish form" accordingly
(refer to setValue.submit and formResource.value.doc.is_published to locate the
code).
…t compatibility TextMorph keeps exiting chars in DOM during animation, so Playwright resolves the button's accessible name as merged old+new text. aria-label bypasses inner text entirely, giving tests a stable selector.
1 participant
Summary
torphdep and integratesTextMorphfor animated action button label transitionssetValue.submitmutatesdocoptimistically beforeoriginalDocsyncs, causing a briefisDirty=truewindow that triggers extra reactive renders)isSavingfrom the edit form store and makestogglePublishreturn a promise so consumers can await settled stateHow the fix works
On click, the button config is snapshotted before the handler mutates doc state. The snapshot is held until the
setValue.submitpromise resolves and Vue's deepisDirtywatcher has had a tick to settle — then the live config takes over. Double-clicks are ignored while frozen.Test plan
Summary by CodeRabbit
Bug Fixes