Skip to content

Commit d6ed939

Browse files
author
epriestley
committedMar 17, 2021
Replace Differential "lint stars" with icons
Summary: Ref T9764. These stars are inconsistent, not accessible, and generally weird. They predate icons. Update them to use icons instead. Test Plan: {F8545721} {F8545722} {F8545723} Maniphest Tasks: T9764 Differential Revision: https://secure.phabricator.com/D21640
1 parent 527dd3c commit d6ed939

File tree

7 files changed

+168
-107
lines changed

7 files changed

+168
-107
lines changed
 

‎resources/celerity/map.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
'core.pkg.css' => '0ae696de',
1313
'core.pkg.js' => 'ab3502fe',
1414
'dark-console.pkg.js' => '187792c2',
15-
'differential.pkg.css' => '5c459f92',
15+
'differential.pkg.css' => 'ffb69e3d',
1616
'differential.pkg.js' => '5080baf4',
1717
'diffusion.pkg.css' => '42c75c37',
1818
'diffusion.pkg.js' => '78c9885d',
@@ -67,7 +67,7 @@
6767
'rsrc/css/application/differential/core.css' => '7300a73e',
6868
'rsrc/css/application/differential/phui-inline-comment.css' => '9863a85e',
6969
'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d',
70-
'rsrc/css/application/differential/revision-history.css' => '8aa3eac5',
70+
'rsrc/css/application/differential/revision-history.css' => '237a2979',
7171
'rsrc/css/application/differential/revision-list.css' => '93d2df7d',
7272
'rsrc/css/application/differential/table-of-contents.css' => 'bba788b9',
7373
'rsrc/css/application/diffusion/diffusion-icons.css' => '23b31a1b',
@@ -569,7 +569,7 @@
569569
'differential-core-view-css' => '7300a73e',
570570
'differential-revision-add-comment-css' => '7e5900d9',
571571
'differential-revision-comment-css' => '7dbc8d1d',
572-
'differential-revision-history-css' => '8aa3eac5',
572+
'differential-revision-history-css' => '237a2979',
573573
'differential-revision-list-css' => '93d2df7d',
574574
'differential-table-of-contents-css' => 'bba788b9',
575575
'diffusion-css' => 'e46232d6',

‎src/applications/differential/constants/DifferentialConstantsModule.php

+36
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public function renderModuleStatus(AphrontRequest $request) {
1717
return array(
1818
$this->renderRevisionStatuses($viewer),
1919
$this->renderUnitStatuses($viewer),
20+
$this->renderLintStatuses($viewer),
2021
);
2122
}
2223

@@ -141,4 +142,39 @@ private function renderUnitStatuses(PhabricatorUser $viewer) {
141142
return $view;
142143
}
143144

145+
private function renderLintStatuses(PhabricatorUser $viewer) {
146+
$statuses = DifferentialLintStatus::getStatusMap();
147+
148+
$rows = array();
149+
foreach ($statuses as $status) {
150+
$rows[] = array(
151+
$status->getValue(),
152+
id(new PHUIIconView())
153+
->setIcon($status->getIconIcon(), $status->getIconColor()),
154+
$status->getName(),
155+
);
156+
}
157+
158+
$table = id(new AphrontTableView($rows))
159+
->setHeaders(
160+
array(
161+
pht('Value'),
162+
pht('Icon'),
163+
pht('Name'),
164+
))
165+
->setColumnClasses(
166+
array(
167+
null,
168+
null,
169+
'wide pri',
170+
));
171+
172+
$view = id(new PHUIObjectBoxView())
173+
->setHeaderText(pht('Differential Lint Statuses'))
174+
->setTable($table);
175+
176+
return $view;
177+
}
178+
179+
144180
}

‎src/applications/differential/constants/DifferentialLintStatus.php

