Change 548553 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Fix styling of difficulty indicator on mobile

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

Change 548553 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Fix styling of difficulty indicator on mobile

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

Change 551300 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] TaskExplanationWidget: Make the (i) icon bigger on mobile

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

Change 551301 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] StartEditingDialog: Convert to ProgressDialog for mobile support

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

Change 551302 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] StartEditingDialog: Change layout for mobile

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

(1) Mobile Intro overlay display issues

• The link at the bottom of the intro screen cannot be clicked since it cannot be scrolled into the view

This was because the CTA buttons were in the way; now that they're in the header, this is fixed.

• CTA buttons should be sticky in the dialog header on mobile.

Fixed in the second patch (StartEditingDialog: Convert to ProgressDialog for mobile support)

• Bold text headers should be a separate paragraph.

I didn't notice that the bold text and the plain text following it were supposed to have been different messages, because they're in the same paragraph in desktop (but apparently not in mobile?!). Rather than splitting the messages and forcing them to be retranslated, I hacked around this by applying strong { display: block; } to these paragraphs, as well as forcing a float clear after the <strong> tags so that the non-bolded text in the second paragraph always goes below the floated image. This is in the third patch (StartEditingDialog: Change layout for mobile).

(2) Mobile version of Difficulty overlay should be a single column.

Fixed in the third patch (StartEditingDialog: Change layout for mobile).

(3) Mobile suggested edits card

• The difficulty level labels should be centered

Done in the previous patch (Nov 5).

• Info "i" icon size and tap target should be larger on mobile

Fixed in the first patch (TaskExplanationWidget: Make the (i) icon bigger on mobile)

• More info card on mobile should be a full width card that appears up from the bottom (similar to how references on articles appear on in Minerva)

This has its own task already: T238164.

(4) "Get suggestions' and 'Cancel' buttons should not partially overlay the last portion of text

This was caused by the buttons being at the bottom, moving them into the header fixes this.

(5) The image should be different on mobile's difficulty overlay (it is Difficulty-landscape-rtl.svg found in T235444) to accommodate the mobile format.

Fixed in the third patch (StartEditingDialog: Change layout for mobile).

Change 551300 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] TaskExplanationWidget: Make the (i) icon bigger on mobile

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

Change 551301 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] StartEditingDialog: Convert to ProgressDialog for mobile support

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

Change 551302 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] StartEditingDialog: Change layout for mobile

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

This looks good to me in beta. @RHo please review!

This is really close, but picked up a few unresolved and new issues based on looking at the beta today and seeing the following:

Intro	Difficulty

(1) Mobile Intro overlay display issues
The link at the bottom of the intro screen cannot be clicked since it cannot be scrolled into the view
CTA buttons should be sticky in the dialog header on mobile.
– There is a ~1px space between the bottom of the mobile dialog button and the header section

(2) Mobile version of Difficulty overlay should be a single column.

(3) Mobile suggested edits card
i. The difficulty level labels should be centered
ii. Info "i" icon size and tap target should be larger on mobile
– I think this is due to the old smaller info icon still being in use on OOUI where there is padding around the 20x20px icon but this scaling makes it look not good. Can we instead use the following icon asset for mobile? Tap target is good now.

(4) "Get suggestions' and 'Cancel' buttons should not partially overlay the last portion of text

(5) The image should be different on mobile's difficulty overlay (it is Difficulty-landscape-rtl.svg found in T235444) to accommodate the mobile format.

New issues after latest round of fixes

(6) Many text styles are now larger than expected on mobile:
i) Intro overlay and difficulty overlay headers should be 16px, now they appear to be ~19.2px (1.2em)
ii) 'Subheader' paragraph text on intro and difficulty overlay should be 14.8px.

iii) Easy, Medium, Hard labels in the filter should be 18px and be vertically centered with the icon.
iv) SE Error/Exception title "No suggestions available..." text should be font-size: 16px
v) SE Error/Exception message text ("Please try again later") should be font-size: 16px
vi) SE footer text ("Other users have noted...") should be font-size: 12.8px

