I believe this is the same bug as https://phabricator.wikimedia.org/T191021 @ovasileva @alexhollender let's talk aobut this in product planning.

The user menu (5), hamburger icon (1) and ellipsis (2) are created using mw-ui-icon and those have the best chance of being consistent with the style guide (and each other).

As for the other icons, they come from Echo extension (3,4) with their own custom styles. Once we are happy with the styling of 1,2 and 5, we should update their implementation.

@alexhollender out of interest, do any of the icons ad currently rendered reflect the final state we desire? Or are they all flawed in some way?

In T285259#7173625, @Jdlrobson wrote:

@alexhollender out of interest, do any of the icons ad currently rendered reflect the final state we desire? Or are they all flawed in some way?

currently, in latest vector, none of them are correct : /
(but in Minerva they are all correct!)

I suggest we use this ticket to focus on getting Echo's icons aligned with Vectors (in whatever form makes sense). I would suggest it makes sense for Echo to register menu items with icon class and for the classes to be added on the Vector side just as we have done with the user menu.

One they are standardized (but incorrect) we can look to do T191021. Will leave notes separately on that ticket.

diff --git a/includes/Hooks.php b/includes/Hooks.php
index 68306a36..7cd6855d 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -162,21 +162,30 @@ class Hooks {
 				unset( $content_navigation['user-menu']['login'] );
 			}
 			// Prefix user link items with associated icon.
-			$user_menu = $content_navigation['user-menu'];
+			$content_navigation['user-menu'] = self::getMenuWithIcons( $content_navigation['user-menu'] );
+			$content_navigation['notifications'] = self::getMenuWithIcons( $content_navigation['notifications'], 'mw-ui-icon' );
+		} else {
+			// Remove user page from personal toolbar since it will be inside the personal menu for logged in
+			// users when the feature flag is disabled.
+			unset( $content_navigation['user-page'] );
+		}
+	}
+
+	private static function getMenuWithIcons( $menu, $iconType = 'mw-ui-icon-before' ) {
 			// Loop through each menu to check/append its link classes.
-			foreach ( $user_menu as $menu_key => $menu_value ) {
+			foreach ( $menu as $menu_key => $menu_value ) {
 				// Check if the menu has an icon key (provided by extensions). If not, get the icon from the icon map.
 				$icon_name = array_key_exists( 'icon', $menu_value )
 					? $menu_value['icon']
 					: self::getIconFromKey( $menu_key );
 				// Set the default menu icon classes.
-				$menu_icon_classes = [ 'mw-ui-icon', 'mw-ui-icon-before', 'mw-ui-icon-wikimedia-' . $icon_name ];
+				$menu_icon_classes = [ 'mw-ui-icon', $iconType, 'mw-ui-icon-wikimedia-' . $icon_name ];
 
 				// Add the link-class key to the menu and its relevant classes if it doesn't exist.
 				if ( !( array_key_exists( 'class', $menu_value )
 					|| array_key_exists( 'link-class', $menu_value ) )
 				) {
-					$content_navigation['user-menu'][$menu_key]['link-class'] = $menu_icon_classes;
+					$menu[$menu_key]['link-class'] = $menu_icon_classes;
 				} else {
 					// The link-class or class keys do exist, so append the icon classes to them.
 					foreach ( $menu_value as $link_key => $link_value ) {
@@ -185,7 +194,7 @@ class Hooks {
 							case 'class':
 							case 'link-class':
 								$prior_link_classes = is_array( $link_value ) ? $link_value : [ $link_value ];
-								$content_navigation['user-menu'][$menu_key][$link_key] = array_merge(
+								$menu[$menu_key][$link_key] = array_merge(
 									$prior_link_classes,
 									$menu_icon_classes
 								);
@@ -194,13 +203,8 @@ class Hooks {
 					}
 				}
 			}
-		} else {
-			// Remove user page from personal toolbar since it will be inside the personal menu for logged in
-			// users when the feature flag is disabled.
-			unset( $content_navigation['user-page'] );
-		}
+			return $menu;
 	}
-
 	/**
 	 * Upgrades Vector's watch action to a watchstar.
 	 *

hi @Jdlrobson is T191021 a blocker for this ticket?

Nope. You can do this before or after. You'll likely need to test this on the current version as well as a version with https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/702456 and the core depends-on patch checked out.

@Jdlrobson

In T285259#7173683, @Jdlrobson wrote:

I suggest we use this ticket to focus on getting Echo's icons aligned with Vectors (in whatever form makes sense). I would suggest it makes sense for Echo to register menu items with icon class and for the classes to be added on the Vector side just as we have done with the user menu.

I was trying this approach, and I think it might not work because the "notifications" menu data is still empty inside Hooks::updateUserLinksItems

Nevermind, Jon helped figure out I had an issue with my LocalSettings

Change 704862 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/extensions/Echo@master] Add icon property to echo link data for modern Vector

https://gerrit.wikimedia.org/r/704862

Change 704870 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Update echo badges to use mw-ui-icon implementation in modern Vector

https://gerrit.wikimedia.org/r/704870

Change 704862 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Add icon property to echo link data for modern Vector

https://gerrit.wikimedia.org/r/704862

Change 705411 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/extensions/Echo@master] Update vector badge styles

https://gerrit.wikimedia.org/r/705411

Change 705411 had a related patch set uploaded (by Jdlrobson; author: Bernard Wang):

[mediawiki/extensions/Echo@master] Update vector badge styles

https://gerrit.wikimedia.org/r/705411

Change 705411 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Update vector badge styles

https://gerrit.wikimedia.org/r/705411

Please design review on the beta cluster. I suggest testing the following:

• https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?useskinversion=1
• https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?useskinversion=2&vectoruserlinks=0
• https://en.wikipedia.beta.wmflabs.org/wiki/Main_Page?useskinversion=2&vectoruserlinks=1

When done move to ready for sign off. We'll do a full QA of user links feature later.

@Jdlrobson @bwang

• we're missing padding between the notification icon and the username, but maybe that's happening in T285786?

• the hover color is too light (should match Minerva, see comment here T191021#7231088)

not sure if I should put this in Needs more work or if y'all want to handle those fixes elsewhere

the hover color is too light (should match Minerva, see comment here T191021#7231088)

It does match Minerva? Minerva uses background-color: rgba(0,0,0,0.03); which this is also using. Perhaps we need to change that default? :) What is the correct background color?

In T285259#7231274, @Jdlrobson wrote:

the hover color is too light (should match Minerva, see comment here T191021#7231088)

It does match Minerva? Minerva uses background-color: rgba(0,0,0,0.03); which this is also using. Perhaps we need to change that default? :) What is the correct background color?

it's slightly lighter than Minerva (which I assume means it will be slightly lighter than the hamburger and user menu icon once those are updated?):

I think it's because those two icons already have this opacity: 0.51 rule, which I believe is effective whenever they don't have new notifications inside of them?

Ah yes.. the opacity would definitely explain that. Thanks for expanding. @bwang perhaps we could set the opacity explicitly in the skin style for modern Vector or restrict this style in some way?

@Jdlrobson Yeah I could put up a follow up patch for Echo. I was thinking it might be easier to disable the opacity entirely for user links, and just set the color explicitly, what do you think?

@alexhollender Do you want to preserve the existing behavior for Echo, where the icon is lighter when everything is read, or do you want the echo icons to behave similarly to the user menu icon? And if the latter, do you know exactly what color it should be? I believe right now it's #202122

In T285259#7231489, @bwang wrote:

@alexhollender Do you want to preserve the existing behavior for Echo, where the icon is lighter when everything is read, or do you want the echo icons to behave similarly to the user menu icon? And if the latter, do you know exactly what color it should be? I believe right now it's #202122

it would be fine to simplify things here and remove that extra layer of opacity, since they get little numbers on them anyways when you have notifications. I need to figure out what the right color is for all of these icons, for now just go with whatever we're doing for the user menu icon. I will file a separate task for color consistency at some point (since, for example, the hamburger icon doesn't match the user menu icon, and even the little down arrow next to the user icon doesn't match lol)

Change 707389 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/extensions/Echo@master] Override opacity styles for all read badges for vector user links

https://gerrit.wikimedia.org/r/707389

@alexhollender Ok, if that's the case I'll just remove the opacity, which will look like this: https://jmp.sh/hpK2Jte
I made a mistake earlier, both echo icons and the user menu icons are actually just black #000, so I'll just leave them as be until that other color task

Change 707389 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Override opacity styles for all read badges for vector user links

https://gerrit.wikimedia.org/r/707389

Can skip QA per T286732. All patches have been merged.