Page MenuHomePhabricator

[Notifications] Add Mark as Read functionality
Closed, ResolvedPublic

Assigned To
Authored By
Dmantena
Aug 11 2021, 8:34 PM
Referenced Files
F34940102: Screen Shot 2022-02-01 at 5.20.33 PM.png
Feb 1 2022, 11:29 PM
F34635943: image.png
Sep 7 2021, 6:11 PM
F34635941: image.png
Sep 7 2021, 6:11 PM
F34625649: image.png
Aug 30 2021, 4:49 PM
F34625645: image.png
Aug 30 2021, 4:49 PM
F34625647: image.png
Aug 30 2021, 4:49 PM
F34592353: NotificationsCenterMarkAsRead.png
Aug 13 2021, 12:43 AM

Description

  • Add "Edit" mode to Notifications Center table when tapping Edit button
  • Update displayed notification cell to change display state based on Edit status
  • Add toolbar actions to select/deselect all and mark as read/unread

Product/Design Needs:
[1] Is the expectation that selecting all selects all your notifications, even potential ones that haven't yet been displayed to the user if we have to display these results in pages?
[2] What's the expected behavior when the user is in Edit mode, then scrolls to the bottom of the list to potentially page in more older notifications? What state are those newly loaded old notifications presented in?

Engineering Needs:
[1] The UI needs API on the Notifications Data Controller to interact with here


Proposed designs

Figma: https://www.figma.com/file/cedgOU5CyOR0UVqtjDOvzE/iOS-Notifications?node-id=962%3A4570

Edit viewTap on 'Mark'Tap on 'Mark as read'
image.png (812×375 px, 55 KB)
image.png (812×375 px, 60 KB)
image.png (812×375 px, 72 KB)
Are you sure that you want to mark all N of your notifications as read? Your notifications will be marked as read on all of your devices
Dependencies

https://phabricator.wikimedia.org/T287302

Event Timeline

Dmantena triaged this task as Medium priority.Aug 11 2021, 11:44 PM
Dmantena updated the task description. (Show Details)

@cmadeo @JMinor Per sync, here’s my attempt to better articulate my design questions. Please call out any assumptions I made that are incorrect.

NotificationsCenterMarkAsRead.png (3×11 px, 2 MB)

Edit: Ha, dunno what's up with the thumbnail not rendering correctly, but here's a direct link: https://phab.wmfusercontent.org/file/data/kz2q2xgnfahc6ctrwpri/PHID-FILE-jpndonh5nrwg6ta54agl/NotificationsCenterMarkAsRead.png

Thanks @Dmantena! This is super helpful and I'll share some notes and thoughts below:

  • Totally understand how/why we'll need paging (technically) for the table. My preference would be for this to not be visible to the user outside of loading states. I've put together a ticket for defining loading behavior in the table for paginated results https://phabricator.wikimedia.org/T288835
  • Apologies if this is over simplifying, but it sounds like a lot of our complications re 'Heavy' users could be resolved by removing the 'select all' option in the editing view. With this in mind I think we have the following options:
    • Remove 'Select all' and replace with individual selection
    • Remove 'Select all' and replace with individual selection and a more 'nuclear' option of 'Marking all as read' which is supported by the API. In this scenario, it's still theoretically possible that someone could 'Mark as read' a notification they haven't yet seen in their inbox because it was inbound while they were taking the action. Personally this feels like enough of an edge case that I would rather not prevent folks for whom getting to 'inbox zero' is important from getting a tool for quick selection over this scenario, which let's hope is a pretty rare case and is possible to recover from by going to your 'read' messages.
    • Do the same thing that Apple Mail does (you can check this out by going to Mail > All Mail and then opening the edit menu and tapping on the option to 'select all') Apple mail selects all of the loaded messages, but if you scroll down / take action you'll see that un-loaded messages are unselected. This means that to clear out a big inbox you'd need to: tap select all > tap mark as read > wait for messages to load > rinse and repeat until the inbox is empty. It's not the worst thing, but certainly not the simplest solution for someone who has a lot of messages.

I don't actually have a strong preference between the later two options, so I'm happy to defer to @JMinor or eng!

We are going to move forward with #2 (Mark all as read)
This means that we will have three options available: Mark as read, mark as unread and mark all as read -- we'll want to confirm before actually marking all as read.

Data side of this work is in https://github.com/wikimedia/wikipedia-ios/pull/4069.

Prototype UI work is in notifications-usertesting branch, but it likely needs to be cleaned up before putting into a PR.

Tsevener added a subscriber: Tsevener.

This is ready for design review. There are a couple of workarounds we had to do that you should know about:

  1. For iOS13, we weren't able to display a contextual menu from a toolbar button. So we went with an action sheet instead only for iOS13. We are displaying the contextual menu for iOS14 & 15. This difference comes into play after selecting Edit, choosing cells, then tapping "Mark".

iOS13 screenshot

Screen Shot 2022-02-01 at 5.20.33 PM.png (1×563 px, 287 KB)

  1. To get the count when displaying the text "Are you sure you want to mark all X messages of your notifications as read?" - I was unable to pull a server-side count without some difficulty keeping it in sync with all the local actions that the user was taking, so instead we're just calculating a local count of notifications. BUT we are still calling the nuclear "Mark all as read" API call to the server which marks everything as read. We refresh pretty often so it's likely our local count is close, but it might not be exactly what the server will mark as read. Just letting you know. If accuracy of this count is important, we might want to remove the count from the text.
JMinor claimed this task.