Page MenuHomePhabricator

Variant C/D: smaller start module
Open, Needs TriagePublic

Description

For the variant tests in T246533: Variant tests: "initiation part 2" test (C vs. D), both variants have changes to the startup module in common.

Changes:

  • Remove the account, tutorial and startediting submodules, leaving only email
  • Simplify the email design to just the email icon, the user's email address, and a "confirm" link
  • On mobile, remove the two-stage structure (with a mobile preview that, when clicked, navigates to the full version of the module) and also just show the email address and a confirm link

Mockups: desktop, mobile.

Different states of the email (also shown across the Zeplin mocks for Var D desktop):

Email not set
Unconfirmed email
Confirmed email

Note:

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 15 2020, 12:42 AM
Catrope added a subscriber: RHo.Jul 15 2020, 12:43 AM

@RHo could you address this question?

What should the start module look like in each of the three email states? The mockups show the "email set but unconfirmed" state, but what should the "email set and confirmed" and "email not set" states look like?

either by commenting or by editing the task description

Should this only apply to variant C/D (but not variant A), or should we make this change globally?

@MMiller_WMF points out that this change is incongruent with variant A, because it would kill the call to action to initiate the suggested edits module (which is in the start module in variant A)

RHo added a comment.Jul 22 2020, 9:07 PM

Should this only apply to variant C/D (but not variant A), or should we make this change globally?

@MMiller_WMF points out that this change is incongruent with variant A, because it would kill the call to action to initiate the suggested edits module (which is in the start module in variant A)

Yes, I would agree we should apply only to variant C/D.

  • Should the email address be obscured, as shown in the mockup? If so, what are the specifications for how it should be obscured?

This comes from a ticket in the backlog T237312 so perhaps we can handle it separately on that ticket?

  • What should the start module look like in each of the three email states? The mockups show the "email set but unconfirmed" state, but what should the "email set and confirmed" and "email not set" states look like?
Email not set
Unconfirmed email
Confirmed email

Will also add to the task description.

RHo updated the task description. (Show Details)Jul 22 2020, 9:10 PM
kostajh removed kostajh as the assignee of this task.Jul 27 2020, 6:06 PM

Unlicking the 🍪

I started looking at this but won't have time to get a patch up before vacation.

I think a good way forward might be to create a new module ("EmailCheck" module or whatever) and use that in place of the Start module. The existing Email module is too closely wired to the rest of the start submodule paradigm to make overriding it in PHP and CSS a simple thing to do.

Change 616810 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] [WIP] Homepage: Add StartEmail module

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

I've fleshed out Kosta's patch and it's looking good on desktop, but I still have to fix some issues on mobile.

No emailUncomfirmed emailConfirmed email

A couple of changes that diverge from the Zeplin mocks:

  1. the start-email module on mobile has a border, like the other modules.
    But the zeplin mock shows it without one. Should we keep the border for consistency? The desktop mock also shows it without a border (and the larger suggested edits module without a border), although the mobile mock shows suggested edits module with a border. So per the mocks, should it be: (desktop) start-email, no border, (desktop) suggested edits, no border; (mobile) start-email, no border, (mobile) suggested edits, with border?
    1. on mobile the width needs to be adjusted, and the alignment is off, but I think @Catrope plans to address this in a follow-up, as there are other layout issues when Special:Homepage is loaded in landscape orientation with the layout patch T258005
  2. The Zeplin mocks show the "confirm" and "change" text linked, but the surrounding parentheses are not linked. @RHo do you have a preference on this? I'm not sure if we have an existing convention in MediaWiki for whether the parentheses should form part of the link label.
  3. The "confirm" link in the mocks is red, while in @Catrope's patch it's blue. Red links are a thing in MediaWiki, but I think the standard blue color is more correct in this context, so I wanted to double-check with @RHo about that
RHo added a comment.Aug 19 2020, 10:10 AM

Hi @kostajh - please see comments below:

A couple of changes that diverge from the Zeplin mocks:

  1. the start-email module on mobile has a border, like the other modules.
    But the zeplin mock shows it without one. Should we keep the border for consistency? The desktop mock also shows it without a border (and the larger suggested edits module without a border), although the mobile mock shows suggested edits module with a border. So per the mocks, should it be: (desktop) start-email, no border, (desktop) suggested edits, no border; (mobile) start-email, no border, (mobile) suggested edits, with border?

