Auto Advance Email on Archive #369
Conversation
This reverts commit b45ad8e.
|
Someone is attempting to deploy a commit to the Inbox Zero Team on Vercel. A member of the Team first needs to authorize it. |
|
Shiva Gupta seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
WalkthroughThis pull request introduces a new function Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
apps/web/components/email-list/EmailList.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in 'apps/web/.eslintrc.json': Cannot find module '@typescript-eslint/parser'
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/email-list/EmailList.tsx(3 hunks)apps/web/components/email-list/EmailPanel.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- apps/web/components/email-list/EmailPanel.tsx
🔇 Additional comments (2)
apps/web/components/email-list/EmailList.tsx (2)
229-233: Auto-advance feature is correctly implemented.The implementation for automatically advancing to the next email after archiving works as intended. This matches the PR objective of adding this functionality for the archive action.
🧰 Tools
🪛 Biome (1.9.4)
[error] 229-229: This hook does not specify all of its dependencies: setOpenThreadId
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
255-255: The dependency array is properly updated.The
prevOrNextRowIdhas been correctly added to the dependency array, ensuring the callback uses the latest calculated value when an email is archived.
|
Great that you added a video too! Super helpful. Also see the cla task that's pending |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/email-list/EmailPanel.tsx (1)
14-14: Naming convention inconsistency: prop should use camelCase.The new prop
OpenEmailOnArchiveuses PascalCase naming, which is inconsistent with React/TypeScript conventions and the rest of the codebase where props and function names use camelCase (likeonArchive,close, etc.).Consider renaming to
openEmailOnArchivefor consistency:- OpenEmailOnArchive, + openEmailOnArchive,- OpenEmailOnArchive: () => void; + openEmailOnArchive: () => void;Also applies to: 25-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
apps/web/components/email-list/EmailList.tsx(2 hunks)apps/web/components/email-list/EmailListItem.tsx(0 hunks)apps/web/components/email-list/EmailPanel.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- apps/web/components/email-list/EmailListItem.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/email-list/EmailList.tsx
🔇 Additional comments (1)
apps/web/components/email-list/EmailPanel.tsx (1)
59-62: Implementation looks good.The implementation correctly calls the new function after archiving. This achieves the PR objective of adding functionality to automatically advance to the next email after archiving the current one.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/email-list/EmailList.tsx (1)
273-288: Good implementation of auto-advancing to the next email on archive.The new
setPrevOrNextRowfunction properly handles edge cases:
- When the thread is not found in the list
- When there are no threads available
- When there's only one thread available
In all these cases, it correctly calls
closePanel()to close the email panel.However, I suggest improving the function name to better reflect its purpose:
-function setPrevOrNextRow() { +function advanceToAdjacentThread() {The new name more clearly describes that this function advances to the next thread (or previous if at the end).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/email-list/EmailList.tsx(2 hunks)
🔇 Additional comments (1)
apps/web/components/email-list/EmailList.tsx (1)
511-511: Successfully implemented auto-advance on archive.The
OpenEmailOnArchiveprop now uses thesetPrevOrNextRowfunction to navigate to the next email after archiving, which fulfills the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/email-list/EmailList.tsx(2 hunks)apps/web/components/email-list/EmailPanel.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/email-list/EmailPanel.tsx
🔇 Additional comments (1)
apps/web/components/email-list/EmailList.tsx (1)
511-511: Proper prop addition for the auto-advance feature.The advanceToAdjacentThread function is correctly passed to the EmailPanel component, enabling the "advance to next email after archive" feature.
| @@ -57,7 +58,7 @@ export function EmailPanel({ | |||
| onPlanAiAction={() => onPlanAiAction(row)} | |||
| onArchive={() => { | |||
| onArchive(row); | |||
| close(); | |||
| advanceToAdjacentThread(); | |||
There was a problem hiding this comment.
Moved the logic outside of onArchive() and where close() was before as per comments.
| @@ -270,6 +270,23 @@ export function EmailList({ | |||
| }, 100); | |||
| } | |||
|
|
|||
| function advanceToAdjacentThread() { | |||
There was a problem hiding this comment.
Moved the logic into a function which will only be called onArchive() instead of every render (possibly) as per comments.
| (thread) => thread.id === openThreadId, | ||
| ); | ||
|
|
||
| if (openedRowIndex === -1 || threads.length === 0 || threads.length === 1) |
There was a problem hiding this comment.
Close the email panel when there is only one thread because that thread would have been archived.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Signed the CLA. |
|
Thanks! |
Hi,
I am a first time contributor and I worked on the issue #345 Auto advance mail. The functionality works for archive for now but can work with other actions if needed.
Related to #345.
Demo Video: https://www.loom.com/share/a2b58b50466b49169d3d6fa83e6bbbd2?sid=2e0e800b-71cb-42b0-8601-63134f6cbd3d
Summary by CodeRabbit
New Features
Bug Fixes
Changes
rejectPlanfunction from theEmailListItemcomponent's props, altering its interaction with parent components.