Skip to content

Commit 56a9709

Browse files
author
epriestley
committed
Reduce code duplication for inline "Undo"
Summary: Ref T2009. This is another almost-identical copy of the row scaffolding, which has the same 1up/2up bugs as the 8 other copies of this code. Turn the "undo" element into an InlineCommentView so we can scaffold it. Then, scaffold it with the same code as everything else. Test Plan: Hit "Undo", swapped from 1up to 2up, hit "undo" again, swapped back, tried left/right, everything rendered with proper scaffolding. Reviewers: btrahan, chad Reviewed By: chad Subscribers: epriestley Maniphest Tasks: T2009 Differential Revision: https://secure.phabricator.com/D12019
1 parent 355142f commit 56a9709

File tree

11 files changed

+101
-48
lines changed

11 files changed

+101
-48
lines changed

src/__phutil_library_map__.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,7 @@
11391139
'PHUIDiffInlineCommentEditView' => 'infrastructure/diff/view/PHUIDiffInlineCommentEditView.php',
11401140
'PHUIDiffInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php',
11411141
'PHUIDiffInlineCommentTableScaffold' => 'infrastructure/diff/view/PHUIDiffInlineCommentTableScaffold.php',
1142+
'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php',
11421143
'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php',
11431144
'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php',
11441145
'PHUIDiffTwoUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php',
@@ -4371,6 +4372,7 @@
43714372
'PHUIDiffInlineCommentEditView' => 'PHUIDiffInlineCommentView',
43724373
'PHUIDiffInlineCommentRowScaffold' => 'AphrontView',
43734374
'PHUIDiffInlineCommentTableScaffold' => 'AphrontView',
4375+
'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView',
43744376
'PHUIDiffInlineCommentView' => 'AphrontView',
43754377
'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
43764378
'PHUIDiffTwoUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',

src/applications/differential/controller/DifferentialChangesetViewController.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ public function processRequest() {
217217

218218
return id(new PhabricatorChangesetResponse())
219219
->setRenderedChangeset($parser->renderChangeset())
220-
->setCoverage($coverage);
220+
->setCoverage($coverage)
221+
->setUndoTemplates($parser->getRenderer()->renderUndoTemplates());
221222
}
222223

223224
$diff = $changeset->getDiff();

src/applications/differential/render/DifferentialChangesetRenderer.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,4 +600,19 @@ protected function getChangesetProperties($changeset) {
600600
return array($old, $new);
601601
}
602602

603+
public function renderUndoTemplates() {
604+
$views = array(
605+
'l' => id(new PHUIDiffInlineCommentUndoView())->setIsOnRight(false),
606+
'r' => id(new PHUIDiffInlineCommentUndoView())->setIsOnRight(true),
607+
);
608+
609+
foreach ($views as $key => $view) {
610+
$scaffold = $this->getRowScaffoldForInline($view);
611+
$views[$key] = id(new PHUIDiffInlineCommentTableScaffold())
612+
->addRowScaffold($scaffold);
613+
}
614+
615+
return $views;
616+
}
617+
603618
}

src/applications/differential/view/DifferentialChangesetDetailView.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,9 @@ public function render() {
208208
$icon = id(new PHUIIconView())
209209
->setIconFont($display_icon);
210210

211+
$renderer = DifferentialChangesetHTMLRenderer::getHTMLRendererByKey(
212+
$this->getRenderer());
213+
211214
return javelin_tag(
212215
'div',
213216
array(
@@ -224,6 +227,7 @@ public function render() {
224227
'ref' => $this->getRenderingRef(),
225228
'autoload' => $this->getAutoload(),
226229
'loaded' => $this->getLoaded(),
230+
'undoTemplates' => $renderer->renderUndoTemplates(),
227231
),
228232
'class' => $class,
229233
'id' => $id,
@@ -252,4 +256,5 @@ public function render() {
252256
));
253257
}
254258

259+
255260
}

src/applications/differential/view/DifferentialChangesetListView.php

