Skip to content
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

Notifications: Mark as unread #11573

Merged
merged 3 commits into from Apr 26, 2019
Merged

Conversation

frosty
Copy link
Contributor

@frosty frosty commented Apr 26, 2019

This PR adds the ability to mark notifications as unread. Swipe left-to-right on a read notification to mark unread.

mark-unread

To test:

  • Build and run and go to your Notifications
  • Swipe left-to-right on a read notification and check it becomes unread
  • Ensure you can still swipe on unread notifications to mark them read
  • Open the same WPCom account in a browser and check the state is reflected there. Note that you may need to reload the page to see your changes on the app take effect.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

return []
}
let isRead = note.read

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think if Notification can have a readonly property named isRead?In that way we can use notice.isRead to read the value without updating the data model.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that could make sense, although I only created this local variable because I thought it was slightly more readable in this case. I'll leave it for this PR, but it's something we could consider if we need it elsewhere :) Thanks!

Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @frosty ! Well done with this! I left one comment but feel free to skip it. The test worked as you described even on the iPad.
:shipit:

@frosty frosty merged commit 61185e6 into develop Apr 26, 2019
@frosty frosty deleted the feature/notifications-mark-unread branch April 26, 2019 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants