Skip to content

Commit b50cdc6

Browse files
author
vrana
committedAug 21, 2012
Highlight update time in revision list
Summary: This is another experiment for reducing reviewers response time. I stole the idea (and colors) from [[ http://www.reviewboard.org/media/screenshots/2009/02/02/dashboard.png | ReviewBoard ]]. I actually quite like it (except when everything is red) and I can image that people will review just to have better color balance. The code is not production ready for these reasons: - We load holidays again and again for each revision. I couldn't cache it to static variable because it could persist multiple requests, right? - I don't know how to expand height to the whole cell (I'm really bad in CSS). - CSS rules are probably in wrong file. - We probably want to use different colors. This is how it looks: {F16406} Test Plan: Displayed revision list. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D3190
1 parent 90f5f86 commit b50cdc6

File tree

7 files changed

+110
-26
lines changed

7 files changed

+110
-26
lines changed
 

‎scripts/celerity_mapper.php

+1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@
9090
'differential-changeset-view-css',
9191
'differential-results-table-css',
9292
'differential-revision-history-css',
93+
'differential-revision-list-css',
9394
'differential-table-of-contents-css',
9495
'differential-revision-comment-css',
9596
'differential-revision-add-comment-css',

‎src/__celerity_resource_map__.php

+35-24
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,15 @@
751751
),
752752
'disk' => '/rsrc/css/application/differential/revision-history.css',
753753
),
754+
'differential-revision-list-css' =>
755+
array(
756+
'uri' => '/res/7659ad8d/rsrc/css/application/differential/revision-list.css',
757+
'type' => 'css',
758+
'requires' =>
759+
array(
760+
),
761+
'disk' => '/rsrc/css/application/differential/revision-list.css',
762+
),
754763
'differential-table-of-contents-css' =>
755764
array(
756765
'uri' => '/res/0ac99a19/rsrc/css/application/differential/table-of-contents.css',
@@ -2956,7 +2965,7 @@
29562965
'uri' => '/res/pkg/971b021e/core.pkg.js',
29572966
'type' => 'js',
29582967
),
2959-
'19ebcc79' =>
2968+
'9a22cfb9' =>
29602969
array(
29612970
'name' => 'differential.pkg.css',
29622971
'symbols' =>
@@ -2965,17 +2974,18 @@
29652974
1 => 'differential-changeset-view-css',
29662975
2 => 'differential-results-table-css',
29672976
3 => 'differential-revision-history-css',
2968-
4 => 'differential-table-of-contents-css',
2969-
5 => 'differential-revision-comment-css',
2970-
6 => 'differential-revision-add-comment-css',
2971-
7 => 'differential-revision-comment-list-css',
2972-
8 => 'phabricator-object-selector-css',
2973-
9 => 'aphront-headsup-action-list-view-css',
2974-
10 => 'phabricator-content-source-view-css',
2975-
11 => 'differential-local-commits-view-css',
2976-
12 => 'inline-comment-summary-css',
2977+
4 => 'differential-revision-list-css',
2978+
5 => 'differential-table-of-contents-css',
2979+
6 => 'differential-revision-comment-css',
2980+
7 => 'differential-revision-add-comment-css',
2981+
8 => 'differential-revision-comment-list-css',
2982+
9 => 'phabricator-object-selector-css',
2983+
10 => 'aphront-headsup-action-list-view-css',
2984+
11 => 'phabricator-content-source-view-css',
2985+
12 => 'differential-local-commits-view-css',
2986+
13 => 'inline-comment-summary-css',
29772987
),
2978-
'uri' => '/res/pkg/19ebcc79/differential.pkg.css',
2988+
'uri' => '/res/pkg/9a22cfb9/differential.pkg.css',
29792989
'type' => 'css',
29802990
),
29812991
'ffc69a16' =>
@@ -3098,7 +3108,7 @@
30983108
'aphront-dialog-view-css' => 'a740b991',
30993109
'aphront-error-view-css' => 'a740b991',
31003110
'aphront-form-view-css' => 'a740b991',
3101-
'aphront-headsup-action-list-view-css' => '19ebcc79',
3111+
'aphront-headsup-action-list-view-css' => '9a22cfb9',
31023112
'aphront-headsup-view-css' => 'a740b991',
31033113
'aphront-list-filter-view-css' => 'a740b991',
31043114
'aphront-pager-view-css' => 'a740b991',
@@ -3108,19 +3118,20 @@
31083118
'aphront-tokenizer-control-css' => 'a740b991',
31093119
'aphront-tooltip-css' => 'a740b991',
31103120
'aphront-typeahead-control-css' => 'a740b991',
3111-
'differential-changeset-view-css' => '19ebcc79',
3112-
'differential-core-view-css' => '19ebcc79',
3121+
'differential-changeset-view-css' => '9a22cfb9',
3122+
'differential-core-view-css' => '9a22cfb9',
31133123
'differential-inline-comment-editor' => 'ffc69a16',
3114-
'differential-local-commits-view-css' => '19ebcc79',
3115-
'differential-results-table-css' => '19ebcc79',
3116-
'differential-revision-add-comment-css' => '19ebcc79',
3117-
'differential-revision-comment-css' => '19ebcc79',
3118-
'differential-revision-comment-list-css' => '19ebcc79',
3119-
'differential-revision-history-css' => '19ebcc79',
3120-
'differential-table-of-contents-css' => '19ebcc79',
3124+
'differential-local-commits-view-css' => '9a22cfb9',
3125+
'differential-results-table-css' => '9a22cfb9',
3126+
'differential-revision-add-comment-css' => '9a22cfb9',
3127+
'differential-revision-comment-css' => '9a22cfb9',
3128+
'differential-revision-comment-list-css' => '9a22cfb9',
3129+
'differential-revision-history-css' => '9a22cfb9',
3130+
'differential-revision-list-css' => '9a22cfb9',
3131+
'differential-table-of-contents-css' => '9a22cfb9',
31213132
'diffusion-commit-view-css' => 'c8ce2d88',
31223133
'diffusion-icons-css' => 'c8ce2d88',
3123-
'inline-comment-summary-css' => '19ebcc79',
3134+
'inline-comment-summary-css' => '9a22cfb9',
31243135
'javelin-behavior' => '6fb20113',
31253136
'javelin-behavior-aphront-basic-tokenizer' => '97f65640',
31263137
'javelin-behavior-aphront-drag-and-drop' => 'ffc69a16',
@@ -3174,7 +3185,7 @@
31743185
'maniphest-task-summary-css' => '7839ae2d',
31753186
'maniphest-transaction-detail-css' => '7839ae2d',
31763187
'phabricator-app-buttons-css' => 'a740b991',
3177-
'phabricator-content-source-view-css' => '19ebcc79',
3188+
'phabricator-content-source-view-css' => '9a22cfb9',
31783189
'phabricator-core-buttons-css' => 'a740b991',
31793190
'phabricator-core-css' => 'a740b991',
31803191
'phabricator-directory-css' => 'a740b991',
@@ -3185,7 +3196,7 @@
31853196
'phabricator-keyboard-shortcut' => '971b021e',
31863197
'phabricator-keyboard-shortcut-manager' => '971b021e',
31873198
'phabricator-menu-item' => '971b021e',
3188-
'phabricator-object-selector-css' => '19ebcc79',
3199+
'phabricator-object-selector-css' => '9a22cfb9',
31893200
'phabricator-paste-file-upload' => '971b021e',
31903201
'phabricator-prefab' => '971b021e',
31913202
'phabricator-project-tag-css' => '7839ae2d',

