Page MenuHomePhabricator

[Leftovers] Newcomer homepage: Visual design improvements to the Suggested edits module on Desktop
Closed, ResolvedPublic

Description

Through the course of implementing var C/D, it is observed that the Suggested edits module layout required some further visual design refinements not well-specified/translated from mocks to the original task description of T258704 written by me (@RHo).


Issue A. Article card and image display

The panoramic aspect ratio is not ideal for the article image, often leading to top-and-bottom cropped images or repeated-y images.
This is illustrated in the following comparison table.

VersionImage dimensions (Width x Height)/AspectRatio (W/H)Example 1Example 2
Var C/D implemented from T258704 (according to inaccurate css on the task description at odds with the Zeplin mock.)368x160px2.3
Var C/D mockups332x160px2.075
Var A/B card260x160px1.625
HD aspect ratio16 : 9~1.78
(320x180px)
(320x180px)

Proposed solution:
(i) Update the card image dimensions to width:332px and height:188px (for close to the 16:9 aspect ratio)
(ii) This also means updating the total card width according, done by adjusting the class .suggested-edits-task-card-wrapper to have a width:332px.
(iii) The task explanation below the card also requires adjusting the class .suggested-edits-task-explanation-wrapper width so that the text aligns with the text inside the card... which can be done by adding 8px to padding left and right on this class.
(iv) The task explanation (class .suggested-edits-task-explanation-wrapper) should also have reduced top and bottom padding, reducing from 24px padding top and bottom to 16px
(v) The space between the suggested edits pager and the top of the card is reduced to 8px. This is achieved by changing the margin-bottom on class .suggested-edits-pager from margin:11px to margin;8px, and removing the padding-top:5px from the class .suggested-edits-card-wrapper
(vi) Add a box-shadow:inset 0px 0px 2px #c8ccd1 on the image (class .se-card-image) for so that there is an boundary for when there is an image with white edges:

Current
Proposed with inset box-shadow

Note: changes proposed here may impact the need for T238598


Issue B. Suggested edit module footer

Having the border set to transparent makes the footer appear to be 'floating' below the module.

Current
Proposed change

Add a border with 2px radius to the SE footer {border: solid 2px rgba(234,243,255,.5); border-radius:0 0 2px 2px}


Issue C. Suggested edit module header

The addition of the header info icon appears to have added unintended padding around the SE module header on the top, bottom and right.This is noticeable when compared with the Zeplin mocks:

Header Top paddingSE module header with unexpected extra space over the 16px top padding
Expected header text top position with top padding
Info icon right padding
Expected RHS padding 16px
Bottom padding (actual)
Expected

Proposed fixes: Adjust how the info icon is being placed inside the module so that the padding for both header text and info icon is reduced to the expected.

Event Timeline

RHo created this task.Oct 5 2020, 3:57 PM
RHo updated the task description. (Show Details)Oct 5 2020, 10:01 PM
RHo updated the task description. (Show Details)Oct 6 2020, 10:17 AM
RHo updated the task description. (Show Details)
RHo updated the task description. (Show Details)Oct 7 2020, 1:06 PM
RHo updated the task description. (Show Details)Oct 7 2020, 3:50 PM

Change 634127 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Fix header icon size and padding

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

Catrope added a subscriber: Catrope.

Change 634127 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Fix header icon size and padding

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

This fixes issue C

Catrope claimed this task.Oct 14 2020, 11:54 PM

Change 634130 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Add border to footer

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

Change 634138 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Change image size, styling tweaks

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

Change 634138 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Change image size, styling tweaks

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

This patch fixes issue A, with the following caveats:

(i) Update the card image dimensions to width:332px and height:188px (for close to the 16:9 aspect ratio)
(ii) This also means updating the total card width according, done by adjusting the class .suggested-edits-task-card-wrapper to have a width:332px.

Done, on desktop. But what should happen on mobile? Currently mobile uses 260x128 for the image dimensions, should that change to something else?

(iii) The task explanation below the card also requires adjusting the class .suggested-edits-task-explanation-wrapper to have width:316px;

That's not what I see currently: task-explanation-wrapper has the same width as task-card-wrapper, and because it uses the same LESS variable, changing the image dimensions caused it to automatically adjust its width to 332px to match. For consistency, I'm going with that for now, unless you explicitly say that you want these widths to be different.

RHo updated the task description. (Show Details)Oct 15 2020, 9:39 AM
RHo added a comment.Oct 15 2020, 9:42 AM

Change 634138 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Change image size, styling tweaks

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

This patch fixes issue A, with the following caveats:

(i) Update the card image dimensions to width:332px and height:188px (for close to the 16:9 aspect ratio)
(ii) This also means updating the total card width according, done by adjusting the class .suggested-edits-task-card-wrapper to have a width:332px.

Done, on desktop. But what should happen on mobile? Currently mobile uses 260x128 for the image dimensions, should that change to something else?

No, we are keeping mobile as is (since we haven't made the same layout changes to mobile where there is just the SE module).

(iii) The task explanation below the card also requires adjusting the class .suggested-edits-task-explanation-wrapper to have width:316px;

That's not what I see currently: task-explanation-wrapper has the same width as task-card-wrapper, and because it uses the same LESS variable, changing the image dimensions caused it to automatically adjust its width to 332px to match. For consistency, I'm going with that for now, unless you explicitly say that you want these widths to be different.

The important thing for me is that the text in the explanation wrapper aligns with the title and extract text in the article card, but we can achieve this by adding left and right padding instead of changing the width if that is better? I've updated the description to do this instead.

(ii) This also means updating the total card width according, done by adjusting the class .suggested-edits-task-card-wrapper to have a width:332px.

Done, on desktop. But what should happen on mobile? Currently mobile uses 260x128 for the image dimensions, should that change to something else?

No, we are keeping mobile as is (since we haven't made the same layout changes to mobile where there is just the SE module).

Great, that makes it easy.

(iii) The task explanation below the card also requires adjusting the class .suggested-edits-task-explanation-wrapper to have width:316px;

That's not what I see currently: task-explanation-wrapper has the same width as task-card-wrapper, and because it uses the same LESS variable, changing the image dimensions caused it to automatically adjust its width to 332px to match. For consistency, I'm going with that for now, unless you explicitly say that you want these widths to be different.

The important thing for me is that the text in the explanation wrapper aligns with the title and extract text in the article card, but we can achieve this by adding left and right padding instead of changing the width if that is better? I've updated the description to do this instead.

Gotcha. I've updated the patch to add that padding (and set border-box, otherwise it didn't take)

Change 634127 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Fix header icon size and padding

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

Change 634130 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Add border to footer

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

Change 634138 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SuggestedEdits: Change image size, styling tweaks

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

Etonkovidova added a subscriber: Etonkovidova.EditedOct 24 2020, 12:24 AM

For design review - all specs are in place for Issue A

Issue A. Article card and image display
(i) Update the card image dimensions to width:332px and height:188px (for close to the 16:9 aspect ratio).
(ii) This also means updating the total card width according, done by adjusting the class .suggested-edits-task-card-wrapper to have a width:332px.
.growthexperiments-homepage-module-suggested-edits .suggested-edits-module-wrapper .suggested-edits-card-wrapper .suggested-edits-task-card-wrapper {
    box-shadow: 0 5px 10px 0 rgba(0,0,0,0.15);
    background-color: #ffffff;
    width: 332px;
    padding: 8px;
    border-radius: 2px;
}
(iii) The task explanation below the card also requires adjusting the class .suggested-edits-task-explanation-wrapper width so that the text aligns with the text inside the card... which can be done by adding 8px to padding left and right on this class.
(iv) The task explanation (class .suggested-edits-task-explanation-wrapper) should also have reduced top and bottom padding, reducing from 24px padding top and bottom to 16px
.suggested-edits-task-explanation-wrapper {
    padding: 16px 8px;
    width: 332px;
    -webkit-box-sizing: border-box;
    -moz-box-sizing: border-box;
    box-sizing: border-box;
}

(v) The space between the suggested edits pager and the top of the card is reduced to 8px. This is achieved by changing the margin-bottom on class .suggested-edits-pager from margin:11px to margin;8px, and removing the padding-top:5px from the class .suggested-edits-card-wrapper
.growthexperiments-homepage-module-suggested-edits .suggested-edits-module-wrapper .suggested-edits-pager {
    margin-top: 24px;
    margin-bottom: 8px;
    font-size: 0.875em;
    color: #202122;
}
(vi) Add a box-shadow:inset 0px 0px 2px #c8ccd1 on the image (class .se-card-image) for so that there is an boundary for when there is an image with white edges:
growthexperiments-homepage-module-suggested-edits .suggested-edits-module-wrapper .suggested-edits-card-wrapper .suggested-edits-task-card-wrapper .se-card-content .se-card-image {
    height: 188px;
    width: 332px;
    -webkit-box-shadow: inset 0 0 2px #c8ccd1;
    box-shadow: inset 0 0 2px #c8ccd1;
    background-position: center 25%;
    background-repeat: repeat-x;
{

It's already in production - wmf.14.

Etonkovidova updated the task description. (Show Details)Oct 24 2020, 3:01 AM

For Design review - issue B is already in production

Issue B. Suggested edit module footer
Add a border with 2px radius to the SE footer {border: solid 2px rgba(234,243,255,.5); border-radius:0 0 2px 2px}
.growthexperiments-homepage-module-suggested-edits .growthexperiments-homepage-module-footer {
    margin: 16px -16px -16px -16px;
    padding: 12px 24px 12px;
    border: 2px solid rgba(234,243,255,0.5);
    background-color: #ffffff;
    color: #54595d;
    font-size: 0.9em;
}

For design review - the info icon/header text padding have been cleaned up.

Issue C. Suggested edit module header
Proposed fixes: Adjust how the info icon is being placed inside the module so that the padding for both header text and info icon is reduced to the expected.
Header Top paddingPresent in betalabs
SE module header with unexpected extra space over the 16px top padding
Expected header text top position with top padding
Info icon right paddingPresent in betalabs
Expected RHS padding 16px
Bottom padding (actual)Present in betalabs
Expected

Beautiful *chef's kiss*. Thanks all!

Issue A - card and SE module visual design


Issue B - footer

Issue C - header spacing with info icon

MMiller_WMF closed this task as Resolved.Oct 29 2020, 10:15 PM

Thank you!