– Yes, it should be per the Zeplin mocks please as you've mentioned, but perhaps the border is in the same pale blue as the background instead of no border if that makes more sense in the code change. I want to note though that the pre-initiated onboarding SE-module in variant D does have the base80 border.

A. on mobile the width needs to be adjusted, and the alignment is off, but I think @Catrope plans to address this in a follow-up, as there are other layout issues when Special:Homepage is loaded in landscape orientation with the layout patch T258005

– sgtm

  1. The Zeplin mocks show the "confirm" and "change" text linked, but the surrounding parentheses are not linked. @RHo do you have a preference on this? I'm not sure if we have an existing convention in MediaWiki for whether the parentheses should form part of the link label.

– Let's go with *not* linking the brackets as shown in Zeplin, since this appears to be the standard seen in Special:Preferences parenthetical links, and also "Edit" links on article headers:

  1. The "confirm" link in the mocks is red, while in @Catrope's patch it's blue. Red links are a thing in MediaWiki, but I think the standard blue color is more correct in this context, so I wanted to double-check with @RHo about that

– Yes, I wanted it red to imply that this email does not technically exist yet, to emphasize the need to confirm the email. For the purposes of finishing this task we can leave as is for now, but I have created a new "leftovers" item T260780 which is a proposal to make both the "Add email" and "Confirm email" versions a little more prominent so that users know to complete this activity.

kostajh assigned this task to Catrope.Aug 19 2020, 11:51 AM

– Yes, it should be per the Zeplin mocks please as you've mentioned, but perhaps the border is in the same pale blue as the background instead of no border if that makes more sense in the code change.

Border removed for the start module (leaving the SE module for T258704).

I want to note though that the pre-initiated onboarding SE-module in variant D does have the base80 border.

Thanks for pointing this out, I've added this to T258704 (although the border color is base70, not base80).

  1. The Zeplin mocks show the "confirm" and "change" text linked, but the surrounding parentheses are not linked. @RHo do you have a preference on this? I'm not sure if we have an existing convention in MediaWiki for whether the parentheses should form part of the link label.

– Let's go with *not* linking the brackets as shown in Zeplin, since this appears to be the standard seen in Special:Preferences parenthetical links, and also "Edit" links on article headers:

This came from me trying to reuse the i18n message for the existing (change) link in the variant A start module, where the parentheses are inside the link.


But you're right that we don't generally do this, so I've changed it.

  1. The "confirm" link in the mocks is red, while in @Catrope's patch it's blue. Red links are a thing in MediaWiki, but I think the standard blue color is more correct in this context, so I wanted to double-check with @RHo about that

– Yes, I wanted it red to imply that this email does not technically exist yet, to emphasize the need to confirm the email. For the purposes of finishing this task we can leave as is for now, but I have created a new "leftovers" item T260780 which is a proposal to make both the "Add email" and "Confirm email" versions a little more prominent so that users know to complete this activity.

Making it red was easy, so I've done that. I like the designs in the leftovers task, but since it's a leftover I will resist the temptation to implement them right away :)

  1. The "confirm" link in the mocks is red, while in @Catrope's patch it's blue. Red links are a thing in MediaWiki, but I think the standard blue color is more correct in this context, so I wanted to double-check with @RHo about that

– Yes, I wanted it red to imply that this email does not technically exist yet, to emphasize the need to confirm the email. For the purposes of finishing this task we can leave as is for now, but I have created a new "leftovers" item T260780 which is a proposal to make both the "Add email" and "Confirm email" versions a little more prominent so that users know to complete this activity.

Making it red was easy, so I've done that. I like the designs in the leftovers task, but since it's a leftover I will resist the temptation to implement them right away :)

@RHo, I commented on the patch but perhaps we should just make it a normal blue link. What I wrote on the patch:

This doesn't play nicely with MobileFrontend's initRedlinksCta() code, which looks for red links and loads a drawer with a call to action to create a page rather than taking you directly to it.

