Page MenuHomePhabricator

Proposal: merge styles for user login and signup into a single ResourceLoader module
Open, Needs TriagePublic

Assigned To
None
Authored By
DannyS712
Jul 17 2021, 7:25 AM
Referenced Files
F34677050: image.png
Oct 7 2021, 8:30 AM
F34677048: image.png
Oct 7 2021, 8:30 AM
F34677063: image.png
Oct 7 2021, 8:30 AM
F34677061: image.png
Oct 7 2021, 8:30 AM
F34677059: image.png
Oct 7 2021, 8:30 AM
F34677052: image.png
Oct 7 2021, 8:30 AM
F34677057: image.png
Oct 7 2021, 8:30 AM
F34677054: image.png
Oct 7 2021, 8:30 AM

Description

Currently, there are 3 relevant styles-only ResourceLoader modules registered:

  • mediawiki.special.userlogin.common.styles
  • mediawiki.special.userlogin.signup.styles
  • mediawiki.special.userlogin.login.styles

LoginSignupSpecialPage, which is extended by SpecialCreateAccount and SpecialUserLogin, will add the common styles module, and then one of the others depending on if the page is a signup page or not (i.e. will add the signup styles for Special:CreateAccount and the login styles for Special:UserLogin).

However, instead of using separate modules for these styles, we can just add an overall selector for the individual page (.mw-special-Userlogin or .mw-special-CreateAccount). We can then safely load the styles for both on both pages, because they will only affect one or the other.

Disadvantages:

  • Larger CSS package to load on both

Advantage:

  • Remove the need to register the 2 dedicated modules, reducing the size of the module manifest on all pages

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 added a project: Performance-Team.

If this is approved by the performance team I'm happy to send a patch to implement it myself

Related work recently:

  • Rename mediawiki.special.userlogin.signup.js to mediawiki.special.createaccount (change 598217).
  • mediawiki.special.userlogin.signup: Remove unused debounce dependency (change 535761) , ref T213426.
  • Merge htmlform.checker.js into special.userlogin.signup.js module (change 597903), ref T253362.

Related work that is unfinished, that may serve as a first step to untangle some of this:

Before continuing, it may help to first figure out who would own/test/verify this.

Change 723634 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/core@master] Merge mediawiki.special.userlogin.(common|login|signup).styles

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

Tagging Growth-Team for opinion on the visual change that this is likely to make to the Create Account page on mobile.

I simulated the change as follows:

CurrentlyChanged
Screenshot 2021-10-06 at 22.15.53.png (1×2 px, 147 KB)
Screenshot 2021-10-06 at 22.16.28.png (1×2 px, 148 KB)
Screenshot 2021-10-06 at 22.15.55.png (1×2 px, 151 KB)
Screenshot 2021-10-06 at 22.16.29.png (1×2 px, 155 KB)

I suspect that the absence of this is historically not intentional but by accident given that the "login" equivalent of these are already loaded on mobile, and the way some (but not all) of the fields are currently unlimited in width seems an unlikely design choice.

mewoph subscribed.

Moving to Needs Discussion for us to discuss the UI changes this would entail + code review

Tgr subscribed.

This is how I see those pages:

en.m.wikipedia.org_w_index.php_title=Special_UserLogin.png (969×1 px, 100 KB)
en.m.wikipedia.org_w_index.php_title=Special_CreateAccount.png (1×1 px, 163 KB)
loginsignup

so pretty much identical, and non-full-width above ~1000px viewport width. If the patch changes that, that would be a bug.
(Adding Reading Web as they maintain the mobile skin of these pages.)

This is how I see those pages:

en.m.wikipedia.org_w_index.php_title=Special_UserLogin.png (969×1 px, 100 KB)
en.m.wikipedia.org_w_index.php_title=Special_CreateAccount.png (1×1 px, 163 KB)
loginsignup

so pretty much identical, and non-full-width above ~1000px viewport width. If the patch changes that, that would be a bug.
(Adding Reading Web as they maintain the mobile skin of these pages.)

My screenshots:

  1. patch

image.png (1×1 px, 118 KB)

  1. no patch

image.png (1×1 px, 116 KB)

The patch version seems problematic.

Also, at a wider viewport, the line height seems wrong:

  1. patch

image.png (1×1 px, 180 KB)

  1. no patch

image.png (1×2 px, 202 KB)

The login screen seems off as well on wider viewport:

  1. patch

image.png (1×2 px, 139 KB)

  1. no patch

image.png (1×2 px, 136 KB)

On a phone sized viewport it seems less wrong, but notable that "Join {SITENAME}" is pushed down to the bottom of the viewport, presumably out of the viewport in smaller displays:

  1. patch

image.png (1×880 px, 93 KB)

  1. no patch

image.png (1×880 px, 98 KB)

cc @RHo @MMiller_WMF

This is how I see those pages:

en.m.wikipedia.org_w_index.php_title=Special_UserLogin.png (969×1 px, 100 KB)
en.m.wikipedia.org_w_index.php_title=Special_CreateAccount.png (1×1 px, 163 KB)
loginsignup

so pretty much identical, and non-full-width above ~1000px viewport width. If the patch changes that, that would be a bug.
(Adding Reading Web as they maintain the mobile skin of these pages.)

My screenshots:

  1. patch

image.png (1×1 px, 118 KB)

  1. no patch

image.png (1×1 px, 116 KB)

The patch version seems problematic.

Also, at a wider viewport, the line height seems wrong:

  1. patch

image.png (1×1 px, 180 KB)

  1. no patch

image.png (1×2 px, 202 KB)

The login screen seems off as well on wider viewport:

  1. patch

image.png (1×2 px, 139 KB)

  1. no patch

image.png (1×2 px, 136 KB)

On a phone sized viewport it seems less wrong, but notable that "Join {SITENAME}" is pushed down to the bottom of the viewport, presumably out of the viewport in smaller displays:

  1. patch

image.png (1×880 px, 93 KB)

  1. no patch

image.png (1×880 px, 98 KB)

cc @RHo @MMiller_WMF

I agree the patch appears to be introducing a pretty noticeable display bug in making the fields non-full width on devices (most noticeable on @kostajh's screenshot F34677048).
Since Reading Web maintains the mobile skin though, would this be handled on their backlog to patch the patch...?

Jdlrobson added subscribers: ovasileva, Jdlrobson.

The mobile login page is the same as the desktop login page. This change relates to desktop CSS loading on mobile. The web team typically doesn't maintain this page (although cc @ovasileva as it sounds like we could benefit from some clarity around that).

It sounds like the guidance here, is to avoid any changes for mobile. If so, I've suggested an approach on the patch which keeps the status quo.

The web team typically doesn't maintain this page (although cc @ovasileva as it sounds like we could benefit from some clarity around that

Indeed, it looks like User login falls in the "Special pages" row on https://www.mediawiki.org/wiki/Developers/Maintainers, which says unmaintained 🙀

Theoretically I think the relevant component is MediaWiki-User-login-and-signup, maintained by the Platform team. Not sure how well they are set up to review frontend changes though.

I've added #platform-engineering as it looks like Special:UserLogin is in their domain.