(7) Heart graphic on intro overlay now should have zero top padding (there is now too much space between the heart article image and the stepper)

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

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

– There is a ~1px space between the bottom of the mobile dialog button and the header section

Somewhat surprisingly, reducing the base font size in the dialog from 16px to 14px fixes this. That's because this is a pixels vs ems rounding error in OOUI, and the rounding works in our favor with a 14px base but not with a 16px base.

(6) Many text styles are now larger than expected on mobile:
i) Intro overlay and difficulty overlay headers should be 16px, now they appear to be ~19.2px (1.2em)

This is fixed by reducing the base font size in the dialog.

ii) 'Subheader' paragraph text on intro and difficulty overlay should be 14.8px.

This font size difference does not appear in the desktop design (where they're all 16px), only in the mobile design (where the top non-specific text is all 14.8px, both header and subheader, and the specific ones are 16px). I'm holding off on implementing this until you tell me it's deliberate.

There are also some discrepancies like this between the desktop and mobile versions of the difficulty panel: the "There are many ways you can contribute" text is the same size as the bold text above it on desktop (16px) but not on mobile (14.8px). The bold and non-bold labels for each difficulty level are the same size on desktop (both 14px) but not on mobile (16px vs 14.8px). I'm implementing these font size changes because I do think they look better, but I'm a little surprised at the differences here and there. I'm not implementing the text color difference for the "There are many ways" text (keeping it as colorGray5 even though the mobile mocks have it as colorGray2) because I'm assuming that's an oversight, unless you tell me it's deliberate.

There should be bottom padding after bolded paragraphs of text on both overlay

Added 1em of bottom padding to the strong tag in the "specific" (bottom) section. Based on the mocks, it looks like there shouldn't be bottom padding between the (bolded) header and the (non-bolded) subheader in the non-specific (top) section.

– It is now a separate paragraph, but the text should not wrap around the image

This will probably happen less often with the reduced font size, but we can't prevent this completely without making more significant changes to both the HTML and CSS than I have time to make at this point.

iii) Easy, Medium, Hard labels in the filter should be 18px and be vertically centered with the icon.

I've increased the font size. Couldn't quite get it centered, but increasing the font size made that problem a lot better already. This is also an interim solution until we do T238460.

iv) SE Error/Exception title "No suggestions available..." text should be font-size: 16px

Fixed.

v) SE Error/Exception message text ("Please try again later") should be font-size: 16px

It already is? At least in the "No suggestions found" card the "Select other difficulty levels" text is 16px, and all of these exceptional cards are styled the same way

vi) SE footer text ("Other users have noted...") should be font-size: 12.8px

Since this is also called out in T238280 I'm going to tackle it there.

(7) Heart graphic on intro overlay now should have zero top padding (there is now too much space between the heart article image and the stepper)

It turns out I put the heart above the header instead of below it, which was an oversight on my part. Fixing that should also fix the spacing issue (since the padding is not on the image itself, but on the div that contains both it and the header text).

Change 552143 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] StartEditingDialog: Styling tweaks for mobile

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

In T236854#5680146, @Catrope wrote:

– There is a ~1px space between the bottom of the mobile dialog button and the header section

Somewhat surprisingly, reducing the base font size in the dialog from 16px to 14px fixes this. That's because this is a pixels vs ems rounding error in OOUI, and the rounding works in our favor with a 14px base but not with a 16px base.

– Great, I can see it fixed on beta.

(6) Many text styles are now larger than expected on mobile:
i) Intro overlay and difficulty overlay headers should be 16px, now they appear to be ~19.2px (1.2em)

This is fixed by reducing the base font size in the dialog.

– Actually this is still slightly larger than expected, since 14*1.2 = 16.8px... can we change the base font-size to 13.333px? Or might that lead to a lot of unexpected padding or font-size discreprancies in other places (or a bad idea for another reason)?

