Page MenuHomePhabricator
Create Task
Maniphest T225260

BUG: Notifications tray (mobile) - margins and borders
Closed, ResolvedPublic2 Estimated Story Points
Actions

Assigned To
None
Authored By
• alexhollender_WMF
Jun 6 2019, 10:43 PM
Tags
Referenced Files
F29899096: image.png
Jul 29 2019, 5:46 PM
F29899094: image.png
Jul 29 2019, 5:46 PM
F29878738: Screenshot 2019-07-26 at 11.57.05 AM.png
Jul 29 2019, 5:28 PM
F29878613: Screenshot 2019-07-26 at 11.40.42 AM.png
Jul 29 2019, 5:28 PM
F29874839: image.png
Jul 26 2019, 1:11 AM
F29458301: en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8) (1).png
Jun 11 2019, 4:11 PM
F29458179: en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8).png
Jun 11 2019, 4:11 PM
F29379181: en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8).png
Jun 7 2019, 7:37 PM
View All 13 Files
Subscribers
Aklapper
• alexhollender_WMF
Edtadros
Jdlrobson
phuedx

Description

Description

There are several minor issues with the notifications tray on mobile.

  1. There is a 4px gap between the top border and the first gray cell
image.png (1×750 px, 195 KB)
en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8).png (1×750 px, 71 KB)
  1. The gray cells should not have borders on the right and left
currentcorrect
en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8) (1).png (1×750 px, 170 KB)
en.m.wikipedia.org_wiki_Main_Page(iPhone 6_7_8) (2).png (1×750 px, 170 KB)

Developer notes

  • Points 1 and 2 above can be achieved by:
    1. Removing margin-top: 1px from .notifications-overlay .overlay-content and accounting for the 1px border (e.g. using margin-top: -1px;}
    2. Adding border-left-with: 0; border-right-width: 0; to .notifications-overlay .mw-echo-ui-notificationItemWidget

See also: T227624, T226125

QA instructions

  • While logged in, visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain. Click on the notifications button in the header (will be either a bell icon or a circle with a number). When the notifications drawer opens and assuming you have > 0 notifications (if not register as a new user and you will have a notification), ensure that the gap (depicted in the task description) between the notifications header and first item is not there anymore. Also ensure that there are no borders on both sides of each item.

Details

SubjectRepoBranchLines +/-
There is no reason for notification overlay header to be staticmediawiki/extensions/MobileFrontendmaster+0 -6
Adjustments to Notification tray margins and bordersmediawiki/skins/MinervaNeuemaster+5 -0
Customize query in gerrit

Related Objects

Event Timeline

• alexhollender_WMF created this task.Jun 6 2019, 10:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 6 2019, 10:43 PM
Aklapper added a comment.Jun 7 2019, 10:33 AM

@alexhollender: Which codebase is this task about? MobileFrontend? Notifications? Could you please add a project tag? Thanks!

• alexhollender_WMF added a project: MobileFrontend.Jun 7 2019, 2:07 PM
phuedx triaged this task as Medium priority.Jun 7 2019, 7:26 PM
phuedx updated the task description. (Show Details)
phuedx moved this task from Needs triage to Triaged on the Mobile board.
phuedx moved this task from Triaged to MobileFrontend specific on the Mobile board.
phuedx subscribed.

… gIven that the bugs are minor.

phuedx updated the task description. (Show Details)Jun 7 2019, 7:32 PM
• alexhollender_WMF updated the task description. (Show Details)Jun 7 2019, 7:37 PM
• alexhollender_WMF updated the task description. (Show Details)Jun 11 2019, 4:11 PM
• alexhollender_WMF mentioned this in T225535: Notifications tray (mobile) - design refinements.Jun 19 2019, 4:20 PM
Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.Jul 2 2019, 5:18 PM
Jdlrobson moved this task from Needs Prioritization to Incoming on the Web-Team-Backlog board.
Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.Jul 8 2019, 8:58 PM
Jdlrobson renamed this task from Notifications tray (mobile) - minor bugs to BUG: Notifications tray (mobile) - margins and borders.Jul 9 2019, 10:02 PM
Jdlrobson mentioned this in T227624: BUG: non-circular read/unread dots in Echo notifications.
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Jul 9 2019, 10:04 PM
gerritbot added a comment.Jul 9 2019, 10:05 PM

Change 521777 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Adjustments to Notification tray margins and borders

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

gerritbot added a project: Patch-For-Review.Jul 9 2019, 10:05 PM
Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.Jul 9 2019, 10:05 PM
Jdlrobson subscribed.

