Use ResourceLoaderImageModule in the Popups extension for settings icon (not mwe-popups-settings)
Closed, ResolvedPublic1 Estimated Story Points
Actions

Assigned To

Authored By

	• bmansurov
	Apr 28 2016, 10:11 PM

Description

The result of the spike at T131314#2250027 (Review Icon support to see if it will benefit from ResourceLoaderImageModule):

The extension uses only two icons - cog and horn.

The benefits over the current implementation are:

No need to keep the generated PNGs around, RL will generate them on the fly;
Standard mw-ui-icon, rather than the current custom styled icon.
Future potential benefits are:

Specify different images for RTL and LTR languages if we want to do so;
Multi-colored images based on the single SVG.

Acceptance criteria:

Switch the cog icon implementations to use ResourceLoaderImageModule and remove the css rule for .mwe-popups-settings
Remove the unnecessary png.

Test Plan

Enable Page Previews
Visit any page on the Beta Cluster (https://en.wikipedia.beta.wmflabs.org/wiki/Special:Random)
Dwell on a link for long enough that a preview appears
Observe that the settings cog is visible
Tap the settings cog and get taken to the preferences page (or the settings modal if you're logged out)

Details

	Subject	Repo	Branch	Lines +/-
	Generate cog icon via ResourceLoaderImage module	mediawiki/extensions/Popups	master	+21 -25

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Declined	Jdlrobson	T111329 [GOAL] Page previews on mobileweb
Resolved	Jdlrobson	T144522 [EPIC] Hovercards technical debt and quality enhancements
Resolved	phuedx	T133956 Use ResourceLoaderImageModule in the Popups extension for settings icon (not mwe-popups-settings)

Event Timeline

• bmansurov created this task.Apr 28 2016, 10:11 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 28 2016, 10:11 PM

• bmansurov updated the task description. (Show Details)Apr 28 2016, 10:12 PM

phuedx mentioned this in T131314: Spike: Audit HoverCards JavaScript real and perceived performance.Apr 29 2016, 9:56 AM

Jdlrobson triaged this task as Low priority.May 10 2016, 5:16 PM

Jdlrobson added a parent task: T111329: [GOAL] Page previews on mobileweb.

MBinder_WMF added a project: Web-Team-Backlog.May 17 2016, 6:31 PM

• Jhernandez added a project: Technical-Debt.May 18 2016, 3:29 PM

• Jhernandez moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.

MBinder_WMF added a project: Reading-Web-Tech-Debt.Oct 3 2016, 8:34 PM

MBinder_WMF edited projects, added Technical-Debt (RW-Tech-Debt); removed Technical-Debt.Oct 4 2016, 6:15 PM

MBinder_WMF removed a project: Reading-Web-Tech-Debt.Oct 4 2016, 7:53 PM

• bmansurov moved this task from Uncategorized to Improvements on the Technical-Debt (RW-Tech-Debt) board.Oct 5 2016, 10:43 PM

ovasileva mentioned this in T128965: Popups should use ResourceLoaderImageModule instead of .background-image-svg().Oct 12 2016, 6:44 PM

ovasileva added a parent task: T144522: [EPIC] Hovercards technical debt and quality enhancements.Oct 21 2016, 9:23 AM

ovasileva moved this task from Backlog to Needs Analysis on the Page-Previews board.Dec 20 2016, 12:37 AM

We are using ResourceLoaderImage module for the close icon but not for the settings cog. This should be easy to fix and I recommend we do that soon so we don't forget.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptFeb 15 2017, 10:14 PM

ovasileva moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Mar 6 2017, 4:20 PM

ovasileva added a project: Reading-Web-Sprint-94.

Jdlrobson changed the point value for this task from 0.5 to 1.Mar 8 2017, 5:36 PM

Jdlrobson moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-94 board.

ovasileva moved this task from To Do to Needs Analysis on the Reading-Web-Sprint-94 board.Mar 15 2017, 4:56 PM

ovasileva removed a project: Reading-Web-Sprint-94.Mar 15 2017, 5:58 PM

ovasileva added a project: Reading-Web-Sprint-96.Apr 13 2017, 6:46 PM

Jdlrobson moved this task from Needs Analysis to To Do on the Reading-Web-Sprint-96 board.Apr 19 2017, 5:36 PM

Jdlrobson moved this task from To Do to Doing on the Reading-Web-Sprint-96 board.Apr 20 2017, 9:54 PM

Change 349344 had a related patch set uploaded (by Jdlrobson):
[mediawiki/extensions/Popups@master] Generate cog icon via ResourceLoaderImage module

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

gerritbot added a project: Patch-For-Review.Apr 20 2017, 10:25 PM

Jdlrobson moved this task from Doing to Needs Code Review on the Reading-Web-Sprint-96 board.Apr 20 2017, 10:25 PM

Jdlrobson claimed this task.Apr 20 2017, 11:35 PM

During testing, I found that rEPOP9e48696cb8a0: Generate cog icon via ResourceLoaderImage module breaks the generic preview across all browsers, e.g. in IE9 on Windows 7 I see:

Screen Shot 2017-04-21 at 10.29.32.png (206×338 px, 16 KB)

The settings cog, however, works as expected.

phuedx moved this task from Needs Code Review to Needs More Work on the Reading-Web-Sprint-96 board.Apr 21 2017, 9:37 AM

ovasileva moved this task from Upcoming to 2016-17 Q4 on the Web-Team-Backlog board.Apr 21 2017, 3:53 PM

So far this hasn't proved as easy as I first hoped. Seems there's a lot of CSS depending on the size of that icon being a certain way. I'll try to unravel what's going on and make sure the icon still gets placed in the correct place.

New patch is up and I'm hopeful it should remove the bugs associated with the last. If there's any issues with how it's arranged let's go through them Monday and iron them out in a pair programming session.

Quoting my review:

This LGTM. I've tested the change by triggering generic and normal previews in the following browsers:

macOS Sierra (10.12.3) – Chrome (58.0.3029.81)

macOS Sierra (10.12.3) – Safari (10.0.3)

macOS Sierra (10.12.3) – Firefox (53.0)

Windows 7 – IE9

Windows 10 – IE11

Windows 10 – Edge (latest)

/cc @ABorbaWMF

Change 349344 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Generate cog icon via ResourceLoaderImage module

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

ReleaseTaggerBot added a project: MW-1.29-release (WMF-deploy-2017-04-25_(1.29.0-wmf.21)).Apr 24 2017, 10:00 AM

phuedx moved this task from Needs Code Review to Needs QA on the Reading-Web-Sprint-96 board.Apr 24 2017, 10:39 AM

phuedx updated the task description. (Show Details)

phuedx reassigned this task from Jdlrobson to ABorbaWMF.Apr 24 2017, 2:40 PM

I am still seeing the problem on beta when hovering over link with a missing preview. I tried clearing the cache. Will try again tomorrow.

@ABorbaWMF: That's per the design. "Generic previews", previews that appear after dwelling on a link that can't be previewed, don't show the settings cog.

phuedx updated the task description. (Show Details)Apr 25 2017, 4:04 PM

Tested on https://en.wikipedia.beta.wmflabs.org

Tried a number of system and browser combinations. Appears to be fixed.

phuedx claimed this task.Apr 26 2017, 9:40 AM

phuedx updated the task description. (Show Details)Apr 26 2017, 9:48 AM

I'll be the signer offerer for PP tasks (that I haven't submitted changes for) while @ovasileva is OOO.

Per T133956#3211694, this is 👍

	F7682867: Screen Shot 2017-04-21 at 10.29.32.png
	Apr 21 2017, 9:36 AM

	F7745579: image.png
	Apr 25 2017, 12:39 AM

Use ResourceLoaderImageModule in the Popups extension for settings icon (not mwe-popups-settings)Closed, ResolvedPublic1 Estimated Story PointsActions