+81
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,85 @@ final class DifferentialLintStatus extends Phobject {
99
const LINT_SKIP = 4;
1010
const LINT_AUTO_SKIP = 6;
1111

12+
private $value;
13+
14+
public static function newStatusFromValue($value) {
15+
$status = new self();
16+
$status->value = $value;
17+
return $status;
18+
}
19+
20+
public function getValue() {
21+
return $this->value;
22+
}
23+
24+
public function getName() {
25+
$name = $this->getLintStatusProperty('name');
26+
27+
if ($name === null) {
28+
$name = pht('Unknown Lint Status ("%s")', $this->getValue());
29+
}
30+
31+
return $name;
32+
}
33+
34+
public function getIconIcon() {
35+
return $this->getLintStatusProperty('icon.icon');
36+
}
37+
38+
public function getIconColor() {
39+
return $this->getLintStatusProperty('icon.color');
40+
}
41+
42+
public static function getStatusMap() {
43+
$results = array();
44+
45+
foreach (self::newLintStatusMap() as $value => $ignored) {
46+
$results[$value] = self::newStatusFromValue($value);
47+
}
48+
49+
return $results;
50+
}
51+
52+
private function getLintStatusProperty($key, $default = null) {
53+
$map = self::newLintStatusMap();
54+
$properties = idx($map, $this->getValue(), array());
55+
return idx($properties, $key, $default);
56+
}
57+
58+
private static function newLintStatusMap() {
59+
return array(
60+
self::LINT_NONE => array(
61+
'name' => pht('No Lint Coverage'),
62+
'icon.icon' => 'fa-ban',
63+
'icon.color' => 'grey',
64+
),
65+
self::LINT_OKAY => array(
66+
'name' => pht('Lint Passed'),
67+
'icon.icon' => 'fa-check',
68+
'icon.color' => 'green',
69+
),
70+
self::LINT_WARN => array(
71+
'name' => pht('Lint Warnings'),
72+
'icon.icon' => 'fa-exclamation-triangle',
73+
'icon.color' => 'yellow',
74+
),
75+
self::LINT_FAIL => array(
76+
'name' => pht('Lint Errors'),
77+
'icon.icon' => 'fa-times',
78+
'icon.color' => 'red',
79+
),
80+
self::LINT_SKIP => array(
81+
'name' => pht('Lint Skipped'),
82+
'icon.icon' => 'fa-fast-forward',
83+
'icon.color' => 'blue',
84+
),
85+
self::LINT_AUTO_SKIP => array(
86+
'name' => pht('Lint Not Applicable'),
87+
'icon.icon' => 'fa-code',
88+
'icon.color' => 'grey',
89+
),
90+
);
91+
}
92+
1293
}

‎src/applications/differential/constants/DifferentialUnitStatus.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ private static function newUnitStatusMap() {
8484
),
8585
self::UNIT_AUTO_SKIP => array(
8686
'name' => pht('Tests Not Applicable'),
87-
'icon.icon' => 'fa-upload',
87+
'icon.icon' => 'fa-code',
8888
'icon.color' => 'grey',
8989
),
9090
);

‎src/applications/differential/customfield/DifferentialLintField.php

+7-23
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ protected function getLegacyProperty() {
3838
protected function getDiffPropertyKeys() {
3939
return array(
4040
'arc:lint',
41-
'arc:lint-excuse',
4241
);
4342
}
4443

@@ -84,33 +83,18 @@ protected function renderHarbormasterStatus(
8483
DifferentialDiff $diff,
8584
array $messages) {
8685

87-
$colors = array(
88-
DifferentialLintStatus::LINT_NONE => 'grey',
89-
DifferentialLintStatus::LINT_OKAY => 'green',
90-
DifferentialLintStatus::LINT_WARN => 'yellow',
91-
DifferentialLintStatus::LINT_FAIL => 'red',
92-
DifferentialLintStatus::LINT_SKIP => 'blue',
93-
DifferentialLintStatus::LINT_AUTO_SKIP => 'blue',
94-
);
95-
$icon_color = idx($colors, $diff->getLintStatus(), 'grey');
96-
97-
$message = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff);
86+
$status_value = $diff->getLintStatus();
87+
$status = DifferentialLintStatus::newStatusFromValue($status_value);
9888

99-
$excuse = $diff->getProperty('arc:lint-excuse');
100-
if (strlen($excuse)) {
101-
$excuse = array(
102-
phutil_tag('strong', array(), pht('Excuse:')),
103-
' ',
104-
phutil_escape_html_newlines($excuse),
105-
);
106-
}
89+
$status_icon = $status->getIconIcon();
90+
$status_color = $status->getIconColor();
91+
$status_name = $status->getName();
10792

10893
$status = id(new PHUIStatusListView())
10994
->addItem(
11095
id(new PHUIStatusItemView())
111-
->setIcon(PHUIStatusItemView::ICON_STAR, $icon_color)
112-
->setTarget($message)
113-
->setNote($excuse));
96+
->setIcon($status_icon, $status_color)
97+
->setTarget($status_name));
11498

11599
return $status;
116100
}

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

+40-75
Original file line numberDiff line numberDiff line change
@@ -139,34 +139,8 @@ public function render() {
139139
}
140140

141141
if ($diff) {
142-
$unit_status = idx(
143-
$this->unitStatus,
144-
$diff->getPHID(),
145-
$diff->getUnitStatus());
146-
147-
$lint = self::renderDiffLintStar($row['obj']);
148-
$lint = phutil_tag(
149-
'div',
150-
array(
151-
'class' => 'lintunit-star',
152-
'title' => self::getDiffLintMessage($diff),
153-
),
154-
$lint);
155-
156-
$status = DifferentialUnitStatus::newStatusFromValue($unit_status);
157-
158-
$unit_icon = $status->getIconIcon();
159-
$unit_color = $status->getIconColor();
160-
$unit_name = $status->getName();
161-
162-
$unit = id(new PHUIIconView())
163-
->setIcon($unit_icon, $unit_color)
164-
->addSigil('has-tooltip')
165-
->setMetadata(
166-
array(
167-
'tip' => $unit_name,
168-
));
169-
142+
$lint = $this->newLintStatusView($diff);
143+
$unit = $this->newUnitStatusView($diff);
170144
$base = $this->renderBaseRevision($diff);
171145
} else {
172146
$lint = null;
@@ -287,53 +261,6 @@ public function render() {
287261
return $content;
288262
}
289263

290-
const STAR_NONE = 'none';
291-
const STAR_OKAY = 'okay';
292-
const STAR_WARN = 'warn';
293-
const STAR_FAIL = 'fail';
294-
const STAR_SKIP = 'skip';
295-
296-
private static function renderDiffLintStar(DifferentialDiff $diff) {
297-
static $map = array(
298-
DifferentialLintStatus::LINT_NONE => self::STAR_NONE,
299-
DifferentialLintStatus::LINT_OKAY => self::STAR_OKAY,
300-
DifferentialLintStatus::LINT_WARN => self::STAR_WARN,
301-
DifferentialLintStatus::LINT_FAIL => self::STAR_FAIL,
302-
DifferentialLintStatus::LINT_SKIP => self::STAR_SKIP,
303-
DifferentialLintStatus::LINT_AUTO_SKIP => self::STAR_SKIP,
304-
);
305-
306-
$star = idx($map, $diff->getLintStatus(), self::STAR_FAIL);
307-
308-
return self::renderDiffStar($star);
309-
}
310-
311-
public static function getDiffLintMessage(DifferentialDiff $diff) {
312-
switch ($diff->getLintStatus()) {
313-
case DifferentialLintStatus::LINT_NONE:
314-
return pht('No Linters Available');
315-
case DifferentialLintStatus::LINT_OKAY:
316-
return pht('Lint OK');
317-
case DifferentialLintStatus::LINT_WARN:
318-
return pht('Lint Warnings');
319-
case DifferentialLintStatus::LINT_FAIL:
320-
return pht('Lint Errors');
321-
case DifferentialLintStatus::LINT_SKIP:
322-
return pht('Lint Skipped');
323-
case DifferentialLintStatus::LINT_AUTO_SKIP:
324-
return pht('Automatic diff as part of commit; lint not applicable.');
325-
}
326-
return pht('Unknown');
327-
}
328-
329-
private static function renderDiffStar($star) {
330-
$class = 'diff-star-'.$star;
331-
return phutil_tag(
332-
'span',
333-
array('class' => $class),
334-
"\xE2\x98\x85");
335-
}
336-
337264
private function renderBaseRevision(DifferentialDiff $diff) {
338265
switch ($diff->getSourceControlSystem()) {
339266
case 'git':
@@ -373,4 +300,42 @@ private function renderBaseRevision(DifferentialDiff $diff) {
373300
}
374301
return $link;
375302
}
303+
304+
private function newLintStatusView(DifferentialDiff $diff) {
305+
$value = $diff->getLintStatus();
306+
$status = DifferentialLintStatus::newStatusFromValue($value);
307+
308+
$icon = $status->getIconIcon();
309+
$color = $status->getIconColor();
310+
$name = $status->getName();
311+
312+
return $this->newDiffStatusIconView($icon, $color, $name);
313+
}
314+
315+
private function newUnitStatusView(DifferentialDiff $diff) {
316+
$value = $diff->getUnitStatus();
317+
318+
// NOTE: We may be overriding the value on the diff with a value from
319+
// Harbormaster.
320+
$value = idx($this->unitStatus, $diff->getPHID(), $value);
321+
322+
$status = DifferentialUnitStatus::newStatusFromValue($value);
323+
324+
$icon = $status->getIconIcon();
325+
$color = $status->getIconColor();
326+
$name = $status->getName();
327+
328+
return $this->newDiffStatusIconView($icon, $color, $name);
329+
}
330+
331+
private function newDiffStatusIconView($icon, $color, $name) {
332+
return id(new PHUIIconView())
333+
->setIcon($icon, $color)
334+
->addSigil('has-tooltip')
335+
->setMetadata(
336+
array(
337+
'tip' => $name,
338+
));
339+
}
340+
376341
}

‎webroot/rsrc/css/application/differential/revision-history.css

-5
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,3 @@
5454
td.differential-update-history-new {
5555
background: #aaffaa;
5656
}
57-
58-
.lintunit-star {
59-
text-align: center;
60-
padding: 0 16px;
61-
}

0 commit comments

Comments
 (0)
Failed to load comments.