‎src/applications/differential/controller/DifferentialRevisionListController.php

+1
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,7 @@ private function buildViews($filter, $user_phid, array $revisions) {
438438
$user_phid);
439439

440440
$view = id(clone $template)
441+
->setHighlightAge(true)
441442
->setRevisions($active);
442443
$views[] = array(
443444
'title' => 'Action Required',

‎src/applications/differential/view/DifferentialRevisionListView.php

+38
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,12 @@ final class DifferentialRevisionListView extends AphrontView {
2525
private $handles;
2626
private $user;
2727
private $fields;
28+
private $highlightAge;
2829
const NO_DATA_STRING = 'No revisions found.';
2930

31+
const DAYS_FRESH = 1;
32+
const DAYS_STALE = 3;
33+
3034
public function setFields(array $fields) {
3135
assert_instances_of($fields, 'DifferentialFieldSpecification');
3236
$this->fields = $fields;
@@ -39,6 +43,11 @@ public function setRevisions(array $revisions) {
3943
return $this;
4044
}
4145

46+
public function setHighlightAge($bool) {
47+
$this->highlightAge = $bool;
48+
return $this;
49+
}
50+
4251
public function getRequiredHandlePHIDs() {
4352
$phids = array();
4453
foreach ($this->fields as $field) {
@@ -67,6 +76,15 @@ public function render() {
6776
throw new Exception("Call setUser() before render()!");
6877
}
6978

79+
if ($this->highlightAge) {
80+
$fresh = PhabricatorCalendarHoliday::getNthBusinessDay(
81+
time(),
82+
-self::DAYS_FRESH);
83+
$stale = PhabricatorCalendarHoliday::getNthBusinessDay(
84+
time(),
85+
-self::DAYS_STALE);
86+
}
87+
7088
Javelin::initBehavior('phabricator-tooltips', array());
7189
require_celerity_resource('aphront-tooltip-css');
7290

@@ -81,6 +99,7 @@ public function render() {
8199
$field->setHandles($this->handles);
82100
}
83101

102+
$cell_classes = array();
84103
$rows = array();
85104
foreach ($this->revisions as $revision) {
86105
$phid = $revision->getPHID();
@@ -104,9 +123,25 @@ public function render() {
104123
'');
105124
}
106125
$row = array($flag);
126+
127+
$modified = $revision->getDateModified();
128+
107129
foreach ($this->fields as $field) {
130+
if ($this->highlightAge &&
131+
$field instanceof DifferentialDateModifiedFieldSpecification) {
132+
if ($modified < $stale) {
133+
$class = 'revision-age-old';
134+
} else if ($modified < $fresh) {
135+
$class = 'revision-age-stale';
136+
} else {
137+
$class = 'revision-age-fresh';
138+
}
139+
$cell_classes[count($rows)][count($row)] = $class;
140+
}
141+
108142
$row[] = $field->renderValueForRevisionList($revision);
109143
}
144+
110145
$rows[] = $row;
111146
}
112147

@@ -120,9 +155,12 @@ public function render() {
120155
$table = new AphrontTableView($rows);
121156
$table->setHeaders($headers);
122157
$table->setColumnClasses($classes);
158+
$table->setCellClasses($cell_classes);
123159

124160
$table->setNoDataString(DifferentialRevisionListView::NO_DATA_STRING);
125161

162+
require_celerity_resource('differential-revision-history-css');
163+
126164
return $table->render();
127165
}
128166

‎src/applications/directory/controller/PhabricatorDirectoryMainController.php

+1
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ private function buildRevisionPanel() {
323323
"View Active Revisions \xC2\xBB"));
324324

325325
$revision_view = id(new DifferentialRevisionListView())
326+
->setHighlightAge(true)
326327
->setRevisions($active)
327328
->setFields(DifferentialRevisionListView::getDefaultFields())
328329
->setUser($user);

‎src/view/control/AphrontTableView.php

+11-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ final class AphrontTableView extends AphrontView {
2222
protected $headers;
2323
protected $rowClasses = array();
2424
protected $columnClasses = array();
25+
protected $cellClasses = array();
2526
protected $zebraStripes = true;
2627
protected $noDataString;
2728
protected $className;
@@ -52,6 +53,11 @@ public function setRowClasses(array $row_classes) {
5253
return $this;
5354
}
5455

56+
public function setCellClasses(array $cell_classes) {
57+
$this->cellClasses = $cell_classes;
58+
return $this;
59+
}
60+
5561
public function setNoDataString($no_data_string) {
5662
$this->noDataString = $no_data_string;
5763
return $this;
@@ -191,7 +197,7 @@ public function render() {
191197
}
192198

193199
if ($value !== null) {
194-
$col_classes[$key] = ' class="'.$value.'"';
200+
$col_classes[$key] = $value;
195201
}
196202
}
197203

@@ -226,8 +232,11 @@ public function render() {
226232
continue;
227233
}
228234
$class = $col_classes[$col_num];
235+
if (!empty($this->cellClasses[$row_num][$col_num])) {
236+
$class = trim($class.' '.$this->cellClasses[$row_num][$col_num]);
237+
}
229238
if ($class !== null) {
230-
$table[] = '<td'.$class.'>';
239+
$table[] = '<td class="'.$class.'">';
231240
} else {
232241
$table[] = '<td>';
233242
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @provides differential-revision-list-css
3+
*/
4+
5+
6+
.revision-age-fresh,
7+
.revision-age-stale,
8+
.revision-age-old {
9+
margin: -4px -8px;
10+
padding: 4px 8px;
11+
}
12+
13+
.revision-age-fresh {
14+
background: #b4e3b2;
15+
}
16+
17+
.revision-age-stale {
18+
background: #f3e782;
19+
}
20+
21+
.revision-age-old {
22+
background: #f0acac;
23+
}

0 commit comments

Comments
 (0)
Failed to load comments.