So, after you tap "confirm" you'd see:

We could add code to MobileFrontend to special case this type of situation but for now a regular blue link is the easiest thing to do.

RHo added a comment.Sep 1 2020, 10:29 AM
  1. The "confirm" link in the mocks is red, while in @Catrope's patch it's blue. Red links are a thing in MediaWiki, but I think the standard blue color is more correct in this context, so I wanted to double-check with @RHo about that

– Yes, I wanted it red to imply that this email does not technically exist yet, to emphasize the need to confirm the email. For the purposes of finishing this task we can leave as is for now, but I have created a new "leftovers" item T260780 which is a proposal to make both the "Add email" and "Confirm email" versions a little more prominent so that users know to complete this activity.

Making it red was easy, so I've done that. I like the designs in the leftovers task, but since it's a leftover I will resist the temptation to implement them right away :)

@RHo, I commented on the patch but perhaps we should just make it a normal blue link. What I wrote on the patch:

This doesn't play nicely with MobileFrontend's initRedlinksCta() code, which looks for red links and loads a drawer with a call to action to create a page rather than taking you directly to it.

So, after you tap "confirm" you'd see:

We could add code to MobileFrontend to special case this type of situation but for now a regular blue link is the easiest thing to do.

SGTM @kostajh - maybe this can be looked at with the T260780 leftovers task.

Merging the patch as is, although it needs some more work due to the layout on mobile for variant C / D users:

Change 616810 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Homepage: Add StartEmail module

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

Merging the patch as is, although it needs some more work due to the layout on mobile for variant C / D users:

@Catrope I'll leave this layout fix for you, ok?

Change 626519 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] Homepage: Fix mobile summary styling when not wrapped in a link

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

Change 626519 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Homepage: Fix mobile summary styling when not wrapped in a link

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

Etonkovidova added a subscriber: Etonkovidova.

For Design Review:

(minor) Note: on Desktop - 'Add email' - may be make it bold? All other links on the page are bold. It'd look more stand out e.g.

(for illustration - no issues)
Desktop

Email not set
Unconfirmed email
Confirmed email

Mobile:

Email not set
Unconfirmed email
Confirmed email

Change 627621 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Revert "Homepage: Fix mobile summary styling when not wrapped in a link"

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

The changes look good for variation C/D, but for variation A users there are some problems, so I've created a revert of the patch although it may will be easier to just make a new patch which fixes the issues; I'll leave that up to @Catrope.

Change 627911 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] Homepage: Fix styling for mobile start module

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

Remarkably, all three of these fairly disparate-looking styling issues were because of one silly CSS mistake. The attached patch fixes it.

Change 627911 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Homepage: Fix styling for mobile start module

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

Change 627802 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@wmf/1.36.0-wmf.9] Homepage: Fix styling for mobile start module

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

Change 627621 abandoned by Catrope:
[mediawiki/extensions/GrowthExperiments@master] Revert "Homepage: Fix mobile summary styling when not wrapped in a link"

Reason:
Superseded by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/ /627911/

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

Change 627802 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@wmf/1.36.0-wmf.9] Homepage: Fix styling for mobile start module

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

Mentioned in SAL (#wikimedia-operations) [2020-09-16T23:37:39Z] <catrope@deploy1001> Synchronized php-1.36.0-wmf.9/extensions/GrowthExperiments/: Fix styling for mobile start module (T258008); Revert wider task card on desktop (T263042, T258704); Fix width of sidebar modules in narrow mode in variant A (T263068) (duration: 01m 09s)

The issues mentioned in this comment - have been fixed.
One thing to take a look at - the arrows look kind of big (it's in production)

Moving again to Design Review - the screenshots in my comment still need to be reviewed. Also I went through usual cross browser compatibility checks - looks fine.

Thanks @Etonkovidova - the arrows are actually larger on the new desktop version, but you're right about "Add email".
Moving back "needs more work" to fix the following:
*"Add email" should be bold (per the task description).

  • Text (for all states - Add email, unconfirmed, and confirmed email) should be vertically centered with the icons.

The issues mentioned in this comment - have been fixed.
One thing to take a look at - the arrows look kind of big (it's in production)

Moving again to Design Review - the screenshots in my comment still need to be reviewed. Also I went through usual cross browser compatibility checks - looks fine.

! In T258008#6464944, @Etonkovidova wrote:

For Design Review:

(minor) Note: on Desktop - 'Add email' - may be make it bold? All other links on the page are bold. It'd look more stand out e.g.

(for illustration - no issues)
Desktop

Email not set
Unconfirmed email
Confirmed email

Mobile:

Email not set
Unconfirmed email
Confirmed email

Change 630970 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] StartEmail: Styling fixes

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

Change 630970 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] StartEmail: Styling fixes

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

  • bold for the link Add email - corrected.

Text (for all states - Add email, unconfirmed, and confirmed email) should be vertically centered with the icons.

Please review

.growthexperiments-homepage-module-startemail .growthexperiments-homepage-startemail-address-wrapper {
    vertical-align: middle;

hi @Catrope and @Etonkovidova - unfortunately it looks like the font-size and icon size has gotten bigger than expected now, presumably related to the base change that was made to fix the other items on the onboarding screens.

Actual
  • Icon larger than expected (should be 20x20px)
  • Font-size is larger than expected – it's expected that this should use the OOUI 'tag' font style for smaller UI text, which on Desktop is font-size:13px, and on Mobile is font-size:14.8px
  • Icon colour for unconfirmed and confirmed email should also be Base20 instead of Base10 (unrelated to the font-size issue)

Change 633285 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] StartEmail: Styling tweaks

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

  • Icon larger than expected (should be 20x20px)

It was 20x20 on desktop, and larger on mobile proportional to the base font size. I've fixed it to be 20x20 on both.

  • Font-size is larger than expected – it's expected that this should use the OOUI 'tag' font style for smaller UI text, which on Desktop is font-size:13px, and on Mobile is font-size:14.8px

Fixed. It looks small though.

  • Icon colour for unconfirmed and confirmed email should also be Base20 instead of Base10 (unrelated to the font-size issue)

That icon wasn't even base10, it was pure black. Changed to base20 (or, technically, 65% opacity).

Change 633285 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] StartEmail: Styling tweaks

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

(1)

  • Icon larger than expected (should be 20x20px)

It was 20x20 on desktop, and larger on mobile proportional to the base font size. I've fixed it to be 20x20 on both.

Desktopoverall look
Mobileoverall look

(2)

  • Font-size is larger than expected – it's expected that this should use the OOUI 'tag' font style for smaller UI text, which on Desktop is font-size:13px, and on Mobile is font-size:14.8px

Fixed. It looks small though.

Note: It does look small, but it seems to be passing the accessibility test.

DesktopMobile

(3)

  • Icon colour for unconfirmed and confirmed email should also be Base20 instead of Base10 (unrelated to the font-size issue)

That icon wasn't even base10, it was pure black. Changed to base20 (or, technically, 65% opacity).

.growthexperiments-homepage-module-startemail .oo-ui-iconWidget:not(.oo-ui-image-progressive) {
    opacity: 0.65;
}

@Catrope provided the screenshots - betalabs UI is the same.

Thanks all, LGTM!

(1)

  • Icon larger than expected (should be 20x20px)

It was 20x20 on desktop, and larger on mobile proportional to the base font size. I've fixed it to be 20x20 on both.

Desktopoverall look
Mobileoverall look

(2)

  • Font-size is larger than expected – it's expected that this should use the OOUI 'tag' font style for smaller UI text, which on Desktop is font-size:13px, and on Mobile is font-size:14.8px

Fixed. It looks small though.

Note: It does look small, but it seems to be passing the accessibility test.

DesktopMobile

This is right and per spec.

(3)

  • Icon colour for unconfirmed and confirmed email should also be Base20 instead of Base10 (unrelated to the font-size issue)

That icon wasn't even base10, it was pure black. Changed to base20 (or, technically, 65% opacity).

.growthexperiments-homepage-module-startemail .oo-ui-iconWidget:not(.oo-ui-image-progressive) {
    opacity: 0.65;
}

@Catrope provided the screenshots - betalabs UI is the same.

Thanks, good catch.