Skip to content

Commit 3963c86

Browse files
author
epriestley
committedJan 3, 2019
Pass timeline view data to comment previews, restoring Differential comment previews
Summary: Ref T13222. In D19918, I refactored how timelines get "view data". Today, this is always additional data about which images/changesets/diffs are visible on the current revision/commit/mock, so we can tell if inline comments should be linked to a `#anchor` on the same page (if the inline is rendered there somewhere) or to a `/D123?id=1&vs=2` full link on a different page (if it isn't), but in general this could be any sort of state information about the current page that affects how the timeline should render. Previously, comment previews did not use any specialized object code and always rendered a "generic" timeline story. This was actually a bug, but none of the code we have today cares about this (since it's all inline related, and inlines render separately) so it never impacted anything. After the `TimelineEngine` change, the preview renders with Differential-specific code. This is more correct, but we were not passing the preview the "view data" so it broke. This preview doesn't actually need the view data and we could just make it bail out if it isn't present, but pass it through for consistency and so this works like we'd expect if we do something fancier with view data in the future. Test Plan: Viewed comment and inline comment previews in Differential, saw old behavior restored. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222 Differential Revision: https://secure.phabricator.com/D19943
1 parent 65e89c2 commit 3963c86

File tree

7 files changed

+42
-141
lines changed

7 files changed

+42
-141
lines changed
 

‎resources/celerity/map.php

+12-23
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
'core.pkg.css' => '47535fd5',
1313
'core.pkg.js' => 'bd89cb1d',
1414
'differential.pkg.css' => '06dc617c',
15-
'differential.pkg.js' => 'ef0b989b',
15+
'differential.pkg.js' => '853c3461',
1616
'diffusion.pkg.css' => 'a2d17c7d',
1717
'diffusion.pkg.js' => '6134c5a1',
1818
'maniphest.pkg.css' => '4845691a',
@@ -376,7 +376,6 @@
376376
'rsrc/js/application/diff/DiffChangesetList.js' => '0a84bcc1',
377377
'rsrc/js/application/diff/DiffInline.js' => 'e83d28f3',
378378
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
379-
'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
380379
'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
381380
'rsrc/js/application/differential/behavior-populate.js' => 'f0eb6708',
382381
'rsrc/js/application/differential/behavior-user-select.js' => 'a8d8459d',
@@ -422,7 +421,7 @@
422421
'rsrc/js/application/repository/repository-crossreference.js' => '9a860428',
423422
'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072',
424423
'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08',
425-
'rsrc/js/application/transactions/behavior-comment-actions.js' => '59e27e74',
424+
'rsrc/js/application/transactions/behavior-comment-actions.js' => 'd848ec84',
426425
'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243',
427426
'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
428427
'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '0e1eca96',
@@ -575,7 +574,7 @@
575574
'javelin-behavior-bulk-job-reload' => 'edf8a145',
576575
'javelin-behavior-calendar-month-view' => 'fe33e256',
577576
'javelin-behavior-choose-control' => '327a00d1',
578-
'javelin-behavior-comment-actions' => '59e27e74',
577+
'javelin-behavior-comment-actions' => 'd848ec84',
579578
'javelin-behavior-config-reorder-fields' => 'b6993408',
580579
'javelin-behavior-conpherence-menu' => '4047cd35',
581580
'javelin-behavior-conpherence-participant-pane' => 'd057e45a',
@@ -593,7 +592,6 @@
593592
'javelin-behavior-device' => 'a3714c76',
594593
'javelin-behavior-diff-preview-link' => '051c7832',
595594
'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
596-
'javelin-behavior-differential-feedback-preview' => '51c5ad07',
597595
'javelin-behavior-differential-populate' => 'f0eb6708',
598596
'javelin-behavior-differential-user-select' => 'a8d8459d',
599597
'javelin-behavior-diffusion-commit-branches' => 'bdaf4d04',
@@ -1246,14 +1244,6 @@
12461244
'javelin-typeahead-source',
12471245
'javelin-util',
12481246
),
1249-
'51c5ad07' => array(
1250-
'javelin-behavior',
1251-
'javelin-stratcom',
1252-
'javelin-dom',
1253-
'javelin-request',
1254-
'javelin-util',
1255-
'phabricator-shaped-request',
1256-
),
12571247
'522431f7' => array(
12581248
'javelin-behavior',
12591249
'javelin-util',
@@ -1323,15 +1313,6 @@
13231313
'javelin-vector',
13241314
'javelin-dom',
13251315
),
1326-
'59e27e74' => array(
1327-
'javelin-behavior',
1328-
'javelin-stratcom',
1329-
'javelin-workflow',
1330-
'javelin-dom',
1331-
'phuix-form-control-view',
1332-
'phuix-icon-view',
1333-
'javelin-behavior-phabricator-gesture',
1334-
),
13351316
'5c54cbf3' => array(
13361317
'javelin-behavior',
13371318
'javelin-stratcom',
@@ -1991,6 +1972,15 @@
19911972
'javelin-util',
19921973
'phabricator-shaped-request',
19931974
),
1975+
'd848ec84' => array(
1976+
'javelin-behavior',
1977+
'javelin-stratcom',
1978+
'javelin-workflow',
1979+
'javelin-dom',
1980+
'phuix-form-control-view',
1981+
'phuix-icon-view',
1982+
'javelin-behavior-phabricator-gesture',
1983+
),
19941984
'db34a142' => array(
19951985
'phui-inline-comment-view-css',
19961986
),
@@ -2335,7 +2325,6 @@
23352325
'differential.pkg.js' => array(
23362326
'phabricator-drag-and-drop-file-upload',
23372327
'phabricator-shaped-request',
2338-
'javelin-behavior-differential-feedback-preview',
23392328
'javelin-behavior-differential-populate',
23402329
'javelin-behavior-differential-diff-radios',
23412330
'javelin-behavior-aphront-drag-and-drop-textarea',

‎resources/celerity/packages.php

-1
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@
193193
'phabricator-drag-and-drop-file-upload',
194194
'phabricator-shaped-request',
195195

196-
'javelin-behavior-differential-feedback-preview',
197196
'javelin-behavior-differential-populate',
198197
'javelin-behavior-differential-diff-radios',
199198
'javelin-behavior-aphront-drag-and-drop-textarea',

‎src/applications/transactions/editengine/PhabricatorEditEngine.php

+8
Original file line numberDiff line numberDiff line change
@@ -1958,11 +1958,19 @@ private function buildCommentResponse($object) {
19581958
if ($request->isAjax() && $is_preview) {
19591959
$preview_content = $this->newCommentPreviewContent($object, $xactions);
19601960

1961+
$raw_view_data = $request->getStr('viewData');
1962+
try {
1963+
$view_data = phutil_json_decode($raw_view_data);
1964+
} catch (Exception $ex) {
1965+
$view_data = array();
1966+
}
1967+
19611968
return id(new PhabricatorApplicationTransactionResponse())
19621969
->setObject($object)
19631970
->setViewer($viewer)
19641971
->setTransactions($xactions)
19651972
->setIsPreview($is_preview)
1973+
->setViewData($view_data)
19661974
->setPreviewContent($preview_content);
19671975
} else {
19681976
return id(new AphrontRedirectResponse())

‎src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php

+12-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ final class PhabricatorApplicationTransactionResponse
88
private $isPreview;
99
private $previewContent;
1010
private $object;
11+
private $viewData = array();
1112

1213
protected function buildProxy() {
1314
return new AphrontAjaxResponse();
@@ -56,14 +57,24 @@ public function getPreviewContent() {
5657
return $this->previewContent;
5758
}
5859

60+
public function setViewData(array $view_data) {
61+
$this->viewData = $view_data;
62+
return $this;
63+
}
64+
65+
public function getViewData() {
66+
return $this->viewData;
67+
}
68+
5969
public function reduceProxyResponse() {
6070
$object = $this->getObject();
6171
$viewer = $this->getViewer();
6272
$xactions = $this->getTransactions();
6373

6474
$timeline_engine = PhabricatorTimelineEngine::newForObject($object)
6575
->setViewer($viewer)
66-
->setTransactions($xactions);
76+
->setTransactions($xactions)
77+
->setViewData($this->viewData);
6778

6879
$view = $timeline_engine->buildTimelineView();
6980

‎src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

+8
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,13 @@ private function renderCommentPanel() {
391391
$form->appendChild($invisi_bar);
392392
$form->addClass('phui-comment-has-actions');
393393

394+
$timeline = $this->transactionTimeline;
395+
396+
$view_data = array();
397+
if ($timeline) {
398+
$view_data = $timeline->getViewData();
399+
}
400+
394401
Javelin::initBehavior(
395402
'comment-actions',
396403
array(
@@ -405,6 +412,7 @@ private function renderCommentPanel() {
405412
'actionURI' => $this->getAction(),
406413
'drafts' => $draft_keys,
407414
'defaultButtonText' => $this->getSubmitButtonName(),
415+
'viewData' => $view_data,
408416
));
409417
}
410418

‎webroot/rsrc/js/application/differential/behavior-comment-preview.js

-116
This file was deleted.

‎webroot/rsrc/js/application/transactions/behavior-comment-actions.js

+2
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ JX.behavior('comment-actions', function(config) {
9090
data.__preview__ = 1;
9191
data[input_node.name] = serialize_actions();
9292

93+
data.viewData = JX.JSON.stringify(config.viewData);
94+
9395
return data;
9496
}
9597

0 commit comments

Comments
 (0)
Failed to load comments.