ii) 'Subheader' paragraph text on intro and difficulty overlay should be 14.8px.

This font size difference does not appear in the desktop design (where they're all 16px), only in the mobile design (where the top non-specific text is all 14.8px, both header and subheader, and the specific ones are 16px). I'm holding off on implementing this until you tell me it's deliberate.

– This was deliberate as a way to reduce the space taken up by this first text block on mobile pushing down the 'specific' reason text. But I see your point about keeping as 16px for consistency.

There are also some discrepancies like this between the desktop and mobile versions of the difficulty panel: the "There are many ways you can contribute" text is the same size as the bold text above it on desktop (16px) but not on mobile (14.8px). The bold and non-bold labels for each difficulty level are the same size on desktop (both 14px) but not on mobile (16px vs 14.8px). I'm implementing these font size changes because I do think they look better, but I'm a little surprised at the differences here and there. I'm not implementing the text color difference for the "There are many ways" text (keeping it as colorGray5 even though the mobile mocks have it as colorGray2) because I'm assuming that's an oversight, unless you tell me it's deliberate.

– Thanks for making the changes, the differences in text-size are intentional because on Desktop the difficulty level explanations are shown in two columns where I wanted smaller text to help reduce vertical spacing in the dialog, whereas on Mobile it is single column. The colorgray2 instead of colorgray5 on Mobile for that text is an oversight though, so I've updated now on the Zeplin.

There should be bottom padding after bolded paragraphs of text on both overlay

Added 1em of bottom padding to the strong tag in the "specific" (bottom) section. Based on the mocks, it looks like there shouldn't be bottom padding between the (bolded) header and the (non-bolded) subheader in the non-specific (top) section.

– Yes, you're right, thanks for fixing.

– It is now a separate paragraph, but the text should not wrap around the image

This will probably happen less often with the reduced font size, but we can't prevent this completely without making more significant changes to both the HTML and CSS than I have time to make at this point.

– Fair enough, let's leave it for now.

iii) Easy, Medium, Hard labels in the filter should be 18px and be vertically centered with the icon.

I've increased the font size. Couldn't quite get it centered, but increasing the font size made that problem a lot better already. This is also an interim solution until we do T238460.

– OK let's leave until that other task is worked on.

iv) SE Error/Exception title "No suggestions available..." text should be font-size: 16px

Fixed.

v) SE Error/Exception message text ("Please try again later") should be font-size: 16px

It already is? At least in the "No suggestions found" card the "Select other difficulty levels" text is 16px, and all of these exceptional cards are styled the same way

– Same comment as before about base font size, but not deal-breakers.

vi) SE footer text ("Other users have noted...") should be font-size: 12.8px

Since this is also called out in T238280 I'm going to tackle it there.

– OK

(7) Heart graphic on intro overlay now should have zero top padding (there is now too much space between the heart article image and the stepper)

It turns out I put the heart above the header instead of below it, which was an oversight on my part. Fixing that should also fix the spacing issue (since the padding is not on the image itself, but on the div that contains both it and the header text).

– Great, LGTM.

– Actually this is still slightly larger than expected, since 14*1.2 = 16.8px... can we change the base font-size to 13.333px? Or might that lead to a lot of unexpected padding or font-size discreprancies in other places (or a bad idea for another reason)?

Approximately everything that you asked to be 16px (including on desktop) is actually 16.8px for this reason. We can make things 16/14em (=1.142857...em) instead of 1.2em if you want, or just hard-code them to 16px (I'm not sure where we're at with the whole em->px partial migration). But you guessed correctly that changing the base font-size to 13.333px will make a big mess.

– This was deliberate as a way to reduce the space taken up by this first text block on mobile pushing down the 'specific' reason text. But I see your point about keeping as 16px for consistency.

