-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Fix unread notes count in inbox panel #35396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test Results SummaryCommit SHA: ae6d71e
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on it @joshuatf
Changes LGTM, but I'm getting the following error. I've looked into it briefly, but I don't see any changes causing the error 🤔 It might be just my local.
I think that adding a delay makes sense. |
|
@pmcpinto @jarekmorawski I added a 3 second delay to this. After a note comes into view, it will be marked as read after a few seconds so that the user can continue to see the "unread" indicator when it is first scrolled into review. Please let me know if you want to tweak that time at all. @moon0326 This is ready for review when you have time. |
Sounds good to me. Thanks! |
moon0326
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.


All Submissions:
Changes proposed in this Pull Request:
is_readis_readin the data store@pmcpinto @jarekmorawski This PR marks notes as read when viewing them. One issue with this is that a note is instantly marked as read and the unread indicator flashes off the screen almost immediately. It might be worth adding a delay before this occurs or caching the notes read status temporarily to avoid losing the unread indicator immediately.
Also cc @moon0326 to make sure I'm not creating regressions with the existing logic. It seems like a choice was made in woocommerce/woocommerce-admin#6047 and woocommerce/woocommerce-admin#7896 to only mark notes as read when they were actioned. This obviously undoes some of that work, but we are unlikely to see read notes if they are not actioned.
Closes #34929 .
How to test the changes in this Pull Request:
Other information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: