@alexhollender_WMF @ovasileva @Edtadros

TLDR: I couldn't find any visual changes that were obviously wrong using Pixel, but there were a couple ambiguous changes I had questions about.

I ran the following Pixel commands to see the visual changes between the disabled and enabled $wgVectorVisualEnhancementNext feature flag:

1. I edited Pixel's LocalSettings.php to disable the flag: $wgVectorVisualEnhancementNext = false;
2. I ran ./pixel.js reference to obtain reference images.
3. I edited Pixel's LocalSettings.php to enable the flag: $wgVectorVisualEnhancementNext = true;
4. I ran ./pixel.js test to obtain test images and compare against the reference images.

The report generated is located at https://joyful-elf-dff4a2.netlify.app/desktop/ . The "reference" column shows images with the feature flag disabled while the "test" column shows images with the feature flag enabled.

I couldn't find any obviously wrong changes, but I did notice the following which might be noteworthy:

• Echo's labels now show when there are zero items whereas before they didn't show:

• There are some minor changes to legacy Vector's tabs associated with this flag, but they are very minor.

In T322673#8381222, @nray wrote:

• Echo's labels now show when there are zero items whereas before they didn't show:

this would be a blocker if it's actually occuring

I don't think this is blocked on anything?

Change 855027 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Hide Echo notification count when zero

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

In T322673#8381222, @nray wrote:

The report generated is located at https://joyful-elf-dff4a2.netlify.app/desktop/ . The "reference" column shows images with the feature flag disabled while the "test" column shows images with the feature flag enabled.

I believe quite a few of the pixel errors in these results are regressions.

• Placement of ToC dropdown menus are off. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853396

• The spacing between the main menu button and logo has been reduced by 6px on viewports larger than tablet. This also affects the alignment of the searchbar. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855090

• T323174 The spacing between the collapsed ToC button and page title has been reduced by 6px on viewports larger than tablet. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101
  
  
  

• T323177 The language button is shifted a few pixels due to the addition of align-items: center to the page titlebar in this patch.
  
  
  

• The spacing of the search bar in the sticky header is also misaligned. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853400

• T323172 There is extra spacing around the icon buttons on viewports below tablet.

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/857026

• T323176 Sticky header icon buttons are missing 8px padding

Change 855041 had a related patch set uploaded (by Nray; author: Nray):

[operations/mediawiki-config@master] Enable VectorVisualEnhancementsNext flag on Beta

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

Change 855041 abandoned by Nray:

[operations/mediawiki-config@master] Enable VectorVisualEnhancementsNext flag on Beta

Reason:

Will be covered by 854129

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

@Edtadros FYI, the VectorVisualEnhancementsNext feature has now been enabled on beta so QA testing can begin on that

Change 855027 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Hide Echo notification count when zero

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

Change 855156 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Echo in Legacy Vector should not be impacted by feature flag

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

Change 855158 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Dropdown arrow pixel should only apply to modern Vector

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

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

[mediawiki/skins/Vector@master] [Technical] Fix sticky header search input alignment when VisualEnhancementFlag is enabled

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

Change 853400 merged by jenkins-bot:

[mediawiki/skins/Vector@master] [Technical] Fix sticky header search input alignment when VisualEnhancementFlag is enabled

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

Change 855156 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Echo in Legacy Vector should not be impacted by feature flag

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

@bwang can you fix the visibility of your screenshots?

I've manually reviewed the Pixel failures and there are 2 issues with legacy Vector that must be fixed before this change rolls out those are:

• https://gerrit.wikimedia.org/r/855158
• https://gerrit.wikimedia.org/r/855156

I've merged the following as these seemed like good technical improvements to our codebase and did address the issues flagged above:

• The spacing of the search bar in the sticky header is also misaligned. Fixed by https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853400

Looks like a good change but doesn't quite work for me:

• The spacing between the main menu button and logo has been reduced by 6px on viewports larger than tablet. This also affects the alignment of the searchbar. Fixed by https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855090

After the change, there still seems a slight misalignment:
{F35736539}

The fix in https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853396 and https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101, while small, seems to complicate the codebase a so I'm curious if these issues are important enough to fix from a design perspective. Suggest reaching out to Alex about it and perhaps delaying to after the deploy if possible:

• The language button is shifted a few pixels due to the addition of align-items: center to the page titlebar in this patch.
• The spacing between the collapsed ToC button and page title has been reduced by 6px on viewports larger than tablet.
• Placement of ToC dropdown menus are off.

