Page MenuHomePhabricator

Newcomer tasks: mobile module styling issues
Closed, ResolvedPublic

Description

@RHo recorded a set of styling issues to be fixed on the mobile version of the suggested edits module:

A. Filter issues ticketed separately in T238460: Newcomer tasks: difficulty filter styling issues on mobile

B. Suggested edits pager text should be…
( patch )

  • font-size: 12.8px / 0.8em.
  • have with 32px padding-top on mobile (between it and the filter)
  • have with 24px padding-r on mobile (between it and the filter)

C. Image should be…
( patch )

  • scaled to be full-width of card (See bottom of description for example of when this does not happen)
  • height is 128px (ratio 1:2 with image width)
  • is there any rules or algorithm used by Page Previews to center image previews on a person's face? Instead of cutting off faces?

D. Placeholder image should not have dark borders on the edges
( no longer an issue per T238280#5698220 )

E. Left and right arrow icons should be 16px from left and right edges of card respectively, and sized 20x20px.
( patch )

F. Article text title should…
( patch )

  • be font-size: 16px
  • have 8px top padding (between it and the image)
  • have 16px left and right padding

G. Article text extract should…
( patch )

  • be font-size: 12.8px
  • have 4px top padding (between it and the title)
  • have 16px left and right padding
  • have line-height: 19.2px

H. Suggested task explanation section should…
( done, can't find patch link at the moment)

  • have padding-top 24px (between task type and bottom of card)
  • have padding-bottom 32px (between task type short description text and card footer section)

I. Difficulty level tag should end with ribbon points ending flush against RHS edge of card (possibly fixed by adding to class .suggested-edits-difficulty-indicator { margin-right: 4px; }
( patch )

J. Task type short description text should be font-size 12.8px and line-height: 19.2px
( done, can't find patch link at the moment )

K. Card footer text should be font-size 12.8px and line-height: 19.2px
( patch )

L. Only the card footer is white, the rest of the Suggested edits module background-color should be rgba (234,243,255,0.5).
( patch )

M. Pageviews icon and text should…
( patch )

  • be vertically center aligned to each other (since text should only be kept to 1 line)
  • have the text at font-size:12px
  • have the icon at 16x16px (80% icon size)
  • have the icon be coloured #54595d (or else #222 at opacity:0.75)

Example of image not being full width

When this article is a suggested edit: https://cs.wikipedia.org/wiki/IBM_PCjr

Its image ends up with white bars on the sides:

Details

ProjectBranchLines +/-Subject
mediawiki/extensions/GrowthExperimentsmaster+73 -3
mediawiki/extensions/GrowthExperimentsmaster+5 -0
mediawiki/extensions/GrowthExperimentsmaster+2 -2
mediawiki/extensions/GrowthExperimentsmaster+2 -0
mediawiki/extensions/GrowthExperimentsmaster+4 -1
mediawiki/extensions/GrowthExperimentsmaster+3 -3
mediawiki/extensions/GrowthExperimentsmaster+5 -0
mediawiki/extensions/GrowthExperimentsmaster+15 -0
mediawiki/extensions/GrowthExperimentsmaster+8 -2
mediawiki/extensions/GrowthExperimentsmaster+9 -1
mediawiki/extensions/GrowthExperimentsmaster+1 -0
mediawiki/extensions/GrowthExperimentsmaster+6 -0
mediawiki/extensions/GrowthExperimentsmaster+13 -7
mediawiki/extensions/GrowthExperimentsmaster+3 -1
mediawiki/extensions/GrowthExperimentsmaster+5 -0
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
MMiller_WMF updated the task description. (Show Details)Nov 21 2019, 8:10 PM

Change 553337 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits (mobile): Adjust pager text

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

C. Image should be…

scaled to be full-width of card (See bottom of description for example of when this does not happen)
height is 128px (ratio 1:2 with image width)
is there any rules or algorithm used by Page Previews to center image previews on a person's face? Instead of cutting off faces?

There's not a great option here. Portrait orientation image thumbnails from restbase may or may not be under the minimum width (260px) we use for the card. We can stretch a 200px with image so that it's 260px but it will look pixelated and not great, in addition to already not looking that good because it's a portrait image being forced into a landscape orientation holder.

Another option would be to not show images at all if they're in portrait orientation, and just stick with the placeholder image.

D. Placeholder image should not have dark borders on the edges

Hmm, this is odd. Do you see this consistently? I don't see it on iOS nor via Chrome/Firefox on desktop using mobile view.

M. Pageviews icon and text should…

be vertically center aligned to each other (since text should only be kept to 1 line)

I think we are discussing this in another issue, but do you also want to hide the overflow in cases when the text extends beyond one line, maybe adding ellipses in this case?

RHo added a comment.Nov 27 2019, 7:06 PM

D. Placeholder image should not have dark borders on the edges

Hmm, this is odd. Do you see this consistently? I don't see it on iOS nor via Chrome/Firefox on desktop using mobile view.

Oh you're right, I'm no longer able to reproduce issue at all now, I think it is because the placeholder image has been added as a background-image on top of the background-color:#000 attribute in the .se-image-card class .

Change 553401 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested edits: Adjust mobile styling for previous/next

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

RHo added a comment.Nov 27 2019, 7:59 PM

C. Image should be…

scaled to be full-width of card (See bottom of description for example of when this does not happen)
height is 128px (ratio 1:2 with image width)
is there any rules or algorithm used by Page Previews to center image previews on a person's face? Instead of cutting off faces?

There's not a great option here. Portrait orientation image thumbnails from restbase may or may not be under the minimum width (260px) we use for the card. We can stretch a 200px with image so that it's 260px but it will look pixelated and not great, in addition to already not looking that good because it's a portrait image being forced into a landscape orientation holder.

Another option would be to not show images at all if they're in portrait orientation, and just stick with the placeholder image.

I have two proposals instead of stretching to fit when the thumbnails are too small, we can either:
A. Add background colour #EAEC90 (Base80) to the image (.se-card-image class):


or alternatively
B. Make the image background-repeat: repeat-x; to the class

My preference is for option B.

RHo added a comment.Nov 27 2019, 8:00 PM

M. Pageviews icon and text should…

be vertically center aligned to each other (since text should only be kept to 1 line)

I think we are discussing this in another issue, but do you also want to hide the overflow in cases when the text extends beyond one line, maybe adding ellipses in this case?

Yes thanks!

Change 553406 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust thumbnail presentation

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

Change 553410 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust styles for title on mobile

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

Change 553412 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust extract styling on mobile

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

Change 553337 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits (mobile): Adjust pager text

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

Change 553406 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust thumbnail presentation

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

Change 553410 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust styles for title on mobile

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

Change 553751 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust margin on difficulty indicator

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

Change 553412 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust extract styling on mobile

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

Change 553751 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Adjust margin on difficulty indicator

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

Change 555500 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits mobile (item L): Set background color

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

Change 555501 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested edits mobile (item K): Fix footer text size

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

Change 555505 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested edits mobile (item M): Adjust pageviews icon and text

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

kostajh updated the task description. (Show Details)Dec 6 2019, 2:19 PM
kostajh updated the task description. (Show Details)
kostajh moved this task from In Progress to Code Review on the Growth-Team (Current Sprint) board.

Change 553401 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested edits mobile (item E): Adjust styling for previous/next

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

Change 555500 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits mobile (item L): Set background color

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

Change 555501 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested edits mobile (item K): Fix footer text size

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

Change 555505 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested edits mobile (item M): Adjust pageviews icon and text

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

Change 555963 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Display pageview info as a single line

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

Change 555963 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Display pageview info as a single line

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

Etonkovidova added a comment.EditedDec 10 2019, 8:31 PM

B. Suggested edits pager text should be…

font-size: 12.8px / 0.8em.
 have with 32px padding-top on mobile (between it and the filter)
 have with 24px padding-r on mobile (between it and the filter)

Currently in betalabs

.growthexperiments-homepage-module-suggested-edits.growthexperiments-homepage-module-mobile-overlay .suggested-edits-pager, .growthexperiments-homepage-module-suggested-edits.growthexperiments-homepage-module-mobile-details .suggested-edits-pager {
    font-size: 0.8em;
    margin: 32px 0 24px 0;

To compare - testwiki

All below notes refer to the following screenshot (the same as in my previous comment).

C. Image should be…

  scaled to be full-width of card (See bottom of description for example of when this does not happen)
height is 128px (ratio 1:2 with image width)

D and E are done.

F. Article text title should…

be font-size: 16px

have 8px top padding (between it and the image)
have 16px left and right padding

"se-card-title"
    font-size: 1em;
    padding-top: 8px;

G. Article text extract should…

be font-size: 12.8px
have 4px top padding (between it and the title)

have 16px left and right padding
have line-height: 19.2px

"se-card-extract"  

padding-top: 4px;
    font-size: 0.8em;
    line-height: 1.5;

H. Suggested task explanation section should…

have padding-top 24px (between task type and bottom of card)
have padding-bottom 32px (between task type short description text and card footer section)

J. Task type short description text should be font-size 12.8px and line-height: 19.2px

NOTE: @RHo - is font-size ok here?
.suggested-edits-task-explanation-wrapper .suggested-edits-short-description {
    height: 20px;
    color: #54595d;
    font-size: 0.9em;
}

I. Difficulty level tag should end with ribbon points ending flush against RHS edge of card (possibly fixed by >adding to class .suggested-edits-difficulty-indicator { margin-right: 4px; }

K. Card footer text should be font-size 12.8px and line-height: 19.2px

NOTE: @RHo - line-height: 1.5; is fine?
.growthexperiments-homepage-module-suggested-edits.growthexperiments-homepage-module-mobile-overlay .suggested-edits-footer, .growthexperiments-homepage-module-suggested-edits.growthexperiments-homepage-module-mobile-details .suggested-edits-footer {
    font-size: 0.8em;
    line-height: 1.5;

L. Only the card footer is white, the rest of the Suggested edits module background-color should be rgba (234,243,255,0.5).

.body.mw-special-Homepage .overlay-content .growthexperiments-homepage-module-mobile-overlay.growthexperiments-homepage-module-suggested-edits {
    padding: 0;
    background-color: rgba(234,243,255,0.5);

M. Pageviews icon and text should…

be vertically center aligned to each other (since text should only be kept to 1 line)

have the text at font-size:12px
have the icon at 16x16px (80% icon size)
have the icon be coloured #54595d (or else #222 at opacity:0.75)

NOTE: @RHo - please check 1) the font size is bigger than in the specs 2) no wrapping for the page view text causes the text not to be displayed fully when the pageviews number is 6-digit number

.growthexperiments-homepage-module-suggested-edits .suggested-edits-module-wrapper .suggested-edits-card-wrapper .suggested-edits-task-card-wrapper .se-card-content .se-card-text .se-card-pageviews {
    font-size: 0.875em;
    padding-top: 10px;
    color: #54595d;
    text-overflow: ellipsis;
    overflow: hidden;
    white-space: nowrap;
Etonkovidova closed this task as Resolved.Dec 10 2019, 10:07 PM
Etonkovidova reopened this task as Open.
Etonkovidova moved this task from QA to Design Review on the Growth-Team (Current Sprint) board.

Testing today in cs.betalabs:

Example 1 (easy tasks)
Example 2 (Mixed task levels)
All except for E, F(iii) & G(iii), H(ii), J, and M(i) & M(iii) are fixed.

E. Left and right arrow icons should be 16px from left and right edges of card respectively, and sized 20x20px.
The left arrow is still not the same distance from the card due to the left and right arrow icons not being centered internally in the div:

However, this can be addressed in this separate bug task T238279.

F(ii) & G(iii) - Article text title and extract should have 16px left and right padding
Actual is showing 10px padding.

H(ii). Suggested task explanation should have padding-bottom 32px (between task type short description text and card footer section) is not done as can be seen in the following screenshot:

J. Task type short description should be font-size:12.8px/0.8em and line-height:19.2px/1.5**
Actual in betalabs is showing font-size:0.9em, and with no line-height specified.

M(i) Pageviews text should have font-size:12px / 0.75em; and (iii) have icon coloured #54595d or else #222 @ opacity:0.75.
In betalabs it is showing the font-size as 0.8em.

.
For the icon, the opacity should actually be 0.65 to get it closer to expected #54595d colour, since the original icon colour is black and not #222 as I thought.

@RHo for the issues noted in T238280#5740052 can you please clarify if you want all of these modifications for both desktop and mobile, or just mobile?

RHo added a comment.Dec 16 2019, 10:30 PM

@RHo for the issues noted in T238280#5740052 can you please clarify if you want all of these modifications for both desktop and mobile, or just mobile?

hi @kostajh - sure, all except for E & M(iii) are for mobile only. I think E can be tackled on that separate task T238279 though for both Mobile and Desktop.

Change 558478 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Mobile card padding [F(ii) & G(iii)]

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

Change 558487 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Fix padding for explanation wrapper [H(ii)]

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

Change 558488 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Line-height and font size on short desc [J]

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

OK, patches are up for everything except for E, which we can deal with in T238279

Change 558490 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Pageviews text & Icon [M(i) & M(iii)]

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

Change 558478 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits: Mobile card padding [F(ii) & G(iii)]

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

Change 558487 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Fix padding for explanation wrapper [H(ii)]

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

Change 558488 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Line-height and font size on short desc [J]

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

Change 558490 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Suggested Edits Mobile: Pageviews text & Icon [M(i) & M(iii)]

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

RHo added a comment.Dec 20 2019, 7:31 PM

Hi @Etonkovidova - fyi there seems to be some regression on some items previously fixed in this task as of today (picked up while reviewing T236854), not sure if it is just me or temporary:

  • height of card
  • height of image
  • position of page views text

Hi @Etonkovidova - fyi there seems to be some regression on some items previously fixed in this task as of today (picked up while reviewing T236854), not sure if it is just me or temporary:

  • height of card
  • height of image
  • position of page views text

Thanks, I haven't tested this task yet with the most recent patches. Can such regression be caused by some of the patches above? Pinging @kostajh

Tgr added a comment.Dec 21 2019, 4:06 AM

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/559586 changed those, but only minimally. They do scale with the font size now, though, so maybe something affects that.

Returning back to Ready for Development:

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/559586 changed those, but only minimally. They do scale with the font size now, though, so maybe something affects that.

Agree that the patch should not affect much, but the pageview info looks off:

productionbetalabs

Also, it looks like the image height was recalculated to 144 (9em x 16 px (as a base) instead of the intended base).

Tgr added a comment.Jan 4 2020, 12:35 AM

Right, on mobile the title font size is 1em and the text font size is 0.8em and for desktop the title font size is 1.2em and the font size is 0.875em so the box size in em will have to be different. Having all those small styling differences between mobile and desktop is not great for maintainability... I'll add a different height for mobile for now, but at some point we should consider if the minor visual difference (text size being 73% vs 80% of title size) is worth maintaining separate CSS size info for mobile.

Change 561940 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] Fix task card CSS height calculation

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

Change 561940 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Fix task card CSS height calculation

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

The two issues mentioned in my comment - https://phabricator.wikimedia.org/T238280#5772559 - have been corrected (checked in cswiki betalabs). Moving to Design Review.

Thanks, most points in the description are resolved and the outstanding/regression items all have tasks already:

  • C - Created T244210 to address about the image width
  • E - see T238279 - arrows different distances from card
  • M - see T243307 - regression from previous fix (the info icon is too big again)
MMiller_WMF closed this task as Resolved.Feb 12 2020, 7:07 PM

Thank you!