[_]: feat/compose message dialog and dialog manager context#21
Open
[_]: feat/compose message dialog and dialog manager context#21
Conversation
📝 WalkthroughWalkthroughAdds a dialog manager provider and centralizes compose-message dialog wiring; introduces a Compose Message dialog with TipTap editor, toolbar primitives, recipient validation, click-outside hook and tests, new i18n keys, and a default user name constant. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested reviewers
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 22
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/App.tsx (1)
21-23: 🧹 Nitpick | 🔵 TrivialAdd
dispatchto the dependency array.The
useEffectdepends ondispatchbut it's not listed in the dependency array. Whiledispatchfrom Redux is typically stable, including it satisfies the exhaustive-deps rule.Proposed fix
useEffect(() => { dispatch(initializeUserThunk()); - }, []); + }, [dispatch]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 21 - 23, The useEffect calling initializeUserThunk omits dispatch from its dependency array; update the dependency array on the useEffect that calls dispatch(initializeUserThunk()) to include dispatch (e.g., useEffect(() => { dispatch(initializeUserThunk()); }, [dispatch])) so the hook lists its dependency and satisfies the exhaustive-deps rule—target the useEffect where dispatch and initializeUserThunk are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.tsx`:
- Around line 18-19: The variable name isNewMessageDialogOpen misrepresents the
destructured function from useActionDialog which is a generic dialog checker;
rename the destructured symbol back to a generic name (e.g., isDialogOpen or
isActionDialogOpen) and update its usage where you call it to derive
isComposeMessageDialogOpen (the call using ActionDialog.ComposeMessage should
remain unchanged) so the function name accurately reflects that it can check any
dialog; update references to isNewMessageDialogOpen across the file (including
the declaration from useActionDialog and the isComposeMessageDialogOpen
assignment) to the chosen generic name.
In `@src/components/compose-message/components/editorBar/config.ts`:
- Line 1: The file currently disables TypeScript checks and uses `any` in the
handlePaste function; remove the top-line eslint-disable and replace `any` with
TipTap/DOM types — e.g., import `type { Editor } from '@tiptap/core'` and type
the paste handler parameters as `editor: Editor` and `event: ClipboardEvent` (or
the ProseMirror signature `view: EditorView, event: ClipboardEvent, slice:
Slice` if that matches the implementation). Update the handlePaste function
signature to use those types and adjust any internal references if needed so
explicit TipTap/DOM types are used instead of `any`.
In `@src/components/compose-message/components/editorBar/EditorBarButton.tsx`:
- Around line 1-16: EditorBarButton currently renders icon-only buttons without
an accessible name and doesn't expose the active state to assistive tech; update
the EditorBarButton component to accept an accessible name prop (e.g., ariaLabel
or label) or derive one from children and pass it to the rendered <button> as
aria-label/aria-labelledby, and also map the visual isActive prop to
aria-pressed on the button (aria-pressed={!!isActive}) while preserving disabled
and onClick behavior so screen readers receive both the name and the toggle
state.
In `@src/components/compose-message/components/editorBar/index.tsx`:
- Around line 25-28: Rename the props interface from ActionBarProps to
EditorBarProps to match the component name EditorBar; update the interface
declaration (currently ActionBarProps), any places that import or reference
ActionBarProps (including the EditorBar component's props type and any exports)
to use EditorBarProps instead, and ensure the exported type name is updated so
all consumers compile without type errors.
- Around line 92-111: Rename the misspelled constant textAligment to
textAlignment and update all references to it in this module (e.g., where the
array is passed to the editor bar rendering/mapping logic) so the identifier
matches the correct spelling; ensure the constant still satisfies
EditorBarItem[] and that any onClick/isActive closures (editor.chain...) remain
unchanged.
In `@src/components/compose-message/components/RecipientInput.tsx`:
- Line 65: Replace the hardcoded fallback string in the RecipientInput component
(the expression name={recipient?.name || 'My Internxt'}) with a translated
fallback using the app i18n context: import and use the translation function
(e.g., useTranslation / t) in RecipientInput and change the fallback to
t('recipient.defaultName') (or the appropriate translation key) so the UI uses
localized text; ensure the translation key exists in the locale files and update
any prop-types/TS types if needed.
- Around line 61-69: The UserChip list uses recipient.email as the React key
which can duplicate; update the key prop in the recipients.map to use
recipient.id (the canonical unique id produced by crypto.randomUUID()) instead
of recipient.email so each <UserChip ... key={recipient.id} /> is stable and
unique; ensure the onRemoveRecipient call still uses recipient.id and keep
recipient.email only for the email prop.
- Around line 36-46: Add basic email validation so the component only accepts
well-formed addresses: in handleKeyDown (and mirror the same logic in
handleBlur) validate the trimmed value (inputValue.trim().replace(/,$/, ''))
against a simple email regex before calling onAddRecipient; if the value fails
validation, do not call onAddRecipient and do not clear inputValue (optionally
set a validation state/flag to show an error). Reference handleKeyDown,
handleBlur, inputValue, recipients, and onAddRecipient when making the change.
In `@src/components/compose-message/components/RichTextEditor.tsx`:
- Around line 3-5: The RichTextEditor currently types its prop as editor:
Editor; change RichTextEditorProps to allow editor: Editor | null (or optional)
and ensure the component passes that value directly to TipTap's EditorContent
(which accepts Editor | null) so the component no longer requires the parent to
guard; update any internal references in RichTextEditor (and usages that pass
readyEditor) to handle null safely (e.g., conditional calls or early returns
where methods are used) and update the prop type wherever RichTextEditorProps or
the component signature is referenced.
In `@src/components/compose-message/hooks/useEditorBar.ts`:
- Around line 45-60: Create two GitHub issues to track replacing
globalThis.prompt with a custom modal: one titled "Custom modal for URL
attachment" referencing the useEditorBar.ts function setLink (replace
globalThis.prompt in setLink) and one titled "Custom modal for image attachment"
referencing addImage (replace globalThis.prompt in addImage). In each issue
include a short description, acceptance criteria (UI mock or behavior: open
modal, validate input, allow cancel, return value), mention the file/component
(useEditorBar.ts / compose-message), and tag frontend or UX for prioritization.
- Around line 28-43: Extract the repeated click-outside logic in useEditorBar
into a reusable hook (e.g., useClickOutside) that takes a ref (like
colorPickerRef, fontPickerRef, sizePickerRef) and a callback (e.g., () =>
setShowColorPicker(false), etc.), registers a single document mousedown
listener, calls the callback when event.target is outside the ref.current, and
cleans up the listener on unmount; then replace the three inline handlers in
useEditorBar with three calls to useClickOutside for each picker, and import the
new hook into useEditorBar.
In `@src/components/compose-message/index.tsx`:
- Line 136: The TODO for attachment handling is untracked; create a GitHub issue
describing the required attachment feature (accepting files, preview, upload
flow, validation, and API contract) and then update the TODO comment in
src/components/compose-message/index.tsx to reference that issue (e.g., "TODO:
implement attachments — see `#ISSUE`"). In the ComposeMessage component implement
a minimal scaffold: add a handleAttachments handler (or prop) and an attachments
state placeholder, wire a file input/button UI stub to the handler so behavior
is testable, and add a small unit/integration test that asserts the file-input
handler is called; finally ensure the updated TODO includes the issue number so
work is tracked.
- Around line 54-58: Remove the debug console.log inside handlePrimaryAction:
eliminate the console.log('html', html) call and either remove the unused html
variable if it's not needed or use it for the intended action before calling
onClose; update the useCallback body in handlePrimaryAction (which references
readyEditor.getHTML and onClose) so there are no leftover debug logs.
- Around line 64-70: The overlay button in the ComposeMessage component
currently rendered with onClick={onClose} and data-testid="dialog-overlay" lacks
an accessible label; add an aria-label (for example aria-label="Close dialog" or
similar) to that button element so screen readers can describe its purpose while
leaving the onClose handler and data-testid unchanged.
- Around line 47-48: The component currently discards the value returned by
useEditor and instead stores the instance via onCreate into readyEditor/state,
which can cause divergence; change useEditor to capture its return value (const
editor = useEditor({ ...EDITOR_CONFIG }) ) and remove the onCreate handler and
the readyEditor/setReadyEditor state, then update all references of readyEditor
to use the editor variable (keeping EDITOR_CONFIG unchanged) so the hook-managed
editor instance is the single source of truth.
- Line 26: The code unsafely casts the result of
getComposeMessageDialogData(ActionDialog.ComposeMessage) to DraftMessage when
that function returns unknown|null; add a runtime type guard (e.g.,
isDraftMessage) to validate the shape before assigning to draft, use it to set
draft to the validated value or a safe default (empty DraftMessage) and update
usages accordingly; ensure the type guard checks required DraftMessage fields
and reference getComposeMessageDialogData, ActionDialog.ComposeMessage,
DraftMessage, and the draft variable so reviewers can find and apply the fix.
In `@src/components/user-chip/index.tsx`:
- Around line 34-41: The close icon's click currently bubbles to parent
elements; update the XIcon onClick to first call event.stopPropagation() and
then invoke the onRemove handler so the click doesn't trigger parent click
handlers; locate the XIcon usage (props: onRemove, position) and replace the
direct onClick={onRemove} with a wrapper that stops propagation before calling
onRemove.
- Around line 35-40: The XIcon remove control (XIcon with onClick={onRemove}) is
not keyboard accessible; make it focusable and operable via keyboard by adding a
tabIndex={0}, role="button" (or use a <button> wrapper), an accessible label
(aria-label or aria-labelledby), and a keyDown handler that invokes onRemove
when Enter or Space is pressed; ensure the existing onClick/onRemove behavior
and the position-based opacity class remain unchanged.
In `@src/context/dialog-manager/DialogManager.context.tsx`:
- Line 20: The current close-update uses [dialogKey]: {
...prevDialogs[dialogKey], isOpen: false, data: null } which can produce an
object missing ActionDialogState.key when prevDialogs[dialogKey] is undefined;
update the state update in DialogManager.context.tsx so the object always
includes key: dialogKey (e.g. [dialogKey]: { key: dialogKey,
...prevDialogs[dialogKey], isOpen: false, data: null }) to preserve the
consistent ActionDialogState shape for functions that rely on .key.
In `@src/context/dialog-manager/types/index.ts`:
- Line 17: The declared signature closeDialog: (key: ActionDialog, config?:
DialogActionConfig) => void exposes an unused optional config and should match
the actual contract; remove the unused parameter by changing the type to
closeDialog: (key: ActionDialog) => void in the types/index.ts declaration and
then update all implementations/usages of closeDialog (search for closeDialog
references) to stop passing a config argument or adapt them to the new
single-parameter API so the type and implementations are consistent.
In `@src/context/dialog-manager/useActionDialog.tsx`:
- Line 21: The return expression in useActionDialog is currently converting
valid falsy payloads to null by using the || operator; update the expression
that reads ctx.actionDialogs[key]?.data || null to use the nullish coalescing
operator so it returns falsy values like false, 0, or '' correctly (i.e., change
to ctx.actionDialogs[key]?.data ?? null) within the function that accesses
actionDialogs.
In `@src/i18n/locales/en.json`:
- Around line 119-126: Align the casing of CC/BCC labels: update the keys inside
composeMessageDialog ("cc" and "bcc") to use the same casing found in the
top-level labels section (e.g., "CC" and "BCC") so the UI is consistent; locate
the composeMessageDialog object and change the string values for "cc" and "bcc"
to match the existing "labels" entries (or vice versa if you prefer the other
style) and run a quick search for other occurrences to keep all label usages
consistent.
---
Outside diff comments:
In `@src/App.tsx`:
- Around line 21-23: The useEffect calling initializeUserThunk omits dispatch
from its dependency array; update the dependency array on the useEffect that
calls dispatch(initializeUserThunk()) to include dispatch (e.g., useEffect(() =>
{ dispatch(initializeUserThunk()); }, [dispatch])) so the hook lists its
dependency and satisfies the exhaustive-deps rule—target the useEffect where
dispatch and initializeUserThunk are used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ae657505-3278-4745-a506-73a87e967179
📒 Files selected for processing (21)
src/App.tsxsrc/components/Sidenav/index.tsxsrc/components/compose-message/components/RecipientInput.tsxsrc/components/compose-message/components/RichTextEditor.tsxsrc/components/compose-message/components/editorBar/EditorBarButton.tsxsrc/components/compose-message/components/editorBar/EditorBarGroup.tsxsrc/components/compose-message/components/editorBar/config.tssrc/components/compose-message/components/editorBar/extension/FontSize.tssrc/components/compose-message/components/editorBar/index.tsxsrc/components/compose-message/hooks/useComposeMessage.tssrc/components/compose-message/hooks/useEditorBar.tssrc/components/compose-message/index.tsxsrc/components/compose-message/types/index.tssrc/components/user-chip/index.tsxsrc/context/dialog-manager/DialogManager.context.tsxsrc/context/dialog-manager/index.tssrc/context/dialog-manager/types/index.tssrc/context/dialog-manager/useActionDialog.tsxsrc/i18n/locales/en.jsonsrc/i18n/provider/TranslationProvider.tsxsrc/main.tsx
…ure/compose-message
xabg2
changed the base branch from
master
to
feature/custom-hooks-to-compose-new-messages
Base automatically changed from
feature/custom-hooks-to-compose-new-messages
to
master
March 24, 2026 09:46
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/compose-message/components/RecipientInput.tsx (1)
52-58:⚠️ Potential issue | 🟡 MinorApply email validation in
handleBluras well.The
isValidEmailcheck was added tohandleKeyDown(line 43) but not tohandleBlur. Users can still add invalid emails by typing and clicking away from the input field.🔧 Proposed fix
const handleBlur = () => { const email = inputValue.trim(); - if (email) { + if (email && isValidEmail(email)) { onAddRecipient(email); setInputValue(''); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/components/RecipientInput.tsx` around lines 52 - 58, handleBlur currently adds whatever is in inputValue without validation, allowing invalid emails; update handleBlur to call the same isValidEmail check used in handleKeyDown before calling onAddRecipient and only clear the input via setInputValue('') after a successful add (or optionally show/handle invalid input). Locate the handleBlur function and integrate the isValidEmail(email) guard (same validator used in handleKeyDown), call onAddRecipient(email) only when valid, and ensure inputValue is not cleared on invalid entries so users can correct it.
♻️ Duplicate comments (3)
src/components/compose-message/index.tsx (2)
52-56:⚠️ Potential issue | 🟡 MinorRemove debug
console.logbefore merging.As noted in a previous review, the
console.log('html', html)statement should be removed for production code.🧹 Suggested fix
const handlePrimaryAction = useCallback(() => { const html = editor?.getHTML(); - console.log('html', html); + // TODO: Send the email with html content onClose(); }, [editor, onClose]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/index.tsx` around lines 52 - 56, In handlePrimaryAction (the useCallback that references editor and onClose) remove the debug console.log('html', html) statement before merging; simply capture the editor HTML as needed and call onClose(), or replace the console.log with an appropriate production logger if telemetry/logging is required instead of leaving a console.debug.
26-26:⚠️ Potential issue | 🟡 MinorUnsafe type cast without validation.
As noted in a previous review,
getDialogDatareturnsunknown | null, but this is cast directly toDraftMessagewithout validation. This could cause runtime issues if unexpected data is passed.🛡️ Suggested fix with type guard
+const isDraftMessage = (data: unknown): data is DraftMessage => { + if (!data || typeof data !== 'object') return false; + const d = data as Record<string, unknown>; + return ( + (d.subject === undefined || typeof d.subject === 'string') && + (d.to === undefined || Array.isArray(d.to)) && + (d.cc === undefined || Array.isArray(d.cc)) && + (d.bcc === undefined || Array.isArray(d.bcc)) && + (d.body === undefined || typeof d.body === 'string') + ); +}; - const draft = (getComposeMessageDialogData(ActionDialog.ComposeMessage) ?? {}) as DraftMessage; + const rawData = getComposeMessageDialogData(ActionDialog.ComposeMessage); + const draft: DraftMessage = isDraftMessage(rawData) ? rawData : {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/index.tsx` at line 26, The code unsafely casts the result of getComposeMessageDialogData(ActionDialog.ComposeMessage) to DraftMessage; add a runtime type guard (e.g., isDraftMessage) that checks the expected DraftMessage properties and only assign the cast when the guard returns true, otherwise use a safe default (empty object or typed default). Locate the usage around the draft variable in compose-message (the getComposeMessageDialogData call and DraftMessage type) and replace the direct cast with a guarded check that validates the unknown | null return before treating it as DraftMessage.src/components/compose-message/components/editorBar/EditorBarButton.tsx (1)
8-17:⚠️ Potential issue | 🟡 MinorAdd
type="button"and accessible label to the button.The button is missing
type="button", which can cause unintended form submissions if used within a form context. Additionally, as noted in a previous review, icon-only buttons need anaria-labelfor screen reader accessibility.🔧 Proposed fix
export interface EditorBarButtonProps { onClick: () => void; + ariaLabel?: string; isActive?: boolean; disabled?: boolean; children: React.ReactNode; } -export const EditorBarButton = ({ onClick, isActive, disabled, children }: EditorBarButtonProps) => ( +export const EditorBarButton = ({ onClick, ariaLabel, isActive, disabled, children }: EditorBarButtonProps) => ( <button + type="button" onClick={onClick} disabled={disabled} + aria-label={ariaLabel} aria-pressed={isActive} className={`p-1 rounded transition-colors ${isActive ? 'bg-gray-10 text-primary' : 'hover:bg-gray-5 text-gray-60'} ${disabled ? 'opacity-50 cursor-not-allowed' : ''}`} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/components/editorBar/EditorBarButton.tsx` around lines 8 - 17, The EditorBarButton is missing an explicit type and an accessible label; update the EditorBarButton component to include type="button" on the <button> element and add an optional ariaLabel prop to the EditorBarButtonProps, then pass aria-label={ariaLabel} to the button (require callers to provide ariaLabel for icon-only buttons). Edit the EditorBarButton function signature to accept ariaLabel?: string alongside onClick, isActive, disabled, children and ensure the rendered <button> includes type="button" and aria-label={ariaLabel}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/compose-message/index.tsx`:
- Around line 138-140: The Paperclip attachment Button currently uses an empty
onClick handler (onClick={() => {}}) which gives no feedback; either remove the
onClick prop entirely or wire it to a placeholder action such as invoking an
existing UI feedback helper (e.g., call toast.info("Attachments coming soon") or
trigger openAttachmentModal/openAttachmentPicker if available). Update the
Button (the one rendering PaperclipIcon) to use a meaningful stub handler or
remove the prop so users receive feedback when they tap the attachments control.
- Line 89: The XIcon currently uses onClick={onClose} but is not
keyboard-accessible; wrap the XIcon in a semantic <button> (e.g., replace the
bare <XIcon onClick={onClose} ... /> with a <button type="button"
onClick={onClose} aria-label="Close" className="..."> containing <XIcon
className="..." /></button>) so it becomes focusable, reachable via keyboard,
and has an accessible name; ensure you preserve existing className styling (or
move it to the button) and remove the onClick from the raw XIcon to keep only
the button handling.
---
Outside diff comments:
In `@src/components/compose-message/components/RecipientInput.tsx`:
- Around line 52-58: handleBlur currently adds whatever is in inputValue without
validation, allowing invalid emails; update handleBlur to call the same
isValidEmail check used in handleKeyDown before calling onAddRecipient and only
clear the input via setInputValue('') after a successful add (or optionally
show/handle invalid input). Locate the handleBlur function and integrate the
isValidEmail(email) guard (same validator used in handleKeyDown), call
onAddRecipient(email) only when valid, and ensure inputValue is not cleared on
invalid entries so users can correct it.
---
Duplicate comments:
In `@src/components/compose-message/components/editorBar/EditorBarButton.tsx`:
- Around line 8-17: The EditorBarButton is missing an explicit type and an
accessible label; update the EditorBarButton component to include type="button"
on the <button> element and add an optional ariaLabel prop to the
EditorBarButtonProps, then pass aria-label={ariaLabel} to the button (require
callers to provide ariaLabel for icon-only buttons). Edit the EditorBarButton
function signature to accept ariaLabel?: string alongside onClick, isActive,
disabled, children and ensure the rendered <button> includes type="button" and
aria-label={ariaLabel}.
In `@src/components/compose-message/index.tsx`:
- Around line 52-56: In handlePrimaryAction (the useCallback that references
editor and onClose) remove the debug console.log('html', html) statement before
merging; simply capture the editor HTML as needed and call onClose(), or replace
the console.log with an appropriate production logger if telemetry/logging is
required instead of leaving a console.debug.
- Line 26: The code unsafely casts the result of
getComposeMessageDialogData(ActionDialog.ComposeMessage) to DraftMessage; add a
runtime type guard (e.g., isDraftMessage) that checks the expected DraftMessage
properties and only assign the cast when the guard returns true, otherwise use a
safe default (empty object or typed default). Locate the usage around the draft
variable in compose-message (the getComposeMessageDialogData call and
DraftMessage type) and replace the direct cast with a guarded check that
validates the unknown | null return before treating it as DraftMessage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88d23d5f-6817-452f-89df-e39238b9dcad
📒 Files selected for processing (7)
src/App.tsxsrc/components/compose-message/components/RecipientInput.tsxsrc/components/compose-message/components/RichTextEditor.tsxsrc/components/compose-message/components/editorBar/EditorBarButton.tsxsrc/components/compose-message/components/editorBar/index.tsxsrc/components/compose-message/index.tsxsrc/constants.ts
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/compose-message/components/RecipientInput.tsx (1)
73-73: 🧹 Nitpick | 🔵 TrivialFallback still not internationalized.
Using
DEFAULT_USER_NAMEis an improvement over the hardcoded'My Internxt'string, but it still doesn't address the localization concern raised in the previous review. For consistency with the rest of the application, consider using the translation context (e.g.,t('recipient.defaultName')).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/components/RecipientInput.tsx` at line 73, The name prop in RecipientInput currently falls back to the constant DEFAULT_USER_NAME (name={recipient?.name || DEFAULT_USER_NAME}); replace that fallback with the localized string from the translation context (e.g., use t('recipient.defaultName') or t('recipient.defaultName', { defaultValue: DEFAULT_USER_NAME })) so the UI uses i18n; ensure you import/use the same translation hook/context used in this component (e.g., const { t } = useTranslation()) and keep recipient?.name as the primary value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/compose-message/components/RecipientInput.tsx`:
- Around line 21-26: The isValidEmail function uses document.createElement which
is inefficient and breaks SSR; replace its DOM-based validation with a simple,
SSR-safe regex check (e.g. a pattern that ensures local@domain.tld like
/^[^\s@]+@[^\s@]+\.[^\s@]+$/) inside isValidEmail so it returns the boolean
result of the regex.test(email) instead of using document; keep the function
name isValidEmail and ensure it still accepts a string and returns a boolean.
In `@src/hooks/useClickOutside.ts`:
- Around line 3-12: The useClickOutside hook currently re-registers listeners on
every render and fires when the picker is hidden; modify useClickOutside(ref,
onClickOutside) to accept a third boolean parameter enabled, and inside the hook
store onClickOutside in a ref (e.g., savedCallbackRef.current = onClickOutside)
so the event handler can call savedCallbackRef.current() without being
recreated; in the effect only depend on ref and enabled (not the callback), and
early-return or skip calling the handler when enabled is false; update callers
in useEditorBar to pass the picker visibility (e.g.,
useClickOutside(colorPickerRef, closeColorPicker, showColorPicker)) for
color/font/size pickers.
---
Duplicate comments:
In `@src/components/compose-message/components/RecipientInput.tsx`:
- Line 73: The name prop in RecipientInput currently falls back to the constant
DEFAULT_USER_NAME (name={recipient?.name || DEFAULT_USER_NAME}); replace that
fallback with the localized string from the translation context (e.g., use
t('recipient.defaultName') or t('recipient.defaultName', { defaultValue:
DEFAULT_USER_NAME })) so the UI uses i18n; ensure you import/use the same
translation hook/context used in this component (e.g., const { t } =
useTranslation()) and keep recipient?.name as the primary value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9fcac69-e505-4807-9b3f-f3eb5ded711f
📒 Files selected for processing (4)
src/components/compose-message/components/RecipientInput.tsxsrc/components/compose-message/hooks/useEditorBar.tssrc/hooks/useClickOutside.test.tssrc/hooks/useClickOutside.ts
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/compose-message/components/RecipientInput.tsx (1)
52-53:⚠️ Potential issue | 🟡 MinorNormalize blur input the same way as the Enter/comma path.
Line 52 currently uses
trim()only, while Line 41 also strips a trailing comma. This creates inconsistent add-recipient behavior between keyboard submit and blur.✏️ Proposed fix
- const email = inputValue.trim(); + const email = inputValue.trim().replace(/,$/, '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/compose-message/components/RecipientInput.tsx` around lines 52 - 53, The blur handler in RecipientInput uses inputValue.trim() which differs from the Enter/comma path that also strips a trailing comma; update the blur path to normalize inputValue the same way (trim and remove a single trailing comma) before validating/adding—i.e., transform inputValue the same way you do in the Enter/comma flow (use the same normalization logic used where isValidEmail is called) so the check around const email = ... and the subsequent isValidEmail(email) call behaves identically.src/utils/is-valid-email/index.ts (1)
1-5:⚠️ Potential issue | 🟠 MajorGuard
documentusage inisValidEmailto prevent non-browser runtime failures.Line 2 unconditionally uses
document.createElement(...). In SSR/node contexts this can throw and break recipient parsing paths that reuse this utility.💡 Proposed fix
+const EMAIL_FALLBACK_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + -export const isValidEmail = (email: string) => { - const input = document.createElement('input'); - input.type = 'email'; - input.value = email; - return input.checkValidity(); -}; +export const isValidEmail = (email: string): boolean => { + if (typeof document === 'undefined') { + return EMAIL_FALLBACK_RE.test(email); + } + + const input = document.createElement('input'); + input.type = 'email'; + input.value = email; + return input.checkValidity(); +};Run this to verify whether this utility can execute in non-DOM runtimes:
#!/bin/bash set -euo pipefail echo "== isValidEmail usages ==" rg -n --type=ts --type=tsx '\bisValidEmail\b|is-valid-email' echo echo "== Vitest config environments (node/jsdom) ==" fd -HI 'vitest.config.*' -x sh -c 'echo "--- $1"; sed -n "1,240p" "$1"' sh {} echo echo "== Build/runtime config hints for SSR/server paths ==" fd -HI 'vite.config.*' -x sh -c 'echo "--- $1"; sed -n "1,260p" "$1"' sh {} rg -n --type=ts --type=tsx 'renderToString|hydrateRoot|ssr|server' srcExpected confirmation: if any execution path is node/SSR, current Line 2 is unsafe without a guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/is-valid-email/index.ts` around lines 1 - 5, The isValidEmail utility currently calls document.createElement unconditionally and will throw in non-browser runtimes; update isValidEmail to first guard for a DOM environment (e.g., check typeof document !== 'undefined' and typeof document.createElement === 'function') and only use the input.checkValidity() approach when those checks pass, otherwise fall back to a safe alternative (for example return a simple regex-based validation or false) so server/SSR/node usages won't crash; locate the isValidEmail function in src/utils/is-valid-email/index.ts and implement this guard and fallback logic around the DOM usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/compose-message/components/RecipientInput.tsx`:
- Around line 52-53: The blur handler in RecipientInput uses inputValue.trim()
which differs from the Enter/comma path that also strips a trailing comma;
update the blur path to normalize inputValue the same way (trim and remove a
single trailing comma) before validating/adding—i.e., transform inputValue the
same way you do in the Enter/comma flow (use the same normalization logic used
where isValidEmail is called) so the check around const email = ... and the
subsequent isValidEmail(email) call behaves identically.
In `@src/utils/is-valid-email/index.ts`:
- Around line 1-5: The isValidEmail utility currently calls
document.createElement unconditionally and will throw in non-browser runtimes;
update isValidEmail to first guard for a DOM environment (e.g., check typeof
document !== 'undefined' and typeof document.createElement === 'function') and
only use the input.checkValidity() approach when those checks pass, otherwise
fall back to a safe alternative (for example return a simple regex-based
validation or false) so server/SSR/node usages won't crash; locate the
isValidEmail function in src/utils/is-valid-email/index.ts and implement this
guard and fallback logic around the DOM usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2baa460-4cec-426f-b2cb-44d1c231b8d0
📒 Files selected for processing (6)
src/components/Sidenav/index.tsxsrc/components/compose-message/components/RecipientInput.tsxsrc/utils/bytes-to-string/bytesToString.test.tssrc/utils/bytes-to-string/index.tssrc/utils/is-valid-email/index.tssrc/utils/is-valid-email/isValidEmail.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding a new component to Compose messages (styling the text with different sizes, colors, using bold, italic, crossed text, etc).
Also a simple Dialog Manager context has been added so we can open, close and pass params to Dialogs easily.