Conversation
Introduce an optional from field for email outputs and propagate it through rendering and sending. Changes include: adding from to the email control Zod schema and framework JSON schema, exposing from in the EmailRenderOutput DTO, returning from in the API email output renderer (both raw and sanitized branches), and using bridge output from (email and name) in the worker send-email usecase to override integration defaults for from and senderName. This enables per-step custom sender addresses/names when provided.
Introduce a SenderConfigDrawer component and wire it into the email editor/preview to allow editing the sender name/email per email step. EmailPreviewHeader now shows an edit button and reads values from the form (falling back to integration defaults). EmailEditor toggles the drawer and restricts body editing to DEV environment. sanitize-control-values now includes the from field and tightens typing for email controls. Also remove a Sentry breadcrumb and add a console.log for bridge outputs in the email send usecase for debugging.
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
bot
added
@novu/shared
@novu/worker
@novu/dashboard
@novu/api-service
labels
github-actions
bot
changed the title
WalkthroughAdds an optional 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/worker/src/app/workflow/usecases/send-message/send-message-email.usecase.ts (1)
159-181:⚠️ Potential issue | 🟠 MajorRemove raw bridge output logging to avoid PII leakage (Line 160).
bridgeOutputscan include email body/subject/sender data; logging it verbatim is a privacy/compliance risk andconsole.logbypasses structured logging controls. Please remove this or replace with a redacted debug log gated to non‑prod.🔧 Suggested fix (redacted debug log)
- console.log('bridgeOptions', JSON.stringify(bridgeOutputs, null, 2)); + if (process.env.NODE_ENV !== 'production') { + Logger.debug({ hasBridgeOutputs: !!bridgeOutputs }, 'Bridge outputs received', LOG_CONTEXT); + }apps/api/src/app/environments-v1/usecases/output-renderers/email-output-renderer.usecase.ts (1)
128-147:⚠️ Potential issue | 🟠 MajorGuard against empty sender overrides in output.
...(from && { from })will include{}or empty strings. If downstream uses nullish coalescing, that can override integration defaults with empty values. Compute ahasFrompredicate and only include when meaningful.✅ Suggested guard
async execute(renderCommand: EmailOutputRendererCommand): Promise<EmailRenderOutput> { const { body, subject: controlSubject, disableOutputSanitization, layoutId: stepLayoutId, from, } = renderCommand.controlValues as EmailControlType; + const hasFrom = Boolean(from?.email?.trim() || from?.name?.trim()); if (!body || typeof body !== 'string') { return { subject: controlSubject as string, body: body as string, - ...(from && { from }), + ...(hasFrom && { from }), }; } @@ if (disableOutputSanitization) { return { subject: translatedSubject, body: cleanedHtml, - ...(from && { from }), + ...(hasFrom && { from }), }; } @@ return { subject: translatedSubject, body: sanitizedBody, - ...(from && { from }), + ...(hasFrom && { from }), }; }Also applies to: 207-221
🤖 Fix all issues with AI agents
In `@apps/dashboard/src/components/workflow-editor/steps/email/email-preview.tsx`:
- Around line 40-49: The sender line currently always renders a clickable
<button> with onClick={onEditSenderClick} even when onEditSenderClick is
undefined, producing a focusable but non-functional control; update the
rendering in the email-preview component so that when onEditSenderClick is
truthy you render the interactive <button> as now (keeping className, children:
displaySenderName and displaySenderEmail and the RiEdit2Line icon), and when
onEditSenderClick is falsy render a non-interactive element (e.g., a <div> or a
<span>) or render the button with disabled and aria-disabled attributes so it is
not focusable/interactive; ensure the visual styling and the presence/absence of
the edit icon match the clickability and that the accessible name
(displaySenderName + displaySenderEmail) remains unchanged.
- Around line 20-26: EmailPreviewHeader currently calls useFormContext()
directly (which throws if no FormProvider exists) and then attempts optional
chaining on values; change EmailPreviewHeader to accept an optional prop (e.g.,
from?: { email?: string; name?: string }) and use that prop as the primary
source for from.email/from.name, falling back to calling useFormContext() only
when the prop is undefined OR, alternatively, guard the useFormContext() call by
wrapping it in a try/catch and treating missing provider as undefined; update
usages (StepPreview, LayoutEmailEditor, ConfigureEmailStepPreview) to pass from
when they can, and in EmailPreviewHeader derive
displaySenderName/displaySenderEmail from prop values first, then from
formContext.watch('from.email') / watch('from.name'), then the existing
senderName/senderEmail/defaults.
In `@libs/application-generic/src/schemas/control/email-control.schema.ts`:
- Around line 15-20: The current `from` schema allows empty objects/strings;
update the `from` field so that when an object is provided it must include a
non-empty, valid email and an optional non-empty name: replace the current
`from: z.object({ email: z.string().optional(), name: z.string().optional()
}).optional()` with an optional object where `email:
z.string().email().nonempty()` and `name: z.string().nonempty().optional()` so
an empty `{}` or `email: ""`/`name: ""` will fail validation (use
`z.string().email().nonempty()` and `z.string().nonempty().optional()` in the
`from` object).
In `@packages/shared/src/dto/workflows/preview-step-response.dto.ts`:
- Around line 20-26: The DTO classes RenderOutput, ChatRenderOutput,
SmsRenderOutput, PushRenderOutput, EmailRenderOutput, InAppRenderOutput, and
DelayRenderOutput are plain data holders and should be converted to TypeScript
interfaces; replace each class declaration with an interface declaration
preserving all property names and types (including optional properties like
EmailRenderOutput.from.email/name), keep any exported names the same, and ensure
any imports/uses referencing these types continue to work (update any "new"
usages if present to use plain objects). Make no changes to
DigestOutputProcessor (it remains a class).
🧹 Nitpick comments (5)
apps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsx (2)
15-16: Useis...boolean naming for drawer state.
This improves readability and aligns with the boolean naming guideline.As per coding guidelines: “Use descriptive variable names with auxiliary verbs (isLoading, hasError)”.✏️ Rename suggestion
- const [senderDrawerOpen, setSenderDrawerOpen] = useState(false); + const [isSenderDrawerOpen, setIsSenderDrawerOpen] = useState(false); @@ - <EmailPreviewHeader minimalHeader={isEditorV2} onEditSenderClick={() => setSenderDrawerOpen(true)}> + <EmailPreviewHeader minimalHeader={isEditorV2} onEditSenderClick={() => setIsSenderDrawerOpen(true)}> @@ - <SenderConfigDrawer open={senderDrawerOpen} onOpenChange={setSenderDrawerOpen} /> + <SenderConfigDrawer open={isSenderDrawerOpen} onOpenChange={setIsSenderDrawerOpen} />
2-50: Consider lazy-loadingSenderConfigDrawersince it's opened on demand.This reduces the initial bundle size; wrap it in
Suspensewith a fallback. Per the coding guidelines: "Use dynamic loading for non-critical components" and "Wrap client components in Suspense with fallback".💤 Lazy-load the drawer
-import { useState } from 'react'; +import { lazy, Suspense, useState } from 'react'; @@ -import { SenderConfigDrawer } from '@/components/workflow-editor/steps/email/sender-config-drawer'; +const SenderConfigDrawer = lazy(() => + import('@/components/workflow-editor/steps/email/sender-config-drawer').then((module) => ({ + default: module.SenderConfigDrawer, + })) +); @@ - <SenderConfigDrawer open={senderDrawerOpen} onOpenChange={setSenderDrawerOpen} /> + <Suspense fallback={null}> + <SenderConfigDrawer open={senderDrawerOpen} onOpenChange={setSenderDrawerOpen} /> + </Suspense>apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsx (3)
92-95: Redundanttrigger()calls aftersetValuewithshouldValidate: true.Since
setValueis already called withshouldValidate: true, the subsequenttrigger()call is redundant and causes double validation.♻️ Proposed fix to remove redundant trigger calls
onBlur={() => { setValue('from.name', localName, { shouldDirty: true, shouldValidate: true }); - trigger(); }}onBlur={() => { setValue('from.email', localEmail, { shouldDirty: true, shouldValidate: true }); - trigger(); }}Also applies to: 117-120
33-47: Same redundanttrigger()pattern in toggle handler.The
trigger()call on line 46 is also redundant since bothsetValuecalls above already useshouldValidate: true.♻️ Proposed fix
- const handleToggleDefaults = async (checked: boolean) => { + const handleToggleDefaults = (checked: boolean) => { if (checked) { setValue('from.email', undefined, { shouldDirty: true, shouldValidate: true }); setValue('from.name', undefined, { shouldDirty: true, shouldValidate: true }); setLocalEmail(''); setLocalName(''); } else { setValue('from.email', '', { shouldDirty: true, shouldValidate: true }); setValue('from.name', '', { shouldDirty: true, shouldValidate: true }); setLocalEmail(''); setLocalName(''); } - - await trigger(); };
17-19: Consider adding type parameter touseFormContextfor improved type safety.The
useFormContext()call lacks a type parameter, which reduces type safety for form field names and values. If a form type exists for the email step configuration, consider using it.- const { control, watch, setValue, trigger } = useFormContext(); + const { control, watch, setValue, trigger } = useFormContext<EmailStepFormType>();
| const formContext = useFormContext(); | ||
| const fromEmail = formContext?.watch('from.email'); | ||
| const fromName = formContext?.watch('from.name'); | ||
|
|
||
| const displaySenderName = senderName || 'Acme Inc.'; | ||
| const displaySenderEmail = senderEmail || 'noreply@novu.co'; | ||
| const displaySenderName = fromName || senderName || 'Acme Inc.'; | ||
| const displaySenderEmail = fromEmail || senderEmail || 'noreply@novu.co'; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "EmailPreviewHeader" apps/dashboard -C 3Repository: novuhq/novu
Length of output: 13315
🏁 Script executed:
# Also search for the component definition to understand its structure
cat -n apps/dashboard/src/components/workflow-editor/steps/email/email-preview.tsx | head -50Repository: novuhq/novu
Length of output: 2629
🏁 Script executed:
# Search for FormProvider usage around the email preview component
rg -n "FormProvider" apps/dashboard/src/components/workflow-editor -C 2Repository: novuhq/novu
Length of output: 1947
🏁 Script executed:
# Check step-preview-hover-card.tsx context around EmailPreviewHeader usage
sed -n '1,100p' apps/dashboard/src/components/step-preview-hover-card.tsxRepository: novuhq/novu
Length of output: 2703
🏁 Script executed:
# Check layout-email-editor.tsx for FormProvider context
cat -n apps/dashboard/src/components/layouts/layout-email-editor.tsxRepository: novuhq/novu
Length of output: 1246
🏁 Script executed:
# Check email-preview-wrapper.tsx for FormProvider context
head -100 apps/dashboard/src/components/workflow-editor/steps/preview/previews/email-preview-wrapper.tsxRepository: novuhq/novu
Length of output: 3324
🏁 Script executed:
# Check email-editor-preview.tsx for FormProvider context
head -80 apps/dashboard/src/components/workflow-editor/steps/email/email-editor-preview.tsxRepository: novuhq/novu
Length of output: 2643
🏁 Script executed:
# Check email-editor.tsx for FormProvider - we know it has onEditSenderClick so likely has form context
head -50 apps/dashboard/src/components/workflow-editor/steps/email/email-editor.tsxRepository: novuhq/novu
Length of output: 2440
🏁 Script executed:
# Check configure-email-step-preview.tsx
head -60 apps/dashboard/src/components/workflow-editor/steps/email/configure-email-step-preview.tsxRepository: novuhq/novu
Length of output: 2356
🏁 Script executed:
# Search for FormProvider usage in workflow-editor directory to understand context
rg -n "FormProvider" apps/dashboard/src/components/workflow-editor --include="*.tsx" -B 2 -A 2Repository: novuhq/novu
Length of output: 496
🏁 Script executed:
# Check email-editor.tsx usage - where is it imported/used
rg -n "EmailEditor\b" apps/dashboard/src/components -A 2 -B 2Repository: novuhq/novu
Length of output: 4915
🏁 Script executed:
# Check the structure around email-preview-wrapper and how it's used
rg -n "EmailCorePreview\|EmailPreviewWrapper" apps/dashboard/src/components -B 2 -A 2Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check step-editor-factory.tsx for FormProvider context
rg -n "FormProvider" apps/dashboard/src/components/workflow-editor/steps/editor/step-editor-factory.tsxRepository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check email-tabs.tsx which wraps EmailEditor
cat -n apps/dashboard/src/components/workflow-editor/steps/email/email-tabs.tsxRepository: novuhq/novu
Length of output: 2400
🏁 Script executed:
# Check step-preview-hover-card.tsx more broadly for FormProvider
rg -n "FormProvider" apps/dashboard/src/components/step-preview-hover-card.tsxRepository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check if there's a higher-level workflow form context
rg -n "FormProvider|useFormContext" apps/dashboard/src/components/workflow-editor/workflow-provider.tsx -A 5 -B 5Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check parent component of step-editor-factory
rg -n "StepEditorFactory" apps/dashboard/src/components -B 5 -A 5Repository: novuhq/novu
Length of output: 4501
🏁 Script executed:
# Check step-editor-layout.tsx to see if FormProvider is provided
cat -n apps/dashboard/src/components/workflow-editor/steps/step-editor-layout.tsx | head -150Repository: novuhq/novu
Length of output: 7591
🏁 Script executed:
# Check if StepEditorProvider or other providers wrap FormProvider
rg -n "FormProvider" apps/dashboard/src/components/workflow-editor/steps/context/ -B 3 -A 3Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check StepPreviewFactory to see how preview components are used
head -80 apps/dashboard/src/components/workflow-editor/steps/preview/step-preview-factory.tsxRepository: novuhq/novu
Length of output: 2621
🏁 Script executed:
# Check if step-preview-hover-card context has FormProvider
rg -n "StepPreview\|FormProvider" apps/dashboard/src/components/step-preview-hover-card.tsx -B 5 -A 5Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check StepEditorProvider to see if it provides FormProvider
cat -n apps/dashboard/src/components/workflow-editor/steps/context/step-editor-context.tsx | head -100Repository: novuhq/novu
Length of output: 4065
🏁 Script executed:
# Check what context provides the form for EmailTabs
rg -n "StepEditorProvider\|FormProvider" apps/dashboard/src/components/workflow-editor/steps/context/step-editor-context.tsx -A 10 -B 2Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Search for where EmailTabs is actually used - it should have form context
rg -n "EmailTabs" apps/dashboard/src/components/workflow-editor -B 5 -A 5Repository: novuhq/novu
Length of output: 8599
🏁 Script executed:
# Find where StepEditorLayout is used to see if FormProvider wraps it
rg -n "StepEditorLayout" apps/dashboard/src/components -B 5 -A 5Repository: novuhq/novu
Length of output: 2436
🏁 Script executed:
# Check step-preview-hover-card parent usage
rg -n "StepPreview\(" apps/dashboard/src/components -B 3 -A 3 | head -60Repository: novuhq/novu
Length of output: 2442
🏁 Script executed:
# Check if there's a FormProvider at the page level where these are used
rg -n "FormProvider" apps/dashboard/src --max-count=20Repository: novuhq/novu
Length of output: 1022
🏁 Script executed:
# Check what page/layout wraps StepEditorLayout - find the workflow editor page
rg -n "StepEditorLayout" apps/dashboard/src --include="*.tsx" -B 10 | head -50Repository: novuhq/novu
Length of output: 496
🏁 Script executed:
# Look for FormProvider wrapper at the page level
rg -n "FormProvider\|useForm" apps/dashboard/src/pages --include="*.tsx" -A 3 -B 3 | head -80Repository: novuhq/novu
Length of output: 496
🏁 Script executed:
# Confirm: does step-preview-hover-card StepPreview get used with a form context or standalone?
rg -n "step-preview-hover-card\|StepPreview" apps/dashboard/src --include="*.tsx" -B 3 -A 3 | head -100Repository: novuhq/novu
Length of output: 496
🏁 Script executed:
# Check what page/layout wraps StepEditorLayout
rg -n "StepEditorLayout" apps/dashboard/src -B 10 | head -80Repository: novuhq/novu
Length of output: 4676
🏁 Script executed:
# Look for FormProvider wrapper at the page level - search pages directory
rg -n "FormProvider" apps/dashboard/src/pages -B 3 -A 3 | head -100Repository: novuhq/novu
Length of output: 37
🏁 Script executed:
# Check step-preview-hover-card usage context
rg -n "step-preview-hover-card" apps/dashboard/src -B 5 -A 5Repository: novuhq/novu
Length of output: 2293
🏁 Script executed:
# Check base-node.tsx usage context - where is it used and is it inside a form?
rg -n "base-node" apps/dashboard/src -B 3 -A 3Repository: novuhq/novu
Length of output: 3695
🏁 Script executed:
# Check if workflow-editor (where base-node is used) has FormProvider
cat -n apps/dashboard/src/pages/edit-workflow-template-v2.tsx | head -100Repository: novuhq/novu
Length of output: 141
🏁 Script executed:
# Verify the exact usage of StepPreview in base-node
sed -n '1,150p' apps/dashboard/src/components/workflow-editor/base-node.tsxRepository: novuhq/novu
Length of output: 6104
🏁 Script executed:
# Find where workflow pages are and check if FormProvider wraps the whole page
fd -type f -name "*.tsx" apps/dashboard/src/pages | xargs grep -l "workflow"Repository: novuhq/novu
Length of output: 227
🏁 Script executed:
# Check one workflow editor page to see how it's structured
cat -n apps/dashboard/src/pages/workflows.tsx | head -80Repository: novuhq/novu
Length of output: 3906
Guard useFormContext() or make form data optional props.
EmailPreviewHeader calls useFormContext() at line 20, which throws synchronously if no provider is present. Optional chaining on lines 21-22 doesn't prevent this crash. Multiple call sites render it without a FormProvider:
step-preview-hover-card.tsx:53– used in standaloneStepPreviewpreview hoverlayout-email-editor.tsx:20– used inLayoutEmailEditorwithout form contextconfigure-email-step-preview.tsx:25– used inConfigureEmailStepPreviewwithout form context
Either wrap all call sites in a FormProvider or refactor EmailPreviewHeader to accept from data as optional props with fallback to hook, preventing crashes when no provider exists.
🤖 Prompt for AI Agents
In `@apps/dashboard/src/components/workflow-editor/steps/email/email-preview.tsx`
around lines 20 - 26, EmailPreviewHeader currently calls useFormContext()
directly (which throws if no FormProvider exists) and then attempts optional
chaining on values; change EmailPreviewHeader to accept an optional prop (e.g.,
from?: { email?: string; name?: string }) and use that prop as the primary
source for from.email/from.name, falling back to calling useFormContext() only
when the prop is undefined OR, alternatively, guard the useFormContext() call by
wrapping it in a try/catch and treating missing provider as undefined; update
usages (StepPreview, LayoutEmailEditor, ConfigureEmailStepPreview) to pass from
when they can, and in EmailPreviewHeader derive
displaySenderName/displaySenderEmail from prop values first, then from
formContext.watch('from.email') / watch('from.name'), then the existing
senderName/senderEmail/defaults.
| <button | ||
| type="button" | ||
| onClick={onEditSenderClick} | ||
| className="group flex items-center gap-1 text-left hover:text-foreground-950 focus:outline-none" | ||
| > | ||
| {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span> | ||
| </> | ||
| {onEditSenderClick && ( | ||
| <RiEdit2Line className="text-foreground-600 size-3.5 opacity-0 transition-opacity group-hover:opacity-100" /> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Avoid rendering a non-functional button when no edit handler is provided.
When onEditSenderClick is undefined, the button is still focusable but does nothing. Consider rendering a non-button element or a disabled/aria-disabled button.
💡 One way to keep behavior explicit
const displaySenderName = fromName || senderName || 'Acme Inc.';
const displaySenderEmail = fromEmail || senderEmail || 'noreply@novu.co';
+ const senderContent = onEditSenderClick ? (
+ <button
+ type="button"
+ onClick={onEditSenderClick}
+ className="group flex items-center gap-1 text-left hover:text-foreground-950 focus:outline-none"
+ >
+ {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span>
+ <RiEdit2Line className="text-foreground-600 size-3.5 opacity-0 transition-opacity group-hover:opacity-100" />
+ </button>
+ ) : (
+ <div className="flex items-center gap-1 text-left">
+ {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span>
+ </div>
+ );
return (
<div className={cn('flex gap-2', className)} {...rest}>
@@
{isLoading ? (
<Skeleton className="h-4 w-40" />
) : (
- <button
- type="button"
- onClick={onEditSenderClick}
- className="group flex items-center gap-1 text-left hover:text-foreground-950 focus:outline-none"
- >
- {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span>
- {onEditSenderClick && (
- <RiEdit2Line className="text-foreground-600 size-3.5 opacity-0 transition-opacity group-hover:opacity-100" />
- )}
- </button>
+ senderContent
)}📝 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.
| <button | |
| type="button" | |
| onClick={onEditSenderClick} | |
| className="group flex items-center gap-1 text-left hover:text-foreground-950 focus:outline-none" | |
| > | |
| {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span> | |
| </> | |
| {onEditSenderClick && ( | |
| <RiEdit2Line className="text-foreground-600 size-3.5 opacity-0 transition-opacity group-hover:opacity-100" /> | |
| )} | |
| </button> | |
| const senderContent = onEditSenderClick ? ( | |
| <button | |
| type="button" | |
| onClick={onEditSenderClick} | |
| className="group flex items-center gap-1 text-left hover:text-foreground-950 focus:outline-none" | |
| > | |
| {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span> | |
| <RiEdit2Line className="text-foreground-600 size-3.5 opacity-0 transition-opacity group-hover:opacity-100" /> | |
| </button> | |
| ) : ( | |
| <div className="flex items-center gap-1 text-left"> | |
| {displaySenderName} <span className="text-foreground-600 text-xs">{`<${displaySenderEmail}>`}</span> | |
| </div> | |
| ); | |
| return ( | |
| <div className={cn('flex gap-2', className)} {...rest}> | |
| {isLoading ? ( | |
| <Skeleton className="h-4 w-40" /> | |
| ) : ( | |
| senderContent | |
| )} | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/components/workflow-editor/steps/email/email-preview.tsx`
around lines 40 - 49, The sender line currently always renders a clickable
<button> with onClick={onEditSenderClick} even when onEditSenderClick is
undefined, producing a focusable but non-functional control; update the
rendering in the email-preview component so that when onEditSenderClick is
truthy you render the interactive <button> as now (keeping className, children:
displaySenderName and displaySenderEmail and the RiEdit2Line icon), and when
onEditSenderClick is falsy render a non-interactive element (e.g., a <div> or a
<span>) or render the button with disabled and aria-disabled attributes so it is
not focusable/interactive; ensure the visual styling and the presence/absence of
the edit icon match the clickability and that the accessible name
(displaySenderName + displaySenderEmail) remains unchanged.
Refactor the SenderConfigDrawer to use local state for sender name/email and a local "use defaults" toggle, initializing values when the drawer opens. Replace reactive watch/trigger usage with getValues and saveForm (force submit) to persist changes, add a Save button in a SheetFooter, and restructure layout using SheetMain/SheetFooter with separators. Simplify form inputs by removing FormField wrappers, enhance placeholders and tooltips, and disable inputs when provider defaults are used. Also tweak the EmailPreview header to render the sender email inside angle brackets with a dotted underline and make the edit icon consistently visible.
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsx`:
- Line 3: Remove the external RiInformation2Line import and all usages of
RiInformation2Line in sender-config-drawer.tsx; instead either render the
corresponding icon SVG from the Figma payload (use your project's Figma asset
helper/component to load the Dev Mode/MCP Server asset and swap it in where
RiInformation2Line is used) or drop the icon and rely on the existing Tooltip
component to convey the info—search for the RiInformation2Line symbol in the
file and replace each occurrence with the Figma asset loader or remove the
element if redundant.
🧹 Nitpick comments (3)
apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsx (3)
28-28: Consider adding type parameter touseFormContext.The
useFormContext()call is untyped, which reduces type safety forgetValuesandsetValuecalls. If a form schema type exists, consider usinguseFormContext<YourFormType>()to get proper type inference.
87-88: Add blank line before return statement.Per coding guidelines, there should be a blank line before return statements for readability.
✨ Suggested fix
onOpenChange(false); }; + return ( <Sheet open={open} onOpenChange={onOpenChange}>As per coding guidelines: "Add blank lines before return statements."
100-100: Hardcodedbg-whitemay not work with dark mode.The
bg-whiteclass is hardcoded, which might cause visibility issues if the dashboard supports dark mode. Consider using a theme-aware background class if available (e.g.,bg-backgroundor similar from your design system).
| @@ -0,0 +1,194 @@ | |||
| import { useEffect, useState } from 'react'; | |||
| import { useFormContext } from 'react-hook-form'; | |||
| import { RiInformation2Line } from 'react-icons/ri'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for react-icons usage in the dashboard
echo "=== Searching for react-icons imports in dashboard ==="
rg "from ['\"]react-icons" apps/dashboard/src -t ts -t tsx -A 1 -B 1 | head -50
echo ""
echo "=== Count of react-icons imports ==="
rg "from ['\"]react-icons" apps/dashboard/src -t ts -t tsx | wc -l
echo ""
echo "=== Checking file structure of sender-config-drawer.tsx ==="
head -20 apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsxRepository: novuhq/novu
Length of output: 1172
🏁 Script executed:
#!/bin/bash
# View the full file to see how RiInformation2Line is used
cat apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsx | grep -A 5 -B 5 "RiInformation2Line" | head -30Repository: novuhq/novu
Length of output: 1585
Remove react-icons import; use Figma assets instead.
The RiInformation2Line import from react-icons violates the coding guideline stating all assets in the dashboard must come from the Figma payload, not external icon packages. Replace the imported icon with an icon from the Figma Dev Mode MCP Server or remove it if the information can be conveyed through the existing Tooltip component.
🤖 Prompt for AI Agents
In
`@apps/dashboard/src/components/workflow-editor/steps/email/sender-config-drawer.tsx`
at line 3, Remove the external RiInformation2Line import and all usages of
RiInformation2Line in sender-config-drawer.tsx; instead either render the
corresponding icon SVG from the Figma payload (use your project's Figma asset
helper/component to load the Dev Mode/MCP Server asset and swap it in where
RiInformation2Line is used) or drop the icon and rely on the existing Tooltip
component to convey the info—search for the RiInformation2Line symbol in the
file and replace each occurrence with the Figma asset loader or remove the
element if redundant.
scopsy
changed the title
scopsy
deleted the
nv-6950-custom-sender-name-in-email-step-on-new-dashboard
branch
1 participant
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer