Page MenuHomePhabricator

Refactor: Consolidate preview template outer container
Closed, ResolvedPublic5 Story Points

Description

Note: Parent task T186129 may require a larger refactor so depending on timeline we may want to wait until that has happened before working on this task.

The Popup's empty / generic / error / disambiguation "preview" and page preview templates have similar outer divs and LESS. This task encompasses the work to make a new popup template that accepts an unescaped HTML "children" string and update the LESS so that empty / generic / error / disambiguation LESS is in preview.less, page preview LESS is in pagePreview.less, and generic popup LESS is in popup.less.

	<div class='mwe-popups mwe-popups-type-${ type }' role='tooltip' aria-hidden>
		<div class='mwe-popups-container'>
			<div class='mw-ui-icon mw-ui-icon-element mw-ui-icon-preview-${ type }'></div>
			${ showTitle ? `<strong class='mwe-popups-title'>${ title }</strong>` : '' }
			<a href='${ url }' class='mwe-popups-extract'>
					<span class='mwe-popups-message'>${ extractMsg }</span>
			</a>
			<footer>
				<a href='${ url }' class='mwe-popups-read-link'>${ linkMsg }</a>
			</footer>
		</div>
	</div>
		<div class='mwe-popups' role='tooltip' aria-hidden>
			<div class='mwe-popups-container'>
				${ hasThumbnail ? `<a href='${ url }' class='mwe-popups-discreet'></a>` : '' }
				<a dir='${ languageDirection }' lang='${ languageCode }' class='mwe-popups-extract' href='${ url }'></a>
				<footer>
					<a class='mwe-popups-settings-icon mw-ui-icon mw-ui-icon-element mw-ui-icon-popups-settings'></a>
				</footer>
			</div>
		</div>

AC

  • A generic popup container element is extracted
  • The generic popup accepts an unescaped HTML string
  • LESS is reorgnaized into three files: popup.less for the generic container, preview.less for empty / generic / error / disambiguation LESS, pagePreview.less for page preveiw LESS.

Testing

  • Verify that no regressions are introduced by checking page previews, disambiguation previews, and error previews in LTR, RTL, and mixed locales in each of the four orientations for portrait and landscape.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 6 2018, 4:53 PM

I wonder if this would happen naturally as part of T186129? Can we fold this into that task or make this a subtask? I'd like to understand how we are planning to provide that support before doing this, in case we realise a drastic change is needed.

Jdlrobson renamed this task from Consolidate preview template outer container to Refactor: Consolidate preview template outer container.Apr 12 2018, 3:40 PM
Jdlrobson triaged this task as Normal priority.
Jdlrobson updated the task description. (Show Details)
Niedzielski set the point value for this task to 5.

@Niedzielski to add AC and QA points. Primary objective is to create a common popup container.

Niedzielski updated the task description. (Show Details)Apr 18 2018, 6:56 PM
Niedzielski moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.

Change 440025 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Hygiene: refactor common popup template code

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

Niedzielski removed Niedzielski as the assignee of this task.Jun 12 2018, 10:34 PM

Change 440025 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: refactor common popup template code

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

Olga will verify page previews is working correctly (quick general QA)

ovasileva closed this task as Resolved.Jun 15 2018, 1:53 PM

Looks good, resolving.