In T322673#8383888, @bwang wrote:

In T322673#8381222, @nray wrote:

The report generated is located at https://joyful-elf-dff4a2.netlify.app/desktop/ . The "reference" column shows images with the feature flag disabled while the "test" column shows images with the feature flag enabled.

I believe quite a few of the pixel errors in these results are regressions.

1. Placement of ToC dropdown menus are off. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853396

2. The spacing between the main menu button and logo has been reduced by 6px on viewports larger than tablet. This also affects the alignment of the searchbar. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855090

3. The spacing between the collapsed ToC button and page title has been reduced by 6px on viewports larger than tablet. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101
   
   
   

4. The language button is shifted a few pixels due to the addition of align-items: center to the page titlebar in this patch. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101
   
   
   

5. Fixed by 853400 The spacing of the search bar in the sticky header is also misaligned.

thanks for documenting these @bwang. none of them seem like blockers to me. if it's helpful to prioritize them I would say these two are more important than the others:

2. The spacing between the main menu button and logo has been reduced by 6px on viewports larger than tablet. This also affects the alignment of the searchbar.
3. Placement of ToC dropdown menus are off. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/853396

Change 855158 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Dropdown arrow pixel should only apply to modern Vector

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

The language button is shifted a few pixels due to the addition of align-items: center to the page titlebar in this patch. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101

@alexhollender_WMF Question relating to this issue, how much do you care about the alignment of the ToC and language button in the titlebar when the title is very long?

Vertically centered:

Vertically centered is simpler from a CSS point of view because we dont need to manually space each button depending on it's padding.

Change 856676 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Drop unnecessary watchstar icon classes on toggle

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

Bug raised by @dom_walden on T317710#8389485 - watchstar icon doesn't update on certain pages. Fixed in above patch.

Change 856676 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Drop unnecessary watchstar icon classes on toggle

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

In T322673#8390153, @bwang wrote:

The language button is shifted a few pixels due to the addition of align-items: center to the page titlebar in this patch. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/855101

@alexhollender_WMF Question relating to this issue, how much do you care about the alignment of the ToC and language button in the titlebar when the title is very long?

Vertically centered:

Not centered:

Vertically centered is simpler from a CSS point of view because we dont need to manually space each button depending on it's padding.

I prefer the top alignment

In T322673#8381340, @alexhollender_WMF wrote:

In T322673#8381222, @nray wrote:

• Echo's labels now show when there are zero items whereas before they didn't show:

this would be a blocker if it's actually occuring

Catching up so apologies if I've missed something. @Jdlrobson, @bwang, @alexhollender_WMF - did we address this one yet?

In T322673#8396476, @ovasileva wrote:

In T322673#8381340, @alexhollender_WMF wrote:

In T322673#8381222, @nray wrote:

• Echo's labels now show when there are zero items whereas before they didn't show:

this would be a blocker if it's actually occuring

Catching up so apologies if I've missed something. @Jdlrobson, @bwang, @alexhollender_WMF - did we address this one yet?

Yes this is fixed.

I've updated my comment above listing the visual regressions with the feature flag. Two of the issues have been fixed since last week, but 2 have been added as well.

In T322673#8396935, @bwang wrote:

I've updated my comment above listing the visual regressions with the feature flag. Two of the issues have been fixed since last week, but 2 have been added as well.

@bwang thanks for the update. where are we tracking the fixes for these? do they need tasks? if tasks already exist can you add the tasks next to the description of the regressions?

@bwang thanks for the update. where are we tracking the fixes for these? do they need tasks? if tasks already exist can you add the tasks next to the description of the regressions?

These are tracked in T323174 and T323172.

In T322673#8383888, @bwang wrote:

• Sticky header icon buttons are missing 8px padding

tracked in T323176

In T322673#8400971, @alexhollender_WMF wrote:

In T322673#8396935, @bwang wrote:

I've updated my comment above listing the visual regressions with the feature flag. Two of the issues have been fixed since last week, but 2 have been added as well.

@bwang thanks for the update. where are we tracking the fixes for these? do they need tasks? if tasks already exist can you add the tasks next to the description of the regressions?

Yep, I made 4 tasks that are subtasks of this: T323176, T323177, T323172, T323174. I've updated my comment to include these tasks too