[buddypress-trac] [BuddyPress Trac] #5513: Bulk Notifications Management

[buddypress-trac]

#5513: Bulk Notifications Management
---------------------------+------------------
 Reporter:  colabsadmin    |       Owner:
     Type:  enhancement    |      Status:  new
 Priority:  normal         |   Milestone:  2.2
Component:  Notifications  |     Version:  2.0
 Severity:  normal         |  Resolution:
 Keywords:  needs-patch    |
---------------------------+------------------
Changes (by boonebgorges):

 * keywords:  has-patch => needs-patch


Comment:

 lakrisgubben - Thanks so much for working up this patch! Looking pretty
 good. I have a few suggestions:

 * Move the checkbox column to the left, rather than the right. This is
 more consistent with other bulk action stuff in WP/BP.
 * When marking all items read/unread, the AJAX request removes them from
 the list, leaving an empty list. When refreshing the page, you see a "You
 have no (un)read notifications" message instead of the table. This is a
 bit jarring - I wonder if we can dynamically remove the table and show the
 "You have no..." message when all items are removed. Could you take a look
 to see how hard this is, given the way our templates currently work?
 * Similarly, removing items from the list dynamically messes with
 pagination. (Try appending `?num=2` to the URL to see what I mean.) The
 solution here might be the same as above: namely, the AJAX request should
 return an entire template. Have a look at how this is done in group
 membership requests.
 * If the two previous points are too hard to solve, maybe we can think
 about scrapping AJAX support for now, and just doing this all with a page
 refresh, in which case the problems go away.
 * Related to the previous point: it looks like this won't work with JS
 enabled. With this kind of form, enabling no-JS support is very easy, so
 let's do it. When JS is disabled, let's make sure the Select All checkbox
 doesn't show, since it won't do anything.
 * Small things:
     - Function names should be more specific.
 `bp_notifications_bulk_management` should say something about `dropdown`
 etc (see if there's another function in BP whose function name you can
 copy). `bp_legacy_theme_ajax_notifications_bulk_management()` should be a
 verb rather than a noun; see the other AJAX handlers.
     - Let's use `foreach()` rather than `for()` in
 `bp_legacy_theme_ajax_notifications_bulk_managaement()`.
     - The 'do_action' param in the AJAX request is confusingly named.
 `do_action` has a particular connotation in WP-land :)

 Thanks again for your work on this! BTW, feel free to do your work and
 patching in Git if it's more comfortable - we have a mirror at
 https://buddypress.git.wordpress.org, and we can handle git-formatted
 patches without a problem (or run `git diff --no-prefix`).

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5513#comment:6>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac