blurred talk icon echo notifications
Closed, ResolvedPublic

Description

extensions/Flow/modules/notification/icon/Talk.png is 30x30 px, but the css of the icon is defined as follows:

.mw-echo-ui-notificationItemWidget-icon img {

height: 2.5em;
width: 2.5em;

}

this cause blurred icons in the production (enwiki/hewiki etc).

eranroz created this task.Jan 14 2016, 7:35 PM
eranroz updated the task description. (Show Details)
eranroz raised the priority of this task from to Needs Triage.
eranroz added a subscriber: eranroz.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptJan 14 2016, 7:35 PM
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald Transcript

That specific icon has been changed to an SVG, so that will hopefully render less blurrily in wmf11. However, we still have some PNG notification icons, and IIRC the SVGs also have a viewbox of 30x30px.

@Pginer-WMF: thoughts? Should we change the size of these icons from 2.5x2.5em (computes to 35x35px) to 30x30px?

That specific icon has been changed to an SVG, so that will hopefully render less blurrily in wmf11. However, we still have some PNG notification icons, and IIRC the SVGs also have a viewbox of 30x30px.

@Pginer-WMF: thoughts? Should we change the size of these icons from 2.5x2.5em (computes to 35x35px) to 30x30px?

I took the current icons as the reference for size, so using that size (30px) on the UI seems good to me. In that way we can avoid scaling-up graphics which adds distortion to those without SVG support.
We need to take into account that adjusting the icon size requires also to adjust the left padding on the notification content (which is there to leave room for the icon).

There is also a broader issue about the computing of ems to pixels, which is affected by the element nesting, making it problematic to anticipate how it will be translated into pixels.

eranroz added a comment.EditedJan 15 2016, 6:14 AM

We can avoid computing the padding of the content by using right-margin on the icon div (or left margin on the content div) + float instead of position:absolute

MtDu claimed this task.Jan 17 2016, 5:37 AM
MtDu added a subscriber: MtDu.

I'll take care of this quickly.

Change 264564 had a related patch set uploaded (by MtDu):
Don't blur talk icon echo notifications

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

MtDu added a comment.Jan 17 2016, 5:55 AM

There is also a broader issue about the computing of ems to pixels, which is affected by the element nesting, making it problematic to anticipate how it will be translated into pixels.

@Pginer-WMF
Should we create a bug for this?
Thanks,
MtDu

eranroz added a comment.EditedJan 17 2016, 5:59 AM

MtDu - thank you for the quick fix. It is no-doubt better than the current status.
However I think this creates a bit too large padding so we may want to also
.mw-echo-ui-notificationItemWidget-content { padding-right: 3.5em => 30px; }

MtDu added a comment.Jan 17 2016, 1:21 PM

@eranroz,
I don't see a padding right of 3.5 em. Do you mean the padding left in the file mw.echo.ui.NotificationItemWidget.less?
Thanks,
MtDu

MtDu added a comment.Jan 17 2016, 1:25 PM

@eranroz,
It seems like it is padding-left, according to Pginer's comment above. Will push a patch with that. Thanks for the reminder!
MtDu

@Pginer-WMF: The patch at https://gerrit.wikimedia.org/r/264564 looks good to me, but it reduces the space between the icon and the text from 1em (=14px) to 5px. Is that something you're happy with.

@Pginer-WMF: The patch at https://gerrit.wikimedia.org/r/264564 looks good to me, but it reduces the space between the icon and the text from 1em (=14px) to 5px. Is that something you're happy with.

Looking at this again, T122646: Adjust layout for the new notification panel designs suggests there should be equal spacing either side, so that suggests it should be 0.5em (=7px), equal to the padding between the icon and the left edge.

@Pginer-WMF: The patch at https://gerrit.wikimedia.org/r/264564 looks good to me, but it reduces the space between the icon and the text from 1em (=14px) to 5px. Is that something you're happy with.

Looking at this again, T122646: Adjust layout for the new notification panel designs suggests there should be equal spacing either side, so that suggests it should be 0.5em (=7px), equal to the padding between the icon and the left edge.

Actually, sorry, that task suggests that that padding be increased to 0.8em,

I decided to keep the status quo in this patch and just change the icon size. The patch for T122646: Adjust layout for the new notification panel designs can then center the icon and change it to 0.8em, when that one lands.

Change 264564 merged by jenkins-bot:
Don't blur talk icon echo notifications

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

MtDu added a comment.Jan 19 2016, 9:02 PM

Closing as resolved.

MtDu closed this task as Resolved.Jan 19 2016, 9:02 PM
Catrope reopened this task as Open.Jan 19 2016, 9:32 PM

In betalabs there is /static/master/extensions/Echo/modules/icons/chat.svg

.mw-echo-ui-notificationItemWidget-icon img {

height: 30px;
width: 30px;

}

For cross-wiki notifications mw-echo-ui-notificationItemWidget-icon img has 21x21px.

Catrope closed this task as Resolved.Jan 20 2016, 11:58 PM