Skip to content

Commit

Permalink
Move talk page notification data from 'user-menu' to 'notifications'
Browse files Browse the repository at this point in the history
**Note**: This change will affect the order of the yellow talk page
message notification on legacy Vector/other skins by moving it from
after the `#pt-notifications-notice` element to before the
`#pt-notifications-alert` element. This was done because the
notification is related to the list of messages that appear when the
bell icon is clicked so having it in close proximity to that icon is
hopefully more intuitive than having it next to the unrelated inbox
icon. [1]

Per T274428, we need this notification to be inside the `notifications`
array instead of inside the `user-menu` array.

Additionally:

* Per T274428, update the notifications message copy to "You have a new
Talk page message"

* Remove the `onPersonalUrls` hook method inside EchoHooks,
unregister its use as a hook in extension.json, and update its
references in Echo.

[1] T274428#7113896

Bug: T274428
Change-Id: I5ae0ec089bcf0eec1ec7ac13f60e811f54e1d8e1
  • Loading branch information
nicholasray committed May 26, 2021
1 parent 12e3d5a commit 3a351cf
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 26 deletions.
1 change: 0 additions & 1 deletion extension.json
Expand Up @@ -458,7 +458,6 @@
"SkinMinervaReplaceNotificationsBadge": "EchoHooks::onSkinMinervaReplaceNotificationsBadge",
"LoadExtensionSchemaUpdates": "EchoHooks::onLoadExtensionSchemaUpdates",
"GetPreferences": "EchoHooks::getPreferences",
"PersonalUrls": "EchoHooks::onPersonalUrls",
"BeforePageDisplay": "EchoHooks::beforePageDisplay",
"ResourceLoaderRegisterModules": "EchoHooks::onResourceLoaderRegisterModules",
"UserGroupsChanged": "EchoHooks::onUserGroupsChanged",
Expand Down
2 changes: 1 addition & 1 deletion i18n/en.json
Expand Up @@ -64,7 +64,7 @@
"echo-pref-dont-email-read-notifications": "Don't include read notifications in summary emails",
"echo-learn-more": "Learn more",
"echo-log": "Public log",
"echo-new-messages": "You have new messages",
"echo-new-messages": "You have a new Talk page message",
"echo-category-title-edit-user-talk": "Talk page {{PLURAL:$1|message|messages}}",
"echo-category-title-article-linked": "Page {{PLURAL:$1|link|links}}",
"echo-category-title-reverted": "Edit {{PLURAL:$1|revert|reverts}}",
Expand Down
39 changes: 16 additions & 23 deletions includes/EchoHooks.php
Expand Up @@ -1013,29 +1013,6 @@ public static function onSkinMinervaReplaceNotificationsBadge( $user, $title, &$
$badge = $parser->processTemplate( 'NotificationBadge', $data );
}

/**
* Handler for PersonalUrls hook.
* Marks the talk page link when the user has a new message.
* @see https://www.mediawiki.org/wiki/Manual:Hooks/PersonalUrls
* @param array &$personal_urls Array of URLs to append to.
* @param Title &$title Title of page being visited.
* @param SkinTemplate $sk
*/
public static function onPersonalUrls( &$personal_urls, &$title, $sk ) {
$user = $sk->getUser();
if ( !$user->isRegistered() ) {
return;
}

// If the user has new messages, display a talk page alert
if ( self::shouldDisplayTalkAlert( $user, $title )
&& Hooks::run( 'BeforeDisplayOrangeAlert', [ $user, $title ] )
) {
$personal_urls['mytalk']['text'] = $sk->msg( 'echo-new-messages' )->text();
$personal_urls['mytalk']['class'] = [ 'mw-echo-alert' ];
}
}

/**
* Determine if a talk page alert should be displayed.
* We need to check:
Expand Down Expand Up @@ -1148,6 +1125,22 @@ public static function onSkinTemplateNavigationUniversal( $skinTemplate, &$links
$alertLinkClasses[] = 'mw-echo-notifications-badge-long-label';
}

if (
self::shouldDisplayTalkAlert( $user, $title ) &&
MediaWikiServices::getInstance()
->getHookContainer()->run( 'BeforeDisplayOrangeAlert', [ $user, $title ] )
) {
// Move `mytalk` from `user-menu` to `notifications`.
$links['notifications']['mytalk'] = array_merge(
$links['user-menu']['mytalk'],
[
'text' => $skinTemplate->msg( 'echo-new-messages' )->text(),
'class' => [ 'mw-echo-alert' ]
]
);
unset( $links['user-menu']['mytalk'] );
}

$links['notifications']['notifications-alert'] = [
'href' => $url,
'text' => $alertText,
Expand Down
2 changes: 1 addition & 1 deletion modules/ext.echo.init.js
Expand Up @@ -265,7 +265,7 @@ function initDesktop() {
if ( hasUnseenAlerts || hasUnseenMessages ) {
// Clicked on the flyout due to having unread notifications
// This is part of tracking how likely users are to click a badge with unseen notifications.
// The other part is the 'echo.unseen' counter, see EchoHooks::onPersonalUrls().
// The other part is the 'echo.unseen' counter, see EchoHooks::onSkinTemplateNavigationUniversal().
mw.track( 'counter.MediaWiki.echo.unseen.click' );
}
}, function () {
Expand Down

0 comments on commit 3a351cf

Please sign in to comment.