fix(workflows): resolve text overflow issue in workflow rename header#24322
fix(workflows): resolve text overflow issue in workflow rename header#24322lukmaann wants to merge 1 commit intocalcom:mainfrom
Conversation
|
@lukmaann is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
WalkthroughThis PR refactors the workflows page (packages/features/ee/workflows/pages/workflow.tsx): consolidates TRPC query enabled flags; simplifies permission evaluation and read-only mode via readonlyWorkflow; unifies name editing handlers and submission; streamlines scroll-related effect; refactors form data setup, including isMixedEventType and activeOn calculations; compacts translation of reminderBody/emailSubject and step mappings; normalizes verification checks and error setting; centralizes validation error composition; routes saves through a single validate-and-submit path; and updates UI markup/classes for the header, name display (with Tooltip), badges, buttons, and DeleteDialog navigation. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
dosubot
bot
added
workflows
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/features/ee/workflows/pages/workflow.tsx (1)
259-290: Indexing validation errors by stepNumber risks misalignment; use array indexUsing
step.stepNumber - 1can set errors on the wrong item if ordering drifts. Index by iteration index.Apply:
- values.steps.forEach((step) => { + values.steps.forEach((step, index) => { const stripped = step.reminderBody?.replace(/<[^>]+>/g, "") || ""; const isBodyEmpty = !isSMSOrWhatsappAction(step.action) && !isCalAIAction(step.action) && stripped.length <= 1; if (isBodyEmpty) { - form.setError(`steps.${step.stepNumber - 1}.reminderBody`, { + form.setError(`steps.${index}.reminderBody`, { type: "custom", message: t("fill_this_field"), }); } @@ - if ( + if ( (step.action === WorkflowActions.SMS_NUMBER || step.action === WorkflowActions.WHATSAPP_NUMBER) && !verifiedNumbers?.find((v) => v.phoneNumber === step.sendTo) ) { isVerified = false; - form.setError(`steps.${step.stepNumber - 1}.sendTo`, { type: "custom", message: t("not_verified") }); + form.setError(`steps.${index}.sendTo`, { type: "custom", message: t("not_verified") }); } @@ - if (step.action === WorkflowActions.EMAIL_ADDRESS && !verifiedEmails?.find((v) => v === step.sendTo)) { + if (step.action === WorkflowActions.EMAIL_ADDRESS && !verifiedEmails?.find((v) => v === step.sendTo)) { isVerified = false; - form.setError(`steps.${step.stepNumber - 1}.sendTo`, { type: "custom", message: t("not_verified") }); + form.setError(`steps.${index}.sendTo`, { type: "custom", message: t("not_verified") }); } });Additionally, consider avoiding in‑place mutation of
values.stepswhen translating variables; clone and use a normalized payload to keep RHF state intact:const payloadSteps = values.steps.map((s) => ({ ...s, reminderBody: s.reminderBody ? translateVariablesToEnglish(s.reminderBody, { locale: i18n.language, t }) : s.reminderBody, emailSubject: s.emailSubject ? translateVariablesToEnglish(s.emailSubject, { locale: i18n.language, t }) : s.emailSubject, }));…and pass
steps: payloadStepsto the mutation.
🧹 Nitpick comments (6)
packages/features/ee/workflows/pages/workflow.tsx (6)
247-251: Localize error toast messageDirect string interpolation violates i18n guidance for .tsx; prefer
t().- if (err instanceof HttpError) { - showToast(`${err.statusCode}: ${err.message}`, "error"); - } + if (err instanceof HttpError) { + // Consider logging `err` for diagnostics, but show a localized toast + showToast(t("something_went_wrong"), "error"); + }As per coding guidelines
311-315: Avoid throwing on client validation; surface a toast and returnThrowing here can cause unhandled promise rejections depending on the Form wrapper. Prefer a toast + early return.
- throw new Error(`${t("workflow_validation_failed")}: ${errs.join("; ")}`); + showToast(`${t("workflow_validation_failed")}: ${errs.join("; ")}`, "error"); + return;
154-156: Limit smooth scroll to transition (pending → settled) to avoid jankPrevent repeated scrolls; respect reduced motion if needed.
- useEffect(() => { - requestAnimationFrame(() => window.scrollTo({ top: 0, behavior: "smooth" })); - }, [isPending]); + useEffect(() => { + if (prevIsPending.current && !isPending) { + requestAnimationFrame(() => window.scrollTo({ top: 0, behavior: "smooth" })); + } + prevIsPending.current = isPending; + }, [isPending]);Add once, outside:
// at top import { useRef } from "react"; // near other state const prevIsPending = useRef(isPending);
349-357: RHF integration: don’t override register’s onChange without forwardingOverriding
onChangeprevents RHF from tracking dirtiness/touched during editing.const nameField = form.register("name"); <Input {...nameField} data-testid="workflow-name" onChange={(e) => { setNameValue(e.target.value); nameField.onChange(e); }} onKeyDown={handleNameKeyDown} onBlur={handleNameSubmit} className="text-default focus:shadow-outline-gray-focused h-auto w-full whitespace-nowrap border-none p-1 text-sm font-semibold leading-none focus:ring-0" autoFocus />Alternatively, use Controller.
455-457: Prefer client-side navigation over full reloadUse Next.js router to navigate after delete.
// at top: import { useRouter } from "next/navigation"; const router = useRouter(); // ... additionalFunction={async () => { router.push("/workflows"); }}
359-366: Allow wrapping on mobile and truncation on larger screens
Switch the span’s classes to enable wrapping on small screens and truncate on sm+:- className="text-default hover:bg-muted min-w-0 cursor-pointer truncate whitespace-nowrap rounded p-1 text-sm font-semibold leading-none sm:min-w-[100px]" + className="text-default hover:bg-muted min-w-0 cursor-pointer break-words whitespace-normal rounded p-1 text-sm font-semibold leading-snug sm:whitespace-nowrap sm:truncate sm:min-w-[100px]"Consider disabling the Tooltip on touch devices where hover isn’t available.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/workflows/pages/workflow.tsx(15 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/pages/workflow.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/pages/workflow.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/pages/workflow.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/pages/workflow.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/pages/workflow.tsx (6)
packages/trpc/react/trpc.ts (2)
trpc(54-138)RouterOutputs(143-143)packages/lib/server/repository/workflow-permissions.ts (1)
WorkflowPermissions(4-9)packages/features/ee/workflows/lib/actionHelperFunctions.ts (3)
isSMSAction(25-27)isSMSOrWhatsappAction(33-35)isCalAIAction(37-39)packages/lib/constants.ts (1)
SENDER_ID(30-30)packages/ui/components/button/Button.tsx (1)
Button(221-349)packages/features/ee/workflows/components/WorkflowDetailsPage.tsx (1)
WorkflowDetailsPage(38-242)
🔇 Additional comments (5)
packages/features/ee/workflows/pages/workflow.tsx (5)
213-218: Verify sender/senderName mappingYou set
senderName = step.senderandsender = SENDER_IDfor non‑SMS. InWorkflowDetailsPage.addAction, non‑SMS usessenderName = SENDER_NAMEandsender = SENDER_ID. Please confirm DB semantics so UI displays a human "From name" for email steps and preserves SMS sender IDs for SMS steps. If needed, map like:// Example mapping (if applicable): senderName: isSMSAction(step.action) ? SENDER_NAME : step.sender ?? SENDER_NAME, sender: isSMSAction(step.action) ? step.sender : SENDER_ID,(Based on learnings)
331-336: Header container changes improve mobile resiliencemin-w-0 on flex containers and compact paddings look good for overflow control.
395-419: Action buttons sizing/alignmentUsing
size="sm", spacing tweaks and disabled states are appropriate for mobile.
427-438: Prop wiring to WorkflowDetailsPagePassing
permissions,allOptions, and handlers is consistent with the refactor.
96-105: Guard getVerifiedEmails query by teamId
Change theenabledflag toverifiedEmailsProp ? false : !!workflow?.team?.idso the query only fires whenteamIdis defined.
| if (!workflowData) return; | ||
| if (workflowData.userId && workflowData.activeOn.find((a) => !!a.eventType.teamId)) { | ||
| setIsMixedEventType(true); | ||
| } | ||
|
|
There was a problem hiding this comment.
Potential undefined access on activeOn
workflowData.activeOn.find(...) can throw if activeOn is undefined/null. Use optional chaining.
Apply:
- if (workflowData.userId && workflowData.activeOn.find((a) => !!a.eventType.teamId)) {
+ if (workflowData.userId && workflowData.activeOn?.find((a) => !!a.eventType.teamId)) {📝 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.
| if (!workflowData) return; | |
| if (workflowData.userId && workflowData.activeOn.find((a) => !!a.eventType.teamId)) { | |
| setIsMixedEventType(true); | |
| } | |
| if (!workflowData) return; | |
| if (workflowData.userId && workflowData.activeOn?.find((a) => !!a.eventType.teamId)) { | |
| setIsMixedEventType(true); | |
| } |
🤖 Prompt for AI Agents
In packages/features/ee/workflows/pages/workflow.tsx around lines 183 to 187,
the code accesses workflowData.activeOn without guarding against undefined which
can throw; update the condition to safely handle missing activeOn by using
optional chaining or a fallback array (e.g. workflowData.activeOn?.find(...) or
(workflowData.activeOn || []).find(...)) so the find call won't run on
undefined, and keep the existing userId check and setIsMixedEventType(true)
behavior.
|
closing in ref #24257 |
3 participants
What does this PR do?
This PR fixes the text overflow issue in the workflow rename header on mobile devices.
When users renamed workflows with long titles, the header layout broke and caused overflow issues, making it hard to use on small screens.
This update ensures:
Proper text wrapping for long workflow names
Improved alignment and spacing for mobile view
Consistent header layout and visual balance across devices
Fixes Mobile Responsiveness Issues on Workflows Page: Poor Layout and Text Overflow #24256
Visual Demo
Video Demo (Before and After):
Before:
bug-24257-before.mp4
After:
bug-24257-after.mp4
The before video shows how the workflow rename header overflowed on mobile when long names were entered.
The after video demonstrates the fixed behavior — titles now wrap correctly, and the layout remains intact and responsive.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/workflowson a mobile device or in browser DevTools (e.g., iPhone 14 view).Checklist