OK, happy to implement it, just checking. It was somewhere in the grey area between "this seems consistent enough that it's definitely intended" and "this seems inconsistent enough that it's definitely not intended".

In T236854#5682597, @Catrope wrote:

– Actually this is still slightly larger than expected, since 14*1.2 = 16.8px... can we change the base font-size to 13.333px? Or might that lead to a lot of unexpected padding or font-size discreprancies in other places (or a bad idea for another reason)?

Approximately everything that you asked to be 16px (including on desktop) is actually 16.8px for this reason. We can make things 16/14em (=1.142857...em) instead of 1.2em if you want, or just hard-code them to 16px (I'm not sure where we're at with the whole em->px partial migration). But you guessed correctly that changing the base font-size to 13.333px will make a big mess.

Ah cool. Let's make it 1.14285714em instead since that seems the most straightforward (?) update at this point.

I went through comments and recent changes/fixes and below is the summary of major points for this task:

(1) This is the only issue that I saw not addressed - the text says to click 'Cancel' button but there is no 'Cancel' button anymore

(2) A list of tasks that would address the issues not resolved in this task

• T238280: Newcomer tasks: mobile module styling issues
• T238460: Newcomer tasks: difficulty filter styling issues on mobile
• T238164: Newcomer tasks: Use Drawer component for mobile UI

(3) The screenshot summary - to illustrate the fixes (all seem to be in place).

Before	Now

Thanks. It looks like this didn't go to Design Review, so I'm putting it there. @RHo -- if you don't see any issues, than please resolve the task.

@Etonkovidova -- that wording issue with the "Cancel" button, I think we should just let that slide. To accomodate that, we would need different copy for mobile (which has an "X") and desktop (which has a cancel button), or we would need to change the copy to something that works for both. I don't think it's worth it.

In T236854#5676173, @RHo wrote:

(3) Mobile suggested edits card
ii. Info "i" icon size and tap target should be larger on mobile
– I think this is due to the old smaller info icon still being in use on OOUI where there is padding around the 20x20px icon but this scaling makes it look not good. Can we instead use the following icon asset for mobile? Tap target is good now.

mobile-info-icon.svg545 BDownload

hi @Catrope and @MMiller_WMF - everything looks good now except the old info icon is shown and still seems a bit vertically misaligned. Can we update to the asset attached above for mobile and desktop? Not a dealbreaker but it would certainly be great if possible as currently it looks a bit out of kilter with the rest of the icons.

Okay, I'll put this back in Ready for Development so we can get the icon right.

Change 557105 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Consistently use suggested-edits-, not suggestededits-

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

We hadn't yet updated the icon, but we had been using the old icon but enlarged and positioned in a better way. This regressed in rEGREa64079ba6964: Suggested Edits: Use Drawer component on mobile, which broke the mobile-specific styling for this icon by introducing a typo in the CSS class name for its wrapper. The first patch fixes the typo and restores the old styling. I'll submit a patch to use the new icon after lunch.

Change 557139 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Use unpadded (i) icon on mobile

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

Change 557105 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Consistently use suggested-edits-, not suggestededits-

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

Change 557139 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Use unpadded (i) icon on mobile

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

Checking on testwiki wmf.11 - the info icon is bigger (and somwhat stretched from https://phabricator.wikimedia.org/F31102040 (20x20px) as @RHo noticed).

testwiki wmf.11	cswiki wmf.10

However, on iPhone 6s (a real device) the icon looks much better:

@Etonkovidova -- what should happen next here? Are you still testing or should this go to Design Review?

In T236854#5750211, @MMiller_WMF wrote:

@Etonkovidova -- what should happen next here? Are you still testing or should this go to Design Review?

I needed to test it after wmf.11 deployment to group 2 to see how it looks on mobile. Yes, now it's up to @RHo to decide if additional adjustments are needed:

wmf.11 iPhone 6s (a real device):

Thanks @Etonkovidova –this better on the real device.