Page MenuHomePhabricator

Composition experiment: DownloadIcon and ShareIcon should not extend Icon
Closed, ResolvedPublic3 Story Points

Description

The DownloadIcon and ShareIcon in Minerva extend the Icon class to override some of its options, however when rendered it looks and quacks like an Icon.js. Let's rewrite Icon.js and these files so they use composition rather than inheritance.

Acceptance criteria

  • Armed with the ability to define events in props

and experiments with factory functions we should be able to rewrite DownloadIcon as a factory functions that return instances of an Icon class.

  • The same should be possible for the ShareIcon
  • Be prepared to report back to the team on your experiences. (added to Jan. 14's super-happy-dev-time list)

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterGenerate a shareIcon via composition
mediawiki/skins/MinervaNeue : masterComposition: DownloadIcon

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 26 2018, 10:37 PM
Jdlrobson renamed this task from [spikey, ?? hrs] Composition experiment: DownloadIcon and ShareIcon should not extend Button to [spikey, ?? hrs] Composition experiment: DownloadIcon and ShareIcon should not extend Icon.Sep 26 2018, 10:37 PM
Jdlrobson moved this task from Needs Prioritization to Upcoming on the Readers-Web-Backlog board.
Jdlrobson updated the task description. (Show Details)Sep 27 2018, 3:44 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)Sep 27 2018, 3:46 PM
Jdlrobson added a subscriber: Niedzielski.

I'm not sure how to do effective layout composition with Hogan.js. I'd like to discuss what are options are, even if that ends up being just "don't consider Hogan.js," and what specifically we should explore for T205592. For reference, here's the Hoganfile for Icon.js:

<{{tagName}} class="{{base}} {{base}}-{{glyphPrefix}}-{{name}} {{modifier}} {{#isSmall}}mw-ui-icon-small{{/isSmall}} {{#_rotationClass}}{{_rotationClass}}{{/_rotationClass}} {{additionalClassNames}}"
	{{#id}}id="{{id}}"{{/id}}
	{{#href}}href="{{href}}"{{/href}}
	{{#title}}title="{{title}}"{{/title}}>{{label}}</{{tagName}}>

I would be very interested in defining this pattern but T205126 is too open ended and I'm not sure what we hope to bring back to the team. Any other constraints you had in mind would be very useful in scoping the problem. For example, should jQuery and jQuery event handling be avoided?

Seems like T206036 should be done first. We can then revisit this one.

Jdlrobson triaged this task as Normal priority.Oct 3 2018, 8:34 PM
Jdlrobson renamed this task from [spikey, ?? hrs] Composition experiment: DownloadIcon and ShareIcon should not extend Icon to Composition experiment: DownloadIcon and ShareIcon should not extend Icon.Dec 21 2018, 6:58 PM
Jdlrobson changed the task status from Stalled to Open.
Jdlrobson removed a project: Spike.
Jdlrobson updated the task description. (Show Details)

Change 472371 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Composition: DownloadIcon

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

Change 472371 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Composition: DownloadIcon

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

Jdlrobson updated the task description. (Show Details)Jan 3 2019, 6:13 PM
Jdlrobson raised the priority of this task from Normal to Needs Triage.Jan 3 2019, 10:38 PM

Change 482241 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Generate a shareIcon via composition

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

Jdlrobson set the point value for this task to 3.Jan 4 2019, 12:57 AM

Per async estimation

Change 482241 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Generate a shareIcon via composition

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

Jdlrobson assigned this task to nray.Jan 7 2019, 6:08 PM
nray updated the task description. (Show Details)Jan 7 2019, 11:43 PM
nray closed this task as Resolved.Jan 8 2019, 1:04 AM
nray removed nray as the assignee of this task.
nray added a subscriber: nray.