Page MenuHomePhabricator

Talk icon behaviour in beta different to stable. In stable it isn't loaded in non-RL-capable UAs
Closed, ResolvedPublic2 Story Points

Description

Background
From T142594 (by @jhobs):

Repro steps:

For those who like screenshots:


Acceptance Criteria

  • Enable everywhere (for logged in users), remove icon from button
  • Do not feature flag

Test plan

  • Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama and login.
  • Scroll down the page and and look for the "Discussion" button. Make sure that the button contains only the word "Discussion" and no icons (as seen in this screenshot:
    )
  • Repeat the above steps when in beta mode.

Event Timeline

phuedx created this task.Aug 15 2016, 10:38 AM
Restricted Application added subscribers: TerraCodes, Aklapper. ยท View Herald TranscriptAug 15 2016, 10:38 AM

Not that it matters, but T142594 is not by me.

phuedx updated the task description. (Show Details)Aug 15 2016, 4:05 PM
phuedx added a subscriber: โ€ข jhobs.

Yup. It needs to be bundled up with the edit icon and watchstar, but note any icon loaded without JS does impact first paint.

Aren't those icons in stable, and this one in beta?

Jhernandez triaged this task as Normal priority.Aug 17 2016, 3:18 PM
Jhernandez moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson added a subscriber: Nirzar.

Given the height difference do we even want this icon here @Nirzar ?

Change 315541 had a related patch set uploaded (by Jdlrobson):
Load skins.minerva.icons.images.variants in stable

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

Jdlrobson renamed this task from Talk icon isn't loaded in non-RL-capable UAs to Talk icon behaviour in beta different to stable. In stable it isn't loaded in non-RL-capable UAs.Oct 14 2016, 4:29 PM
phuedx added subscribers: ovasileva, MBinder_WMF.

Since I've just reviewed rEMFR7b68671ccdec: Load skins.minerva.icons.images.variants in stable, I feel like this should be brought into the sprint for visibility /cc @ovasileva @MBinder_WMF

phuedx assigned this task to Jdlrobson.Oct 17 2016, 9:59 AM
phuedx moved this task from Needs Analysis to Needs More Work on the Reading-Web-Sprint-83-Y? board.

(although I should note I'm not planning to complete this work this sprint due to still waiting on @Nirzar for an answer to https://phabricator.wikimedia.org/T142976#2584080)

@Nirzar I also notice that we show a blue icon on the user page but a gray icon on a page in the main namespace. Should these be different colours?

Jdlrobson reassigned this task from Jdlrobson to Nirzar.Oct 18 2016, 12:06 AM

user page is more of a link . and this one is a button.

actually i don't mind getting rid of the icon if it makes it easier. we removed the language button anyways.

Both are just as easy - it simply depends what you prefer :)

Change 315541 abandoned by Jdlrobson:
Load skins.minerva.icons.images.variants in stable

Reason:
To be worked on later...

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

ovasileva updated the task description. (Show Details)Nov 11 2016, 4:40 PM
ovasileva moved this task from Design to Upcoming on the Readers-Web-Backlog board.
ovasileva updated the task description. (Show Details)Nov 15 2016, 5:12 PM
ovasileva removed a project: Patch-For-Review.
ovasileva updated the task description. (Show Details)Nov 15 2016, 5:18 PM
ovasileva set the point value for this task to 2.Nov 15 2016, 5:20 PM

Change 321692 had a related patch set uploaded (by Jdlrobson):
Don't show icon on talk button

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

bmansurov updated the task description. (Show Details)

Change 321692 merged by jenkins-bot:
Don't show icon on talk button

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

I have consistently seen no icon on the "Discussion" button when testing the following environments:

Samsung Galaxy Tab 4 stable


Samsung Galaxy Tab 4 Beta mode

iPad Air 2 (iOS 10.2) stable

iPad Air 2 (iOS 10.2) Beta mode

Thus, this issue appears to be fixed.

ovasileva closed this task as Resolved.Nov 28 2016, 11:56 AM