Page MenuHomePhabricator

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

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
  1. The gray cells should not have borders on the right and left
currentcorrect

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

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterThere is no reason for notification overlay header to be static
mediawiki/skins/MinervaNeue : masterAdjustments to Notification tray margins and borders

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 6 2019, 10:43 PM

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

phuedx triaged this task as Normal 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 added a subscriber: phuedx.

… gIven that the bugs are minor.

phuedx updated the task description. (Show Details)Jun 7 2019, 7:32 PM
alexhollender updated the task description. (Show Details)Jun 7 2019, 7:37 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 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

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

Jdlrobson added a subscriber: Jdlrobson.

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.

ovasileva set the point value for this task to 2.Jul 16 2019, 4:49 PM

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

nray assigned this task to Edtadros.Jul 18 2019, 6:40 PM
nray updated the task description. (Show Details)

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

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

@Jdlrobson where can I see this change?

@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

l

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


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

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

@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

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

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

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

alexhollender closed this task as Resolved.Jul 31 2019, 12:56 PM
alexhollender removed alexhollender as the assignee of this task.
alexhollender updated the task description. (Show Details)