While analysing this, I split out several subtasks where it is more appropriate to fix. I wrote a patch as the remaining work involved was trivial.

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Jul 9 2019, 10:06 PM
ovasileva set the point value for this task to 2.Jul 16 2019, 4:49 PM
Jdlrobson moved this task from Upcoming to Readers-Web-Kanbanana-2019-20-Q1 on the Web-Team-Backlog board.Jul 16 2019, 4:50 PM
Jdlrobson edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1); removed Web-Team-Backlog.
Jdlrobson moved this task from Needs Analysis to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
• nray moved this task from Needs Code Review to Needs More Work on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Jul 16 2019, 11:11 PM
Jdlrobson added a comment.Jul 17 2019, 5:20 PM

So it turns out the fix to T195795 is also the fix to this (so I've pulled that one into the sprint)

Jdlrobson mentioned this in T195795: Use a clearly intended height on MinervaNeue header.Jul 17 2019, 5:20 PM
Jdlrobson moved this task from Needs More Work to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Jul 17 2019, 5:34 PM
• nray assigned this task to Edtadros.Jul 18 2019, 6:40 PM
• nray updated the task description. (Show Details)
• nray moved this task from Needs Code Review to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
gerritbot added a comment.Jul 18 2019, 10:34 PM

Change 521777 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Adjustments to Notification tray margins and borders

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

Restricted Repository Identity mentioned this in rSKIN48cf91ba7b6d: Update git submodules.Jul 18 2019, 10:37 PM
ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.15; 2019-07-23).Jul 18 2019, 11:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jul 18 2019, 11:10 PM
Jdlrobson reassigned this task from Edtadros to • alexhollender_WMF.Jul 24 2019, 5:12 PM
Jdlrobson moved this task from Needs QA to Needs Design Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
Jdlrobson added a subscriber: Edtadros.
• alexhollender_WMF added a comment.Jul 25 2019, 3:06 AM

@Jdlrobson where can I see this change?

Jdlrobson added a comment.Jul 25 2019, 4:15 PM

Beta cluster! https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain

• alexhollender_WMF added a comment.Jul 26 2019, 1:11 AM

@Jdlrobson looks like the borders are gone. The gap is now 1px instead of 4px. It seems like it's coming from the margin-top on .notifications-overlay .overlay-content

image.png (724×937 px, 262 KB)

Jdlrobson added a comment.Jul 29 2019, 5:28 PM

l

In T225260#5367720, @alexhollender wrote:

@Jdlrobson looks like the borders are gone.

Just to confirm, you are happy with this outcome, correct?

The gap is now 1px instead of 4px. It seems like it's coming from the margin-top on .notifications-overlay .overlay-content

The negative margin top was intentional and is to allow the border to display but I don't consider it a gap (but maybe my understanding of what you mean by a gap here is the problem).
Removing it results in this:

Screenshot 2019-07-26 at 11.40.42 AM.png (210×1 px, 41 KB)

The header is 54px height, with a bottom border of 1px.
The margin-top accounts for the 1px border at the bottom of overlay-content to make sure the content sits underneath the border.

Note when a row is coloured (e.g. hovered) there is a 1px border around the row in addition to the background.

This rule might help visualise:

.mw-echo-ui-notificationItemWidget-unread:hover {
    background-color: red;
}

Screenshot 2019-07-26 at 11.57.05 AM.png (387×996 px, 67 KB)

I think this will require us to sync today and get on the same page.

• alexhollender_WMF added a comment.Jul 29 2019, 5:46 PM
In T225260#5373675, @Jdlrobson wrote:
In T225260#5367720, @alexhollender wrote:

@Jdlrobson looks like the borders are gone.

Just to confirm, you are happy with this outcome, correct?

correct

Regarding that gap: thanks for clarifying and sorry for the lack of specificity here. It seems that the gap is only present at screen sizes below 720px, which is when the notifications tray becomes fullscreen. Perhaps we can remove the css rule for that case?

below 720pxabove 720px
image.png (336×432 px, 39 KB)
image.png (476×510 px, 52 KB)
gerritbot added a comment.Jul 29 2019, 5:46 PM

Change 526190 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] There is no reason for notification overlay header to be static

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

gerritbot added a project: Patch-For-Review.Jul 29 2019, 5:46 PM
Jdlrobson moved this task from Needs Design Review to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Jul 29 2019, 6:52 PM

Checked with Alex and new patch takes care of the problem.

gerritbot added a comment.Jul 30 2019, 2:16 PM

Change 526190 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] There is no reason for notification overlay header to be static

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

ReleaseTaggerBot edited projects, added MW-1.34-notes (1.34.0-wmf.16; 2019-07-30); removed MW-1.34-notes (1.34.0-wmf.15; 2019-07-23).Jul 30 2019, 3:00 PM
Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Jul 30 2019, 3:07 PM
Maintenance_bot removed a project: Patch-For-Review.Jul 30 2019, 3:11 PM
• alexhollender_WMF closed this task as Resolved.Jul 31 2019, 12:56 PM
• alexhollender_WMF removed • alexhollender_WMF as the assignee of this task.
• alexhollender_WMF updated the task description. (Show Details)
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits