Page MenuHomePhabricator

Composition experiment: DownloadIcon and ShareIcon should not extend Icon
Closed, ResolvedPublic3 Estimated 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)

Event Timeline

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 Web-Team-Backlog board.
Jdlrobson updated the task description. (Show Details)
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 Medium 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

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

nray removed nray as the assignee of this task.
nray subscribed.