Conversation
WalkthroughThe changes update the logic for toggling the "important" status in mail threads. They ensure UI state synchronizes with server responses, update memoization dependencies for importance, and switch the important icon to a new variant. No changes to exported entity signatures were made. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ThreadDisplay
participant Server
User->>ThreadDisplay: Clicks "Toggle Important"
ThreadDisplay->>Server: Sends toggle important request
Server-->>ThreadDisplay: Returns updated thread data
ThreadDisplay->>ThreadDisplay: Updates isImportant state from server data
ThreadDisplay->>User: Updates icon and shows toast
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Pheewww
force-pushed
the
fix/mark-as-important-improvements
branch
from
82a4927 to
1909962
Compare
MrgSub
left a comment
MrgSub
left a comment
There was a problem hiding this comment.
Can we use optimistic actions for this?
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| setIsImportant(isNowImportant); | ||
| if (isImportant) { | ||
| toast.success(t('common.mail.markedAsImportant')); | ||
| } else { |
There was a problem hiding this comment.
The toast message is using the stale isImportant value rather than the newly determined isNowImportant. Since React state updates are asynchronous, the condition should use the new value to display the correct toast message:
// Change this:
if (isImportant) {
toast.success(t('common.mail.markedAsImportant'));
} else {
toast.success(t('common.mail.removedAsImportant'));
}
// To this:
if (isNowImportant) {
toast.success(t('common.mail.markedAsImportant'));
} else {
toast.success(t('common.mail.removedAsImportant'));
}This will ensure the toast message accurately reflects the action that was just performed.
| setIsImportant(isNowImportant); | |
| if (isImportant) { | |
| toast.success(t('common.mail.markedAsImportant')); | |
| } else { | |
| setIsImportant(isNowImportant); | |
| if (isNowImportant) { | |
| toast.success(t('common.mail.markedAsImportant')); | |
| } else { |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
|
This PR has merge conflicts and has been open for more than 3 days. It will be automatically closed. Please resolve the conflicts and reopen the PR if you'd like to continue working on it. |
2 participants
Fixes: #1496
Toast gives failure message for successful mark as important.
https://github.com/user-attachments/assets/98b6f76b-4f88-4efa-b7d9-edb323f14cb6
Toggle Important doesnt change icon ui.
https://github.com/user-attachments/assets/63e32042-f89b-4a41-98c5-ef6de3da101a
Mark as Important/Remove as Important doesnt change on toggling ( the request is successful)
https://github.com/user-attachments/assets/309bcbb4-bf92-4996-bc12-55d61bb8162b
Summary by CodeRabbit