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.

Sign up for GitHub

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

Stop showing zero notification bubble outside yoast pages #21784

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Stop showing zero notification bubble outside yoast pages #21784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions css/src/adminbar.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 10 additions & 7 deletions inc/class-wpseo-admin-bar-menu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ protected function add_root_menu( WP_Admin_Bar $wp_admin_bar ) {
$settings_url = '';
$counter = '';
$notification_popup = '';
$notification_count = 0;

$post = $this->get_singular_post();
if ( $post ) {
Expand All @@ -377,7 +378,10 @@ protected function add_root_menu( WP_Admin_Bar $wp_admin_bar ) {
}

if ( empty( $score ) && ! is_network_admin() && $can_manage_options ) {
$counter = $this->get_notification_counter();
$notification_center = Yoast_Notification_Center::get();
$notification_count = $notification_center->get_notification_count();

$counter = $this->get_notification_counter( $notification_count );
$notification_popup = $this->get_notification_popup();
}

Expand All @@ -389,7 +393,7 @@ protected function add_root_menu( WP_Admin_Bar $wp_admin_bar ) {
];
$wp_admin_bar->add_menu( $admin_bar_menu_args );

if ( ! empty( $counter ) ) {
if ( $notification_count > 0 ) {
$admin_bar_menu_args = [
'parent' => self::MENU_IDENTIFIER,
'id' => 'wpseo-notifications',
Expand Down Expand Up @@ -849,18 +853,17 @@ protected function get_settings_page_url() {
/**
* Gets the notification counter if in a valid context.
*
* @param int $notification_count Number of notifications.
*
* @return string Notification counter markup, or empty string if not available.
*/
protected function get_notification_counter() {
$notification_center = Yoast_Notification_Center::get();
$notification_count = $notification_center->get_notification_count();

protected function get_notification_counter( $notification_count ) {
/* translators: Hidden accessibility text; %s: number of notifications. */
$counter_screen_reader_text = sprintf( _n( '%s notification', '%s notifications', $notification_count, 'wordpress-seo' ), number_format_i18n( $notification_count ) );

return sprintf(
' <div class="wp-core-ui wp-ui-notification yoast-issue-counter%s"><span class="yoast-issues-count" aria-hidden="true">%d</span><span class="screen-reader-text">%s</span></div>',
( $notification_count ) ? '' : ' yst-hidden',
( $notification_count ) ? '' : ' wpseo-no-adminbar-notifications',
$notification_count,
$counter_screen_reader_text
);
Expand Down
2 changes: 1 addition & 1 deletion packages/js/src/shared-admin/helpers/notifications-count.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const updateNotificationsCount = ( total ) => {

const adminBarItems = document.querySelectorAll( "#wp-admin-bar-wpseo-menu .yoast-issue-counter" );
for ( const adminBar of adminBarItems ) {
adminBar.classList.toggle( "yst-hidden", total === 0 );
adminBar.classList.toggle( "wpseo-no-adminbar-notifications", total === 0 );
updateTextContentIfElementExists( adminBar, ".yoast-issues-count", String( total ) );
updateTextContentIfElementExists( adminBar, ".screen-reader-text", screenReaderText );
}
Expand Down
6 changes: 3 additions & 3 deletions packages/js/tests/shared-admin/helpers/__snapshots__/notifications-count.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`updateNotificationsCount updates the total in the admin bar to 0 [yst-hidden class triggering hidden css] 1`] = `
exports[`updateNotificationsCount updates the total in the admin bar to 0 [wpseo-no-adminbar-notifications class triggering hidden css] 1`] = `
<div>
<div
class="no-js"
Expand Down Expand Up @@ -29,7 +29,7 @@ exports[`updateNotificationsCount updates the total in the admin bar to 0 [yst-h
role="menuitem"
>
<div
class="wp-core-ui wp-ui-notification yoast-issue-counter yst-hidden"
class="wp-core-ui wp-ui-notification yoast-issue-counter wpseo-no-adminbar-notifications"
>
<span
aria-hidden="true"
Expand Down Expand Up @@ -63,7 +63,7 @@ exports[`updateNotificationsCount updates the total in the admin bar to 0 [yst-h
>
Notifications
<div
class="wp-core-ui wp-ui-notification yoast-issue-counter yst-hidden"
class="wp-core-ui wp-ui-notification yoast-issue-counter wpseo-no-adminbar-notifications"
>
<span
aria-hidden="true"
Expand Down
4 changes: 2 additions & 2 deletions packages/js/tests/shared-admin/helpers/notifications-count.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const FakeAdminMenu = ( { total = 2 } ) => (
* @returns {JSX.Element} The admin bar count.
*/
const AdminBarCountHtml = ( { total } ) => (
<div className={ `wp-core-ui wp-ui-notification yoast-issue-counter${ total === 0 ? " yst-hidden" : "" }` }>
<div className={ `wp-core-ui wp-ui-notification yoast-issue-counter${ total === 0 ? " wpseo-no-adminbar-notifications" : "" }` }>
<span className="yoast-issues-count" aria-hidden="true">${ total }</span>
<span className="screen-reader-text">${ total } notifications</span>
</div>
Expand Down Expand Up @@ -113,7 +113,7 @@ describe( "updateNotificationsCount", () => {
test.each( [
[ 3, "screen-reader plural in English" ],
[ 1, "screen-reader singular in English" ],
[ 0, "yst-hidden class triggering hidden css" ],
[ 0, "wpseo-no-adminbar-notifications class triggering hidden css" ],
] )( "updates the total in the admin bar to %i [%s]", ( total ) => {
const { container } = render( <FakeAdminBar /> );
updateNotificationsCount( total );
Expand Down