Lines changed: 1 addition & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ public function render() {
123123
'collapsed' => pht('This file content has been collapsed.'),
124124
),
125125
));
126+
126127
Javelin::initBehavior(
127128
'differential-dropdown-menus',
128129
array(
@@ -230,11 +231,8 @@ public function render() {
230231
$this->initBehavior('differential-comment-jump', array());
231232

232233
if ($this->inlineURI) {
233-
$undo_templates = $this->renderUndoTemplates();
234-
235234
Javelin::initBehavior('differential-edit-inline-comments', array(
236235
'uri' => $this->inlineURI,
237-
'undo_templates' => $undo_templates,
238236
'stage' => 'differential-review-stage',
239237
));
240238
}
@@ -257,44 +255,6 @@ public function render() {
257255
return $object_box;
258256
}
259257

260-
/**
261-
* Render the "Undo" markup for the inline comment undo feature.
262-
*/
263-
private function renderUndoTemplates() {
264-
$link = javelin_tag(
265-
'a',
266-
array(
267-
'href' => '#',
268-
'sigil' => 'differential-inline-comment-undo',
269-
),
270-
pht('Undo'));
271-
272-
$div = phutil_tag(
273-
'div',
274-
array(
275-
'class' => 'differential-inline-undo',
276-
),
277-
array('Changes discarded. ', $link));
278-
279-
return array(
280-
'l' => phutil_tag('table', array(),
281-
phutil_tag('tr', array(), array(
282-
phutil_tag('th', array()),
283-
phutil_tag('td', array(), $div),
284-
phutil_tag('th', array()),
285-
phutil_tag('td', array('colspan' => 3)),
286-
))),
287-
288-
'r' => phutil_tag('table', array(),
289-
phutil_tag('tr', array(), array(
290-
phutil_tag('th', array()),
291-
phutil_tag('td', array()),
292-
phutil_tag('th', array()),
293-
phutil_tag('td', array('colspan' => 3), $div),
294-
))),
295-
);
296-
}
297-
298258
private function renderViewOptionsDropdown(
299259
DifferentialChangesetDetailView $detail,
300260
$ref,

src/applications/diffusion/controller/DiffusionDiffController.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ protected function processDiffusionRequest(AphrontRequest $request) {
130130
$parser->setMask($mask);
131131

132132
return id(new PhabricatorChangesetResponse())
133-
->setRenderedChangeset($parser->renderChangeset());
133+
->setRenderedChangeset($parser->renderChangeset())
134+
->setUndoTemplates($parser->getRenderer()->renderUndoTemplates());
134135
}
135136
}

src/infrastructure/diff/PhabricatorChangesetResponse.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ final class PhabricatorChangesetResponse extends AphrontProxyResponse {
44

55
private $renderedChangeset;
66
private $coverage;
7+
private $undoTemplates;
78

89
public function setRenderedChangeset($rendered_changeset) {
910
$this->renderedChangeset = $rendered_changeset;
@@ -15,6 +16,11 @@ public function setCoverage($coverage) {
1516
return $this;
1617
}
1718

19+
public function setUndoTemplates($undo_templates) {
20+
$this->undoTemplates = $undo_templates;
21+
return $this;
22+
}
23+
1824
protected function buildProxy() {
1925
return new AphrontAjaxResponse();
2026
}
@@ -28,6 +34,10 @@ public function reduceProxyResponse() {
2834
$content['coverage'] = $this->coverage;
2935
}
3036

37+
if ($this->undoTemplates) {
38+
$content['undoTemplates'] = $this->undoTemplates;
39+
}
40+
3141
return $this->getProxy()->setContent($content);
3242
}
3343

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
/**
4+
* Render the "Undo" action to recover discarded inline comments.
5+
*
6+
* This extends @{class:PHUIDiffInlineCommentView} so it can use the same
7+
* scaffolding code as other kinds of inline comments.
8+
*/
9+
final class PHUIDiffInlineCommentUndoView
10+
extends PHUIDiffInlineCommentView {
11+
12+
private $isOnRight;
13+
14+
public function setIsOnRight($is_on_right) {
15+
$this->isOnRight = $is_on_right;
16+
return $this;
17+
}
18+
19+
public function getIsOnRight() {
20+
return $this->isOnRight;
21+
}
22+
23+
public function render() {
24+
$link = javelin_tag(
25+
'a',
26+
array(
27+
'href' => '#',
28+
'sigil' => 'differential-inline-comment-undo',
29+
),
30+
pht('Undo'));
31+
32+
return phutil_tag(
33+
'div',
34+
array(
35+
'class' => 'differential-inline-undo',
36+
),
37+
array('Changes discarded. ', $link));
38+
}
39+
40+
}

webroot/rsrc/css/application/differential/changeset-view.css

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,12 @@ td.cov-I {
379379
.differential-inline-undo {
380380
padding: 4px;
381381
text-align: center;
382-
background: #ffeeaa;
382+
background: {$lightyellow};
383+
border: 1px solid {$yellow};
383384
margin: 3px 0 1px;
384-
font: 12px;
385-
color: 444444;
385+
color: {$darkgreytext};
386+
font: {$basefont};
387+
font-size: 12px;
386388
}
387389

388390
.differential-inline-undo a {

webroot/rsrc/js/application/differential/ChangesetViewManager.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ JX.install('ChangesetViewManager', {
3838
_renderer: null,
3939
_highlight: null,
4040
_encoding: null,
41+
_undoTemplates: null,
4142

4243

4344
/**
@@ -193,6 +194,8 @@ JX.install('ChangesetViewManager', {
193194
var root = target.parentNode;
194195
this._moveRows(table, root, target);
195196
root.removeChild(target);
197+
198+
this._onchangesetresponse(response);
196199
},
197200

198201
_moveRows: function(src, dst, before) {
@@ -256,6 +259,10 @@ JX.install('ChangesetViewManager', {
256259
return (JX.Device.getDevice() == 'desktop') ? '2up' : '1up';
257260
},
258261

262+
getUndoTemplates: function() {
263+
return this._undoTemplates;
264+
},
265+
259266
setEncoding: function(encoding) {
260267
this._encoding = encoding;
261268
return this;
@@ -333,6 +340,12 @@ JX.install('ChangesetViewManager', {
333340
this._stabilize = false;
334341
}
335342

343+
this._onchangesetresponse(response);
344+
},
345+
346+
_onchangesetresponse: function(response) {
347+
// Code shared by autoload and context responses.
348+
336349
if (response.coverage) {
337350
for (var k in response.coverage) {
338351
try {
@@ -342,6 +355,10 @@ JX.install('ChangesetViewManager', {
342355
}
343356
}
344357
}
358+
359+
if (response.undoTemplates) {
360+
this._undoTemplates = response.undoTemplates;
361+
}
345362
},
346363

347364
_getContentFrame: function() {

0 commit comments

Comments
 (0)