Conversation
|
Warning Ignoring CodeRabbit configuration file changes. For security, only the configuration from the base branch is applied for open source repositories. WalkthroughSe elimina la mayor parte del diálogo de composición de mensajes (RichTextEditor, EditorBar, RecipientInput, ComposeMessageDialog, tipos y componentes adjuntos), se refactoriza el Sidenav y rutas (se agrega Spam), se reemplaza la utilidad de formateo de bytes por una nueva implementación con prettysize y se añaden componentes de UI de correo (Header, SearchInput, TrayEmptyState) y tipos asociados. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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: 9
🤖 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/features/mail/components/header/index.tsx`:
- Around line 60-68: The Checkbox component in the header lacks an accessible
label; update the Checkbox usage (the Checkbox element inside the flex row) to
provide an accessible name by either adding a descriptive aria-label prop (e.g.,
aria-label="Select all messages") or associating it with a visible or
visually-hidden <label> element tied to the Checkbox; ensure the label text
clearly describes the control (e.g., "Select all" or "Toggle selection") so
screen readers can identify it.
- Around line 40-51: The bulkActionContext menu currently uses XIcon for both
entries; update the archive entry (the object whose name is
translate('actions.archiveAll')) to use an appropriate archive icon (e.g.,
ArchiveIcon) instead of XIcon and add the corresponding import from
'@phosphor-icons/react' at the top of the file so the second item's icon
property references ArchiveIcon rather than XIcon.
In `@src/features/mail/components/searchInput/index.tsx`:
- Around line 28-31: Agregar un estado query en el componente SearchInput y
usarlo para controlar el input: crea const [query, setQuery] =
useState<string>('') dentro de SearchInput; pasar value={query} y onChange={(e)
=> setQuery(e.target.value)} al elemento input (manteniendo searchInput ref) y
reemplazar la cadena vacía hardcodeada en la llamada a
getClearButtonClassName('') por getClearButtonClassName(query); al limpiar
(botón clear) llamar setQuery('') y volver a enfocar con
searchInput.current?.focus() para que el botón de borrar aparezca y funcione
correctamente.
- Around line 62-68: The onKeyUpCapture handler is too late to stop default
arrow-key cursor movements; change the input's onKeyUpCapture to
onKeyDownCapture (or onKeyDown) on the same element in
src/features/mail/components/searchInput/index.tsx, moving the existing logic so
that Escape still calls e.currentTarget.blur() and ArrowUp/ArrowDown call
e.preventDefault() on keydown; keep the same key checks ('Escape', 'ArrowUp',
'ArrowDown') and ensure the event parameter type matches a KeyboardEvent for the
input element when you update the prop name.
- Around line 43-45: La firma de handleSubmit usa el tipo inexistente
React.SubmitEvent; cámbialo a un tipo válido como
SubmitEventHandler<HTMLFormElement> o React.SyntheticEvent<HTMLFormElement> (por
ejemplo: const handleSubmit = (e: SubmitEventHandler<HTMLFormElement> |
React.SyntheticEvent<HTMLFormElement>) => ...) para corregir la tipificación,
importando el tipo desde React si es necesario y manteniendo e.preventDefault()
dentro de la función; asegúrate de actualizar la declaración de handleSubmit en
el componente searchInput para reflejar el nuevo tipo.
In `@src/features/mail/components/trayEmptyState/index.tsx`:
- Line 14: The JSX currently concatenates folderName with
translate('mail.tray.isEmpty'), which breaks localization ordering; update the
component in trayEmptyState (where translate is called) to pass the folder name
as a variable via the translate props (e.g., translate('mail.tray.isEmpty', {
folder: folderName })), and update the translation key in locales
(mail.tray.isEmpty) to use interpolation like "{{folder}} is empty" so languages
can reorder the placeholder correctly.
In `@src/features/mail/MailView.tsx`:
- Around line 5-8: La unión de tipos FolderType no incluye 'spam', lo que choca
con el uso en MailView (prop folder en MailViewProps); actualiza la definición
de FolderType (símbolo FolderType en src/types/mail/index.ts) para añadir 'spam'
a la unión de literales (por ejemplo: 'inbox' | 'sent' | 'drafts' | 'trash' |
'spam') y reexporta/compila para que el componente MailView y la ruta que pasa
folder: 'spam' queden tipados correctamente.
In `@src/routes/layouts/SidebarAndHeaderLayout.tsx`:
- Line 3: Renombra el alias de importación `SidenavWrapper` a `Sidenav` y
actualiza todas las referencias en este módulo (por ejemplo cualquier uso de
`SidenavWrapper` en `SidebarAndHeaderLayout` o líneas alrededor de la import)
para usar `Sidenav` en su lugar; busca también la otra ocurrencia mencionada
(`SidenavWrapper` en la misma file around line 11) y cámbiala para mantener
consistencia de nombre y mejorar legibilidad.
In `@src/types/mail/index.ts`:
- Line 1: The FolderType union is missing the 'spam' variant which the app uses
in navigation/routes; update the exported type FolderType to include 'spam'
(i.e., add 'spam' to the union) so all code using FolderType (e.g., route
guards, navigation components) remains type-safe and aligned with runtime
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba2341c3-8ca3-4606-b5fd-364e06d8ec21
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (28)
package.jsonsrc/components/Sidenav/index.tsxsrc/components/chips/RecipientChip.tsxsrc/components/mail/composeMessageDialog/components/DefaultAttachmentItem.tsxsrc/components/mail/composeMessageDialog/components/RecipientInput.tsxsrc/components/mail/composeMessageDialog/components/RichTextEditor.tsxsrc/components/mail/composeMessageDialog/components/editorBar/EditorBarButton.tsxsrc/components/mail/composeMessageDialog/components/editorBar/EditorBarGroup.tsxsrc/components/mail/composeMessageDialog/components/editorBar/index.tsxsrc/components/mail/composeMessageDialog/components/fontSizeExtension.tssrc/components/mail/composeMessageDialog/index.tsxsrc/components/mail/composeMessageDialog/types/index.tssrc/components/mail/sidenav/index.tsxsrc/features/mail/MailView.tsxsrc/features/mail/components/header/index.tsxsrc/features/mail/components/searchInput/index.tsxsrc/features/mail/components/trayEmptyState/index.tsxsrc/hooks/navigation/useSidenavNavigation.tsxsrc/i18n/locales/en.jsonsrc/index.csssrc/routes/layouts/SidebarAndHeaderLayout.tsxsrc/routes/paths/index.tssrc/types/mail/index.tssrc/types/prettysize.d.tssrc/utils/bytesToString.test.tssrc/utils/bytesToString.tssrc/utils/bytesToString/bytesToString.test.tssrc/utils/bytesToString/index.ts
💤 Files with no reviewable changes (12)
- src/utils/bytesToString.ts
- src/components/mail/composeMessageDialog/components/editorBar/EditorBarButton.tsx
- src/components/mail/composeMessageDialog/components/editorBar/EditorBarGroup.tsx
- src/components/mail/composeMessageDialog/index.tsx
- src/components/mail/composeMessageDialog/components/RecipientInput.tsx
- src/components/mail/composeMessageDialog/components/RichTextEditor.tsx
- src/components/mail/composeMessageDialog/types/index.ts
- src/components/mail/composeMessageDialog/components/DefaultAttachmentItem.tsx
- src/components/mail/composeMessageDialog/components/fontSizeExtension.ts
- src/components/mail/composeMessageDialog/components/editorBar/index.tsx
- src/utils/bytesToString.test.ts
- src/components/chips/RecipientChip.tsx
|
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.coderabbit.yaml (1)
11-20:⚠️ Potential issue | 🟠 MajorLos
path_instructions.pathcon coma pueden dejar reglas sin aplicarse.Ahora mismo cada
pathmezcla dos globs en una sola cadena. Separarlos evita que las instrucciones queden inactivas por patrón no coincidente.🔧 Propuesta de ajuste
path_instructions: - - path: '**/*.ts,**/*.tsx' + - path: '**/*.ts' instructions: | Review the TypeScript/React code for: - Possible bugs and unhandled edge cases - Performance issues and unnecessary re-renders - Type safety improvements - Clean code principles and readability - Security vulnerabilities - - path: '**/*.test.ts,**/*.test.tsx' + - path: '**/*.tsx' + instructions: | + Review the TypeScript/React code for: + - Possible bugs and unhandled edge cases + - Performance issues and unnecessary re-renders + - Type safety improvements + - Clean code principles and readability + - Security vulnerabilities + - path: '**/*.test.ts' + instructions: | + Review tests for: + - Missing edge case coverage + - Proper assertion usage + - Test isolation and mocking correctness + - path: '**/*.test.tsx' instructions: | Review tests for: - Missing edge case coverage - Proper assertion usage - Test isolation and mocking correctness🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 11 - 20, El problema es que la clave path en la sección de path_instructions contiene dos globs combinados en una sola cadena ('**/*.ts,**/*.tsx'), lo que puede dejar una de las reglas sin aplicarse; actualiza la configuración para separar los globs en elementos distintos (por ejemplo como un array de paths o como entradas separadas) y asegurarte de que cada entrada mantenga su bloque instructions; revisa y corregir las apariciones de path_instructions.path y la sección que hoy tiene '**/*.test.ts,**/*.test.tsx' para que cada patrón sea independiente y las instrucciones asociadas se ejecuten correctamente.
♻️ Duplicate comments (2)
src/features/mail/components/searchInput/index.tsx (1)
55-77:⚠️ Potential issue | 🟠 MajorEl “clear” no limpia el campo: falta
valuecontrolado y handler de click.En Line 76 se muestra el icono, pero no ejecuta ninguna limpieza; además el input no está ligado a
query, así que el estado no puede resetear el texto visible.🔧 Propuesta de corrección
<input ref={searchInput} id="globalSearchInput" data-cy="globalSearchInput" autoComplete="off" spellCheck="false" type="text" + value={query} className="inxt-input left-icon h-10 w-full appearance-none rounded-lg border border-transparent bg-gray-5 px-9 text-lg text-gray-100 placeholder-gray-60 outline-none ring-1 ring-gray-10 transition-all duration-150 ease-out hover:shadow-sm hover:ring-gray-20 focus:border-primary focus:bg-surface focus:placeholder-gray-80 focus:shadow-none focus:ring-3 focus:ring-primary/10 dark:focus:bg-gray-1 dark:focus:ring-primary/20" onKeyUpCapture={(e) => { if (e.key === 'Escape') { e.currentTarget.blur(); } }} onChange={(e) => setQuery(e.target.value)} @@ - <XIcon className={getClearButtonClassName(query, openSearchBox)} size={20} /> + <XIcon + className={getClearButtonClassName(query, openSearchBox)} + size={20} + onClick={() => { + setQuery(''); + searchInput.current?.focus(); + }} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/mail/components/searchInput/index.tsx` around lines 55 - 77, The clear icon doesn't reset the visible text because the input isn't controlled and the XIcon has no click handler; update the input JSX to bind value={query} so it reflects state (searchInput ref and setQuery remain unchanged), and add an onClick handler to the XIcon (referencing XIcon and getClearButtonClassName) that calls setQuery('') and then either focuses searchInput.current.focus() or blurs it per desired UX; also make the XIcon accessible by adding an appropriate aria-label/role or wrapping it in a button element so keyboard users can trigger the clear action.src/features/mail/components/header/index.tsx (1)
61-61:⚠️ Potential issue | 🟡 MinorEl
Checkboxsigue sin nombre accesible.Añade
aria-labelo una etiqueta asociada para lectores de pantalla.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/mail/components/header/index.tsx` at line 61, El Checkbox en el componente header (renderizado como <Checkbox />) carece de nombre accesible; actualiza la instancia de Checkbox para proporcionar un nombre visible para lectores de pantalla añadiendo un atributo aria-label (por ejemplo aria-label="Seleccionar todos los mensajes") o envolviéndolo con una etiqueta <label> asociada, asegurándote de usar la prop correcta del componente Checkbox si es de una librería (p. ej. ariaLabel, aria-label, label) y mantener el texto claro y localizado; modifica la línea donde se renderiza Checkbox para pasar esa prop o agregar el label asociado.
🤖 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/features/mail/components/header/index.tsx`:
- Around line 13-51: The menu items in listActionContext and bulkActionContext
have no-op actions (action: () => {}) so selecting them does nothing; replace
those no-ops with calls to the real filter and bulk-action handlers used by the
mail feature (e.g., invoke the component prop or context methods that apply
filters and perform bulk operations). Specifically, update listActionContext
items to call the existing filter handler (e.g., props.onFilter or
mailStore.applyFilter) with the appropriate filter keys like
'all'|'none'|'read'|'unread'|'starred'|'unstarred', and update bulkActionContext
items to call the bulk handlers (e.g., props.onBulkAction or
mailService.trashAll/mailService.archiveAll) instead of empty functions; ensure
the actions also close the menu and update component state or dispatch events as
the rest of the header component expects (refer to listActionContext,
bulkActionContext, MenuItemType, translate, TrashIcon, ArchiveIcon to locate and
wire the calls).
---
Outside diff comments:
In @.coderabbit.yaml:
- Around line 11-20: El problema es que la clave path en la sección de
path_instructions contiene dos globs combinados en una sola cadena
('**/*.ts,**/*.tsx'), lo que puede dejar una de las reglas sin aplicarse;
actualiza la configuración para separar los globs en elementos distintos (por
ejemplo como un array de paths o como entradas separadas) y asegurarte de que
cada entrada mantenga su bloque instructions; revisa y corregir las apariciones
de path_instructions.path y la sección que hoy tiene
'**/*.test.ts,**/*.test.tsx' para que cada patrón sea independiente y las
instrucciones asociadas se ejecuten correctamente.
---
Duplicate comments:
In `@src/features/mail/components/header/index.tsx`:
- Line 61: El Checkbox en el componente header (renderizado como <Checkbox />)
carece de nombre accesible; actualiza la instancia de Checkbox para proporcionar
un nombre visible para lectores de pantalla añadiendo un atributo aria-label
(por ejemplo aria-label="Seleccionar todos los mensajes") o envolviéndolo con
una etiqueta <label> asociada, asegurándote de usar la prop correcta del
componente Checkbox si es de una librería (p. ej. ariaLabel, aria-label, label)
y mantener el texto claro y localizado; modifica la línea donde se renderiza
Checkbox para pasar esa prop o agregar el label asociado.
In `@src/features/mail/components/searchInput/index.tsx`:
- Around line 55-77: The clear icon doesn't reset the visible text because the
input isn't controlled and the XIcon has no click handler; update the input JSX
to bind value={query} so it reflects state (searchInput ref and setQuery remain
unchanged), and add an onClick handler to the XIcon (referencing XIcon and
getClearButtonClassName) that calls setQuery('') and then either focuses
searchInput.current.focus() or blurs it per desired UX; also make the XIcon
accessible by adding an appropriate aria-label/role or wrapping it in a button
element so keyboard users can trigger the clear action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a17d4d90-32ac-469f-bf1c-72ffc5964bf4
📒 Files selected for processing (7)
.coderabbit.yamlsrc/features/mail/components/header/index.tsxsrc/features/mail/components/searchInput/index.tsxsrc/features/mail/components/trayEmptyState/index.tsxsrc/i18n/locales/en.jsonsrc/routes/layouts/SidebarAndHeaderLayout.tsxsrc/types/mail/index.ts
Contributor
Author
|
Ready for review @larryrider |
larryrider
approved these changes
Mar 18, 2026
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
2 participants
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 new Tray List component so we can render the list of emails the user has.
Summary by CodeRabbit
Notas de la versión
Nuevas características
Actualizaciones