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:

Mobile Expected v Actual.png (862×1 px, 365 KB)

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:

image.png (596×426 px, 88 KB)

Details

SubjectRepoBranchLines +/-
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

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?

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

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):

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

or alternatively
B. Make the image background-repeat: repeat-x; to the class
image.png (1×2 px, 591 KB)

My preference is for option B.

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)

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

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;

IMG_8634.PNG (1×640 px, 73 KB)

To compare - testwiki

Screen Shot 2019-12-10 at 12.25.46 PM.png (698×416 px, 115 KB)

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

IMG_8634.PNG (1×640 px, 73 KB)

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)

Screen Shot 2019-12-10 at 12.40.46 PM.png (587×401 px, 68 KB)

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;
}

Screen Shot 2019-12-10 at 12.26.00 PM.png (762×413 px, 76 KB)

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; }

Screen Shot 2019-12-10 at 1.03.03 PM.png (639×426 px, 84 KB)

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

Screen Shot 2019-12-10 at 2.05.59 PM.png (642×392 px, 54 KB)

.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 reopened this task as Open.

Testing today in cs.betalabs:

Example 1 (easy tasks)
image.png (674×386 px, 155 KB)
Example 2 (Mixed task levels)
image.png (695×406 px, 149 KB)
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:

image.png (179×737 px, 59 KB)
image.png (141×414 px, 17 KB)

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.

image.png (311×842 px, 89 KB)

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:

image.png (137×410 px, 27 KB)

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.

image.png (294×820 px, 92 KB)

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.

image.png (269×862 px, 84 KB)
.
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 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

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:

image.png (1×744 px, 103 KB)

  • 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:

image.png (1×744 px, 103 KB)

  • 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

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
Screen Shot 2020-01-02 at 1.27.24 PM.png (607×392 px, 79 KB)
Screen Shot 2020-01-02 at 1.16.40 PM.png (644×396 px, 79 KB)

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

Screen Shot 2020-01-02 at 1.29.37 PM.png (652×362 px, 75 KB)

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)
    image.png (536×1 px, 129 KB)