Opened 10 years ago
Last modified 8 years ago
#6179 accepted enhancement
notification count doesn't match on admin bar
Reported by: | danbp | Owned by: | johnjamesjacoby |
---|---|---|---|
Milestone: | Awaiting Contributions | Priority: | normal |
Severity: | normal | Version: | |
Component: | Toolbar & Notifications | Keywords: | needs-unit-tests needs-testing needs-refresh |
Cc: | danco38@…, mikebolton.ca@… |
Description
BP 2.2 RC2 theme 2013
All is in the title and the joined picture.
To reproduce: mention a user several times (more as once preferably :) )
To solve the issue, i modified the $count in bp_notifications_toolbar_menu function (bp-notifications-adminbar.php:20)
Change
$count = ! empty( $notifications ) ? count( $notifications ) : 0;
to
$count = bp_get_total_mention_count_for_user( bp_loggedin_user_id() );
Attachments (1)
Change History (14)
#1
@
10 years ago
- Summary changed from notification cound doesn't match on admin bar to notification count doesn't match on admin bar
#2
@
10 years ago
- Keywords needs-unit-tests needs-testing added
- Milestone changed from Awaiting Review to 2.2
- Owner set to johnjamesjacoby
- Status changed from new to accepted
#4
@
10 years ago
Actually, i'm not sure it's a bug. I think we're fetching the number of notifications grouped by type. So if a user has 2 mentions, it can be correct to have one notification of the mention type. Checking this before tonight's dev-chat.
#5
@
10 years ago
IMHO, there's no bug see this image :
In the WP Admin Bar, we are grouping notifications by type and it seems pretty constistent to me to have a count set to 3 if i've received 5 notifications concerning 3 different types. 3 == number of lines that are appearing on hover.
If we look at the user's item menus, the notifications count is ok.
#6
@
10 years ago
It's effectively not a bug , but IMHO a count that isn't very usefull, or confusing.
"type" is never mentionned actually to public. It's only counted, then dispatched over the menu. So i don't see the utility of a number beside howdy.
This is a under the hood information, and necessary to get it to work, but...
Instead a count of different things, it could be a simple icon "attention - you received something" and let unchanged the details of what, from, where, on the different usermenu items.
Sorry for this UX pov.
#7
@
10 years ago
- Milestone changed from 2.2 to Future Release
- Type changed from defect (bug) to enhancement
This has bothered me for a long time.
#8
@
10 years ago
I think you're both right that the code is doing what we've told it to, but it's not a great experience. I agree that it makes the most sense for the digit in the top-level menu to match the total number of new notifications across all components, but I'm not confident it ever behaved the way we've wanted it to.
Thanks Paul for the bump to Future Release.
#10
@
8 years ago
- Milestone changed from Future Release to 2.8
Facebook etc shows notifications count, not notifications "types" (group) count.
So I would prefer to make the counter to display the total number of notifications, not types.
Any objections?
#12
@
8 years ago
- Cc mikebolton.ca@… added
I have discovered and partially* solved this issue over the past couple of days, and found this ticket after coming up with my solution.
Since then, I tried the proposed change above:
$count = bp_get_total_mention_count_for_user( bp_loggedin_user_id() );
and found that the notification counter would show '0' regardless of how many notifications there were.
In my solution, I changed that line to:
$count = ! empty( $notifications ) ? bp_notifications_get_unread_notification_count( bp_loggedin_user_id() ) : 0;
This works as desired. It shows a count of individual notifications, rather than a count of groups of notifications.
Something for your consideration.
*I say "partially" only because I can't get the modified function to work in bp-custom.php or functions.php; it works flawlessly if I hack the core file bp-notifications-adminbar.php (not an acceptable solution). This is a problem with my level of experience, not the solution.
view the issue