Skip to content

Commit 2f53987

Browse files
author
epriestley
committed
Store inline comment offset information and show it when highlighting comments
Summary: Ref T13513. When a user selects a text range and uses "New Inline Comment" to create a comment directly from a range, store the offset information alongside the comment. When hovering the comment, highlight the original range. Test Plan: {F7480926, size=full} Maniphest Tasks: T13513 Differential Revision: https://secure.phabricator.com/D21250
1 parent ebef22c commit 2f53987

File tree

9 files changed

+377
-73
lines changed

9 files changed

+377
-73
lines changed

resources/celerity/map.php

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
'core.pkg.css' => 'a560707d',
1313
'core.pkg.js' => '0efaf0ac',
1414
'dark-console.pkg.js' => '187792c2',
15-
'differential.pkg.css' => 'd71d4531',
16-
'differential.pkg.js' => 'ac6914bb',
15+
'differential.pkg.css' => 'b042ee8b',
16+
'differential.pkg.js' => '4b2b5659',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => 'a98c0bf7',
1919
'maniphest.pkg.css' => '35995d6d',
@@ -63,7 +63,7 @@
6363
'rsrc/css/application/diff/diff-tree-view.css' => 'e2d3e222',
6464
'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d',
6565
'rsrc/css/application/differential/add-comment.css' => '7e5900d9',
66-
'rsrc/css/application/differential/changeset-view.css' => 'a5cc67cf',
66+
'rsrc/css/application/differential/changeset-view.css' => 'df3afa61',
6767
'rsrc/css/application/differential/core.css' => '7300a73e',
6868
'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b',
6969
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
@@ -379,9 +379,9 @@
379379
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be',
380380
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
381381
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
382-
'rsrc/js/application/diff/DiffChangeset.js' => '20715b98',
383-
'rsrc/js/application/diff/DiffChangesetList.js' => '9d5b137e',
384-
'rsrc/js/application/diff/DiffInline.js' => '6227a0e3',
382+
'rsrc/js/application/diff/DiffChangeset.js' => 'b6bb0240',
383+
'rsrc/js/application/diff/DiffChangesetList.js' => '2347e0a6',
384+
'rsrc/js/application/diff/DiffInline.js' => '417b3cdb',
385385
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
386386
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
387387
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
@@ -559,7 +559,7 @@
559559
'conpherence-transaction-css' => '3a3f5e7e',
560560
'd3' => '9d068042',
561561
'diff-tree-view-css' => 'e2d3e222',
562-
'differential-changeset-view-css' => 'a5cc67cf',
562+
'differential-changeset-view-css' => 'df3afa61',
563563
'differential-core-view-css' => '7300a73e',
564564
'differential-revision-add-comment-css' => '7e5900d9',
565565
'differential-revision-comment-css' => '7dbc8d1d',
@@ -774,9 +774,9 @@
774774
'phabricator-darklog' => '3b869402',
775775
'phabricator-darkmessage' => '26cd4b73',
776776
'phabricator-dashboard-css' => '5a205b9d',
777-
'phabricator-diff-changeset' => '20715b98',
778-
'phabricator-diff-changeset-list' => '9d5b137e',
779-
'phabricator-diff-inline' => '6227a0e3',
777+
'phabricator-diff-changeset' => 'b6bb0240',
778+
'phabricator-diff-changeset-list' => '2347e0a6',
779+
'phabricator-diff-inline' => '417b3cdb',
780780
'phabricator-diff-path-view' => '8207abf9',
781781
'phabricator-diff-tree-view' => '5d83623b',
782782
'phabricator-drag-and-drop-file-upload' => '4370900d',
@@ -1082,19 +1082,6 @@
10821082
'javelin-behavior',
10831083
'javelin-request',
10841084
),
1085-
'20715b98' => array(
1086-
'javelin-dom',
1087-
'javelin-util',
1088-
'javelin-stratcom',
1089-
'javelin-install',
1090-
'javelin-workflow',
1091-
'javelin-router',
1092-
'javelin-behavior-device',
1093-
'javelin-vector',
1094-
'phabricator-diff-inline',
1095-
'phabricator-diff-path-view',
1096-
'phuix-button-view',
1097-
),
10981085
'225bbb98' => array(
10991086
'javelin-install',
11001087
'javelin-reactor',
@@ -1111,6 +1098,11 @@
11111098
'javelin-request',
11121099
'javelin-typeahead-source',
11131100
),
1101+
'2347e0a6' => array(
1102+
'javelin-install',
1103+
'phuix-button-view',
1104+
'phabricator-diff-tree-view',
1105+
),
11141106
23631304 => array(
11151107
'phui-fontkit-css',
11161108
),
@@ -1259,6 +1251,9 @@
12591251
'javelin-behavior',
12601252
'javelin-uri',
12611253
),
1254+
'417b3cdb' => array(
1255+
'javelin-dom',
1256+
),
12621257
'42c44e8b' => array(
12631258
'javelin-behavior',
12641259
'javelin-workflow',
@@ -1503,9 +1498,6 @@
15031498
'60cd9241' => array(
15041499
'javelin-behavior',
15051500
),
1506-
'6227a0e3' => array(
1507-
'javelin-dom',
1508-
),
15091501
'6337cf26' => array(
15101502
'javelin-behavior',
15111503
'javelin-dom',
@@ -1816,11 +1808,6 @@
18161808
'javelin-uri',
18171809
'phabricator-textareautils',
18181810
),
1819-
'9d5b137e' => array(
1820-
'javelin-install',
1821-
'phuix-button-view',
1822-
'phabricator-diff-tree-view',
1823-
),
18241811
'9f081f05' => array(
18251812
'javelin-behavior',
18261813
'javelin-dom',
@@ -1865,9 +1852,6 @@
18651852
'javelin-install',
18661853
'javelin-dom',
18671854
),
1868-
'a5cc67cf' => array(
1869-
'phui-inline-comment-view-css',
1870-
),
18711855
'a77e2cbd' => array(
18721856
'javelin-behavior',
18731857
'javelin-stratcom',
@@ -1999,6 +1983,19 @@
19991983
'javelin-stratcom',
20001984
'javelin-dom',
20011985
),
1986+
'b6bb0240' => array(
1987+
'javelin-dom',
1988+
'javelin-util',
1989+
'javelin-stratcom',
1990+
'javelin-install',
1991+
'javelin-workflow',
1992+
'javelin-router',
1993+
'javelin-behavior-device',
1994+
'javelin-vector',
1995+
'phabricator-diff-inline',
1996+
'phabricator-diff-path-view',
1997+
'phuix-button-view',
1998+
),
20021999
'b7b73831' => array(
20032000
'javelin-behavior',
20042001
'javelin-dom',
@@ -2124,6 +2121,9 @@
21242121
'javelin-uri',
21252122
'phabricator-notification',
21262123
),
2124+
'df3afa61' => array(
2125+
'phui-inline-comment-view-css',
2126+
),
21272127
'e150bd50' => array(
21282128
'javelin-behavior',
21292129
'javelin-stratcom',

src/applications/celerity/postprocessor/CelerityDefaultPostprocessor.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ public function buildVariables() {
208208
'gentle.highlight' => '#fdf3da',
209209
'gentle.highlight.border' => '#c9b8a8',
210210

211+
'highlight.bright' => '#fdf320',
212+
211213
'paste.content' => '#fffef5',
212214
'paste.border' => '#e9dbcd',
213215
'paste.highlight' => '#fdf3da',

src/infrastructure/diff/PhabricatorInlineCommentController.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,9 @@ public function processRequest() {
320320
->setLineLength($length)
321321
->setContent((string)$this->getCommentText())
322322
->setReplyToCommentPHID($this->getReplyToCommentPHID())
323-
->setIsEditing(true);
323+
->setIsEditing(true)
324+
->setStartOffset($request->getInt('startOffset'))
325+
->setEndOffset($request->getInt('endOffset'));
324326

325327
$document_engine_key = $request->getStr('documentEngineKey');
326328
if ($document_engine_key !== null) {

src/infrastructure/diff/interface/PhabricatorInlineComment.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,24 @@ public function getDocumentEngineKey() {
222222
return $this->getStorageObject()->getAttribute('documentEngineKey');
223223
}
224224

225+
public function setStartOffset($offset) {
226+
$this->getStorageObject()->setAttribute('startOffset', $offset);
227+
return $this;
228+
}
229+
230+
public function getStartOffset() {
231+
return $this->getStorageObject()->getAttribute('startOffset');
232+
}
233+
234+
public function setEndOffset($offset) {
235+
$this->getStorageObject()->setAttribute('endOffset', $offset);
236+
return $this;
237+
}
238+
239+
public function getEndOffset() {
240+
return $this->getStorageObject()->getAttribute('endOffset');
241+
}
242+
225243
public function getDateModified() {
226244
return $this->getStorageObject()->getDateModified();
227245
}

src/infrastructure/diff/view/PHUIDiffInlineCommentView.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ protected function getInlineCommentMetadata() {
9191
'isDraftDone' => $is_draft_done,
9292
'isEditing' => $inline->getIsEditing(),
9393
'documentEngineKey' => $inline->getDocumentEngineKey(),
94+
'startOffset' => $inline->getStartOffset(),
95+
'endOffset' => $inline->getEndOffset(),
9496

9597
'on_right' => $this->getIsOnRight(),
9698
);

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,3 +487,27 @@ unselectable. */
487487
background: {$lightyellow};
488488
color: {$blacktext};
489489
}
490+
491+
.differential-diff tr td.inline-hover {
492+
background: {$gentle.highlight};
493+
}
494+
495+
.differential-diff tr td.inline-hover-bright {
496+
background: {$highlight.bright};
497+
}
498+
499+
.inline-hover-container {
500+
position: absolute;
501+
color: {$lightgreytext};
502+
background: {$lightyellow};
503+
}
504+
505+
.inline-hover-text {
506+
padding-top: 2px;
507+
padding-bottom: 2px;
508+
}
509+
510+
.inline-hover-text-bright {
511+
color: {$blacktext};
512+
background: {$highlight.bright};
513+
}

webroot/rsrc/js/application/diff/DiffChangeset.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ JX.install('DiffChangeset', {
711711
return data.inline;
712712
},
713713

714-
newInlineForRange: function(origin, target) {
714+
newInlineForRange: function(origin, target, options) {
715715
var list = this.getChangesetList();
716716

717717
var src = list.getLineNumberFromHeader(origin);
@@ -742,6 +742,8 @@ JX.install('DiffChangeset', {
742742
isNewFile: is_new
743743
};
744744

745+
JX.copy(data, options || {});
746+
745747
var inline = new JX.DiffInline()
746748
.setChangeset(this)
747749
.bindToRange(data);

0 commit comments

Comments
 (0)