From f04d8ab1a747dc9719d378d9286088b677ce224c Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 1 May 2012 10:15:56 -0700 Subject: [PATCH] Further improve unit/lint rendering Summary: I think this improves things, let me know if you have feedback. Also addresses T840. Test Plan: See screenshots... Reviewers: vrana, btrahan, jungejason Reviewed By: vrana CC: aran, zeeg Maniphest Tasks: T840 Differential Revision: https://secure.phabricator.com/D2357 --- scripts/celerity_mapper.php | 2 +- src/__celerity_resource_map__.php | 52 ++--- src/__phutil_library_map__.php | 4 +- .../DifferentialLintFieldSpecification.php | 183 +++++++++++------ .../field/specification/lint/__init__.php | 5 +- .../DifferentialUnitFieldSpecification.php | 187 ++++++++++++------ .../field/specification/unit/__init__.php | 7 +- .../DifferentialResultsTableView.php | 131 ++++++++++++ .../view/resultstableview/__init__.php | 18 ++ .../DifferentialRevisionDetailView.php | 1 - .../differential/results-table.css | 79 ++++++++ .../differential/revision-detail.css | 68 ------- .../behavior-show-field-details.js | 43 ++-- 13 files changed, 538 insertions(+), 242 deletions(-) create mode 100644 src/applications/differential/view/resultstableview/DifferentialResultsTableView.php create mode 100644 src/applications/differential/view/resultstableview/__init__.php create mode 100644 webroot/rsrc/css/application/differential/results-table.css delete mode 100644 webroot/rsrc/css/application/differential/revision-detail.css diff --git a/scripts/celerity_mapper.php b/scripts/celerity_mapper.php index 7c190690e3..e21f89d0d5 100755 --- a/scripts/celerity_mapper.php +++ b/scripts/celerity_mapper.php @@ -88,7 +88,7 @@ $package_spec = array( 'differential.pkg.css' => array( 'differential-core-view-css', 'differential-changeset-view-css', - 'differential-revision-detail-css', + 'differential-results-table-css', 'differential-revision-history-css', 'differential-table-of-contents-css', 'differential-revision-comment-css', diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 640c179500..98e5e3997f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -571,6 +571,15 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/differential/local-commits-view.css', ), + 'differential-results-table-css' => + array( + 'uri' => '/res/470b474d/rsrc/css/application/differential/results-table.css', + 'type' => 'css', + 'requires' => + array( + ), + 'disk' => '/rsrc/css/application/differential/results-table.css', + ), 'differential-revision-add-comment-css' => array( 'uri' => '/res/849748d3/rsrc/css/application/differential/add-comment.css', @@ -598,15 +607,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/differential/revision-comment-list.css', ), - 'differential-revision-detail-css' => - array( - 'uri' => '/res/a838bf31/rsrc/css/application/differential/revision-detail.css', - 'type' => 'css', - 'requires' => - array( - ), - 'disk' => '/rsrc/css/application/differential/revision-detail.css', - ), 'differential-revision-history-css' => array( 'uri' => '/res/0d7d515d/rsrc/css/application/differential/revision-history.css', @@ -989,7 +989,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-show-field-details' => array( - 'uri' => '/res/1dfd4d10/rsrc/js/application/differential/behavior-show-field-details.js', + 'uri' => '/res/8d57f459/rsrc/js/application/differential/behavior-show-field-details.js', 'type' => 'js', 'requires' => array( @@ -2503,14 +2503,14 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/0c96375e/core.pkg.js', 'type' => 'js', ), - 'cf6f734a' => + '27683aba' => array( 'name' => 'differential.pkg.css', 'symbols' => array( 0 => 'differential-core-view-css', 1 => 'differential-changeset-view-css', - 2 => 'differential-revision-detail-css', + 2 => 'differential-results-table-css', 3 => 'differential-revision-history-css', 4 => 'differential-table-of-contents-css', 5 => 'differential-revision-comment-css', @@ -2522,7 +2522,7 @@ celerity_register_resource_map(array( 11 => 'differential-local-commits-view-css', 12 => 'inline-comment-summary-css', ), - 'uri' => '/res/pkg/cf6f734a/differential.pkg.css', + 'uri' => '/res/pkg/27683aba/differential.pkg.css', 'type' => 'css', ), 70509835 => @@ -2645,7 +2645,7 @@ celerity_register_resource_map(array( 'aphront-dialog-view-css' => '9c4e265b', 'aphront-error-view-css' => '9c4e265b', 'aphront-form-view-css' => '9c4e265b', - 'aphront-headsup-action-list-view-css' => 'cf6f734a', + 'aphront-headsup-action-list-view-css' => '27683aba', 'aphront-headsup-view-css' => '9c4e265b', 'aphront-list-filter-view-css' => '9c4e265b', 'aphront-pager-view-css' => '9c4e265b', @@ -2655,19 +2655,19 @@ celerity_register_resource_map(array( 'aphront-tokenizer-control-css' => '9c4e265b', 'aphront-tooltip-css' => '9c4e265b', 'aphront-typeahead-control-css' => '9c4e265b', - 'differential-changeset-view-css' => 'cf6f734a', - 'differential-core-view-css' => 'cf6f734a', + 'differential-changeset-view-css' => '27683aba', + 'differential-core-view-css' => '27683aba', 'differential-inline-comment-editor' => '70509835', - 'differential-local-commits-view-css' => 'cf6f734a', - 'differential-revision-add-comment-css' => 'cf6f734a', - 'differential-revision-comment-css' => 'cf6f734a', - 'differential-revision-comment-list-css' => 'cf6f734a', - 'differential-revision-detail-css' => 'cf6f734a', - 'differential-revision-history-css' => 'cf6f734a', - 'differential-table-of-contents-css' => 'cf6f734a', + 'differential-local-commits-view-css' => '27683aba', + 'differential-results-table-css' => '27683aba', + 'differential-revision-add-comment-css' => '27683aba', + 'differential-revision-comment-css' => '27683aba', + 'differential-revision-comment-list-css' => '27683aba', + 'differential-revision-history-css' => '27683aba', + 'differential-table-of-contents-css' => '27683aba', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'inline-comment-summary-css' => 'cf6f734a', + 'inline-comment-summary-css' => '27683aba', 'javelin-behavior' => '8a5de8a3', 'javelin-behavior-aphront-basic-tokenizer' => '97f65640', 'javelin-behavior-aphront-drag-and-drop' => '70509835', @@ -2721,7 +2721,7 @@ celerity_register_resource_map(array( 'maniphest-task-summary-css' => '7839ae2d', 'maniphest-transaction-detail-css' => '7839ae2d', 'phabricator-app-buttons-css' => '9c4e265b', - 'phabricator-content-source-view-css' => 'cf6f734a', + 'phabricator-content-source-view-css' => '27683aba', 'phabricator-core-buttons-css' => '9c4e265b', 'phabricator-core-css' => '9c4e265b', 'phabricator-directory-css' => '9c4e265b', @@ -2732,7 +2732,7 @@ celerity_register_resource_map(array( 'phabricator-keyboard-shortcut' => '0c96375e', 'phabricator-keyboard-shortcut-manager' => '0c96375e', 'phabricator-menu-item' => '0c96375e', - 'phabricator-object-selector-css' => 'cf6f734a', + 'phabricator-object-selector-css' => '27683aba', 'phabricator-paste-file-upload' => '0c96375e', 'phabricator-prefab' => '0c96375e', 'phabricator-project-tag-css' => '7839ae2d', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index da2e6b243f..ed7a44915d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -260,6 +260,7 @@ phutil_register_library_map(array( 'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/path', 'DifferentialPrimaryPaneView' => 'applications/differential/view/primarypane', 'DifferentialReplyHandler' => 'applications/differential/replyhandler', + 'DifferentialResultsTableView' => 'applications/differential/view/resultstableview', 'DifferentialRevertPlanFieldSpecification' => 'applications/differential/field/specification/revertplan', 'DifferentialReviewRequestMail' => 'applications/differential/mail/reviewrequest', 'DifferentialReviewedByFieldSpecification' => 'applications/differential/field/specification/reviewedby', @@ -1245,6 +1246,7 @@ phutil_register_library_map(array( 'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialPrimaryPaneView' => 'AphrontView', 'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler', + 'DifferentialResultsTableView' => 'AphrontView', 'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewRequestMail' => 'DifferentialMail', 'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification', @@ -1490,7 +1492,7 @@ phutil_register_library_map(array( 'PhabricatorDefaultFileStorageEngineSelector' => 'PhabricatorFileStorageEngineSelector', 'PhabricatorDefaultSearchEngineSelector' => 'PhabricatorSearchEngineSelector', 'PhabricatorDirectoryController' => 'PhabricatorController', - 'PhabricatorDirectoryMainController' => 'PhabricatorController', + 'PhabricatorDirectoryMainController' => 'PhabricatorDirectoryController', 'PhabricatorDisabledUserController' => 'PhabricatorAuthController', 'PhabricatorDraft' => 'PhabricatorDraftDAO', 'PhabricatorDraftDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php b/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php index f6adf8fa9f..f76c74ab69 100644 --- a/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php +++ b/src/applications/differential/field/specification/lint/DifferentialLintFieldSpecification.php @@ -32,33 +32,50 @@ final class DifferentialLintFieldSpecification } private function getLintExcuse() { - $excuse = $this->getDiffProperty('arc:lint-excuse'); - $excuse = phutil_escape_html($excuse); - $excuse = nl2br($excuse); - - $excuse_markup = ''; - if (strlen($excuse)) { - $excuse_markup = '

Explanation for failure(s):

'. - ''.$excuse.''; - } - return $excuse_markup; + return $this->getDiffProperty('arc:lint-excuse'); } public function renderValueForRevisionView() { $diff = $this->getDiff(); - $path_changesets = mpull($diff->loadChangesets(), 'getId', 'getFilename'); + $path_changesets = mpull($diff->loadChangesets(), 'getID', 'getFilename'); $lstar = DifferentialRevisionUpdateHistoryView::renderDiffLintStar($diff); $lmsg = DifferentialRevisionUpdateHistoryView::getDiffLintMessage($diff); $ldata = $this->getDiffProperty('arc:lint'); $ltail = null; - $have_details = false; + + $rows = array(); + + $rows[] = array( + 'style' => 'star', + 'name' => $lstar, + 'value' => $lmsg, + 'show' => true, + ); + + $excuse = $this->getLintExcuse(); + if ($excuse) { + $rows[] = array( + 'style' => 'excuse', + 'name' => 'Excuse', + 'value' => nl2br(phutil_escape_html($excuse)), + 'show' => true, + ); + } + + $show_limit = 10; + $hidden = array(); if ($ldata) { $ldata = igroup($ldata, 'path'); - $lint_messages = array(); foreach ($ldata as $path => $messages) { - $message_markup = array(); + + $rows[] = array( + 'style' => 'section', + 'name' => phutil_escape_html($path), + 'show' => $show_limit, + ); + foreach ($messages as $message) { $path = idx($message, 'path'); $line = idx($message, 'line'); @@ -69,7 +86,7 @@ final class DifferentialLintFieldSpecification $name = idx($message, 'name'); $description = idx($message, 'description'); - $line_link = phutil_escape_html($line); + $line_link = 'line '.phutil_escape_html($line); if (isset($path_changesets[$path])) { // TODO: Create standalone links for large diffs. Logic is in // DifferentialDiffTableOfContentsView::renderChangesetLink(). @@ -80,54 +97,104 @@ final class DifferentialLintFieldSpecification ), $line_link); } - if ($description != '') { - $have_details = true; + + if ($show_limit) { + --$show_limit; + $show = true; + } else { + $show = false; + if (empty($hidden[$severity])) { + $hidden[$severity] = 0; + } + $hidden[$severity]++; + } + + $rows[] = array( + 'style' => $this->getSeverityStyle($severity), + 'name' => phutil_escape_html(ucwords($severity)), + 'value' => hsprintf( + "(%s) %s at {$line_link}", + $code, + $name), + 'show' => $show, + ); + + if (strlen($description)) { + $rows[] = array( + 'style' => 'details', + 'value' => nl2br(phutil_escape_html($description)), + 'show' => false, + ); + if (empty($hidden['details'])) { + $hidden['details'] = 0; + } + $hidden['details']++; } - $message_markup[] = hsprintf( - '
  • '. - '%s (%s) %s '. - 'at line '.$line_link. - javelin_render_tag( - 'div', - array( - 'sigil' => 'differential-field-detail', - 'style' => 'display: none;', - ), - '%s'). - '
    • ', - $severity, - ucwords($severity), - $code, - $name, - $description); } - $lint_messages[] = - '
  • '. - 'Lint for '.phutil_escape_html($path).''. - ''. - '
    • '; } - $lexcuse = $this->getLintExcuse(); - $ltail = - '
    '. - $lexcuse. - ''. - '
    '; } - Javelin::initBehavior('differential-show-field-details'); - if ($have_details) { - $lmsg .= ' - '.javelin_render_tag( - 'a', - array( - 'href' => '#details', - 'sigil' => 'differential-show-field-details', - ), - 'Details'); - } + $show_string = $this->renderShowString($hidden); - return $lstar.' '.$lmsg.$ltail; + $view = new DifferentialResultsTableView(); + $view->setRows($rows); + $view->setShowMoreString($show_string); + + return $view->render(); } + + private function getSeverityStyle($severity) { + $map = array( + ArcanistLintSeverity::SEVERITY_ERROR => 'red', + ArcanistLintSeverity::SEVERITY_WARNING => 'yellow', + ArcanistLintSeverity::SEVERITY_AUTOFIX => 'yellow', + ArcanistLintSeverity::SEVERITY_ADVICE => 'yellow', + ); + return idx($map, $severity); + } + + private function renderShowString(array $hidden) { + if (!$hidden) { + return null; + } + + // Reorder hidden things by severity. + $hidden = array_select_keys( + $hidden, + array( + ArcanistLintSeverity::SEVERITY_ERROR, + ArcanistLintSeverity::SEVERITY_WARNING, + ArcanistLintSeverity::SEVERITY_AUTOFIX, + ArcanistLintSeverity::SEVERITY_ADVICE, + 'details', + )) + $hidden; + + $singular = array( + ArcanistLintSeverity::SEVERITY_ERROR => 'Error', + ArcanistLintSeverity::SEVERITY_WARNING => 'Warning', + ArcanistLintSeverity::SEVERITY_AUTOFIX => 'Auto-Fix', + ArcanistLintSeverity::SEVERITY_ADVICE => 'Advice', + 'details' => 'Detail', + ); + + $plural = array( + ArcanistLintSeverity::SEVERITY_ERROR => 'Errors', + ArcanistLintSeverity::SEVERITY_WARNING => 'Warnings', + ArcanistLintSeverity::SEVERITY_AUTOFIX => 'Auto-Fixes', + ArcanistLintSeverity::SEVERITY_ADVICE => 'Advice', + 'details' => 'Details', + ); + + $show = array(); + foreach ($hidden as $key => $value) { + if ($value == 1) { + $show[] = $value.' '.idx($singular, $key); + } else { + $show[] = $value.' '.idx($plural, $key); + } + } + + return "Show Full Lint Results (".implode(', ', $show).")"; + } + } diff --git a/src/applications/differential/field/specification/lint/__init__.php b/src/applications/differential/field/specification/lint/__init__.php index e6a63dd8d7..d8295f4821 100644 --- a/src/applications/differential/field/specification/lint/__init__.php +++ b/src/applications/differential/field/specification/lint/__init__.php @@ -6,10 +6,11 @@ +phutil_require_module('arcanist', 'lint/severity'); + phutil_require_module('phabricator', 'applications/differential/field/specification/base'); +phutil_require_module('phabricator', 'applications/differential/view/resultstableview'); phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory'); -phutil_require_module('phabricator', 'infrastructure/javelin/api'); -phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/field/specification/unit/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/unit/DifferentialUnitFieldSpecification.php index 133e58f96a..7d72734578 100644 --- a/src/applications/differential/field/specification/unit/DifferentialUnitFieldSpecification.php +++ b/src/applications/differential/field/specification/unit/DifferentialUnitFieldSpecification.php @@ -32,16 +32,7 @@ final class DifferentialUnitFieldSpecification } private function getUnitExcuse() { - $excuse = $this->getDiffProperty('arc:unit-excuse'); - $excuse = phutil_escape_html($excuse); - $excuse = nl2br($excuse); - - $excuse_markup = ''; - if (strlen($excuse)) { - $excuse_markup = '

    Explanation for failure(s):

    '. - ''.$excuse.''; - } - return $excuse_markup; + return $this->getDiffProperty('arc:unit-excuse'); } public function renderValueForRevisionView() { @@ -50,77 +41,145 @@ final class DifferentialUnitFieldSpecification $ustar = DifferentialRevisionUpdateHistoryView::renderDiffUnitStar($diff); $umsg = DifferentialRevisionUpdateHistoryView::getDiffUnitMessage($diff); - $postponed_count = 0; - $udata = $this->getDiffProperty('arc:unit'); - $utail = null; - $have_details = false; + $rows = array(); + $rows[] = array( + 'style' => 'star', + 'name' => $ustar, + 'value' => $umsg, + 'show' => true, + ); + + $excuse = $this->getUnitExcuse(); + if ($excuse) { + $rows[] = array( + 'style' => 'excuse', + 'name' => 'Excuse', + 'value' => nl2br(phutil_escape_html($excuse)), + 'show' => true, + ); + } + + $show_limit = 10; + $hidden = array(); + + $udata = $this->getDiffProperty('arc:unit'); if ($udata) { - $unit_messages = array(); + $sort_map = array( + ArcanistUnitTestResult::RESULT_BROKEN => 0, + ArcanistUnitTestResult::RESULT_FAIL => 1, + ArcanistUnitTestResult::RESULT_UNSOUND => 2, + ArcanistUnitTestResult::RESULT_SKIP => 3, + ArcanistUnitTestResult::RESULT_POSTPONED => 4, + ArcanistUnitTestResult::RESULT_PASS => 5, + ); + + foreach ($udata as $key => $test) { + $udata[$key]['sort'] = idx($sort_map, idx($test, 'result')); + } + $udata = isort($udata, 'sort'); + foreach ($udata as $test) { - $name = idx($test, 'name'); $result = idx($test, 'result'); - if ($result != DifferentialUnitTestResult::RESULT_POSTPONED && - $result != DifferentialUnitTestResult::RESULT_PASS) { - $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); - $userdata = phutil_utf8_shorten(idx($test, 'userdata'), 512); - $userdata = $engine->markupText($userdata); + $default_hide = false; + switch ($result) { + case ArcanistUnitTestResult::RESULT_POSTPONED: + case ArcanistUnitTestResult::RESULT_PASS: + $default_hide = true; + break; + } - if ($userdata != '') { - $have_details = true; + if ($show_limit && !$default_hide) { + --$show_limit; + $show = true; + } else { + $show = false; + if (empty($hidden[$result])) { + $hidden[$result] = 0; } + $hidden[$result]++; + } - $unit_messages[] = - '
  • '. - ''. - phutil_escape_html(ucwords($result)). - ''. - ' '. - phutil_escape_html($name). - javelin_render_tag( - 'div', - array( - 'sigil' => 'differential-field-detail', - 'style' => 'display: none;', - ), - $userdata). - '
    • '; + $rows[] = array( + 'style' => $this->getResultStyle($result), + 'name' => phutil_escape_html(ucwords($result)), + 'value' => phutil_escape_html(idx($test, 'name')), + 'show' => $show, + ); - } else if ($result == DifferentialUnitTestResult::RESULT_POSTPONED) { - $postponed_count++; + $userdata = idx($test, 'userdata'); + if ($userdata) { + $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine(); + $userdata = $engine->markupText($userdata); + $rows[] = array( + 'style' => 'details', + 'value' => $userdata, + 'show' => false, + ); + if (empty($hidden['details'])) { + $hidden['details'] = 0; + } + $hidden['details']++; } } - - $uexcuse = $this->getUnitExcuse(); - if ($unit_messages) { - $utail = - '
    '. - $uexcuse. - ''. - '
    '; - } } - if ($postponed_count > 0 && - $diff->getUnitStatus() == DifferentialUnitStatus::UNIT_POSTPONED) { - $umsg = $postponed_count.' '.$umsg; + $show_string = $this->renderShowString($hidden); + + $view = new DifferentialResultsTableView(); + $view->setRows($rows); + $view->setShowMoreString($show_string); + + return $view->render(); + } + + private function getResultStyle($result) { + $map = array( + ArcanistUnitTestResult::RESULT_PASS => 'green', + ArcanistUnitTestResult::RESULT_FAIL => 'red', + ArcanistUnitTestResult::RESULT_SKIP => 'blue', + ArcanistUnitTestResult::RESULT_BROKEN => 'red', + ArcanistUnitTestResult::RESULT_UNSOUND => 'yellow', + ArcanistUnitTestResult::RESULT_POSTPONED => 'blue', + ); + return idx($map, $result); + } + + private function renderShowString(array $hidden) { + if (!$hidden) { + return null; } - Javelin::initBehavior('differential-show-field-details'); - if ($have_details) { - $umsg .= ' - '.javelin_render_tag( - 'a', - array( - 'href' => '#details', - 'sigil' => 'differential-show-field-details', - ), - 'Details'); + // Reorder hidden things by severity. + $hidden = array_select_keys( + $hidden, + array( + ArcanistUnitTestResult::RESULT_BROKEN, + ArcanistUnitTestResult::RESULT_FAIL, + ArcanistUnitTestResult::RESULT_UNSOUND, + ArcanistUnitTestResult::RESULT_SKIP, + ArcanistUnitTestResult::RESULT_POSTPONED, + ArcanistUnitTestResult::RESULT_PASS, + 'details', + )) + $hidden; + + $noun = array( + ArcanistUnitTestResult::RESULT_BROKEN => 'Broken', + ArcanistUnitTestResult::RESULT_FAIL => 'Failed', + ArcanistUnitTestResult::RESULT_UNSOUND => 'Unsound', + ArcanistUnitTestResult::RESULT_SKIP => 'Skipped', + ArcanistUnitTestResult::RESULT_POSTPONED => 'Postponed', + ArcanistUnitTestResult::RESULT_PASS => 'Passed', + 'details' => 'Details', + ); + + $show = array(); + foreach ($hidden as $key => $value) { + $show[] = $value.' '.idx($noun, $key); } - return $ustar.' '.$umsg.$utail; + return "Show Full Unit Results (".implode(', ', $show).")"; } } diff --git a/src/applications/differential/field/specification/unit/__init__.php b/src/applications/differential/field/specification/unit/__init__.php index ad46eae787..fa65658b2e 100644 --- a/src/applications/differential/field/specification/unit/__init__.php +++ b/src/applications/differential/field/specification/unit/__init__.php @@ -6,13 +6,12 @@ -phutil_require_module('phabricator', 'applications/differential/constants/unitstatus'); -phutil_require_module('phabricator', 'applications/differential/constants/unittestresult'); +phutil_require_module('arcanist', 'unit/result'); + phutil_require_module('phabricator', 'applications/differential/field/specification/base'); +phutil_require_module('phabricator', 'applications/differential/view/resultstableview'); phutil_require_module('phabricator', 'applications/differential/view/revisionupdatehistory'); phutil_require_module('phabricator', 'applications/markup/engine'); -phutil_require_module('phabricator', 'infrastructure/javelin/api'); -phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/differential/view/resultstableview/DifferentialResultsTableView.php b/src/applications/differential/view/resultstableview/DifferentialResultsTableView.php new file mode 100644 index 0000000000..7b43473c0b --- /dev/null +++ b/src/applications/differential/view/resultstableview/DifferentialResultsTableView.php @@ -0,0 +1,131 @@ +rows = $rows; + return $this; + } + + public function setShowMoreString($show_more_string) { + $this->showMoreString = $show_more_string; + return $this; + } + + public function render() { + + $rows = array(); + + $any_hidden = false; + foreach ($this->rows as $row) { + + $style = idx($row, 'style'); + switch ($style) { + case 'section': + $cells = phutil_render_tag( + 'th', + array( + 'colspan' => 2, + ), + idx($row, 'name')); + break; + default: + $name = phutil_render_tag( + 'th', + array( + ), + idx($row, 'name')); + $value = phutil_render_tag( + 'td', + array( + ), + idx($row, 'value')); + $cells = $name.$value; + break; + } + + $show = idx($row, 'show'); + + $rows[] = javelin_render_tag( + 'tr', + array( + 'style' => $show ? null : 'display: none', + 'sigil' => $show ? null : 'differential-results-row-toggle', + 'class' => 'differential-results-row-'.$style, + ), + $cells); + + if (!$show) { + $any_hidden = true; + } + } + + if ($any_hidden) { + $show_more = javelin_render_tag( + 'a', + array( + 'href' => '#', + 'mustcapture' => true, + ), + $this->showMoreString); + + $hide_more = javelin_render_tag( + 'a', + array( + 'href' => '#', + 'mustcapture' => true, + ), + 'Hide'); + + $rows[] = javelin_render_tag( + 'tr', + array( + 'class' => 'differential-results-row-show', + 'sigil' => 'differential-results-row-show', + ), + ''.$show_more.''); + + $rows[] = javelin_render_tag( + 'tr', + array( + 'class' => 'differential-results-row-show', + 'sigil' => 'differential-results-row-hide', + 'style' => 'display: none', + ), + ''.$hide_more.''); + + Javelin::initBehavior('differential-show-field-details'); + } + + require_celerity_resource('differential-results-table-css'); + + return javelin_render_tag( + 'table', + array( + 'class' => 'differential-results-table', + 'sigil' => 'differential-results-table', + ), + implode("\n", $rows)); + } + + +} diff --git a/src/applications/differential/view/resultstableview/__init__.php b/src/applications/differential/view/resultstableview/__init__.php new file mode 100644 index 0000000000..ae0f10fb08 --- /dev/null +++ b/src/applications/differential/view/resultstableview/__init__.php @@ -0,0 +1,18 @@ +revision; diff --git a/webroot/rsrc/css/application/differential/results-table.css b/webroot/rsrc/css/application/differential/results-table.css new file mode 100644 index 0000000000..a74e0490eb --- /dev/null +++ b/webroot/rsrc/css/application/differential/results-table.css @@ -0,0 +1,79 @@ +/** + * @provides differential-results-table-css + */ + +table.differential-results-table { + border-collapse: separate; + border-spacing: 1px; + width: 100%; + font-size: 11px; + background: #fcfcec; +} + +.differential-results-table th { + font-weight: bold; + text-align: center; + white-space: nowrap; + vertical-align: middle; + padding: 2px 4px; + margin: 0; + width: 70px; +} + +.differential-results-table td { + padding: 2px 8px; + margin: 0; + vertical-align: middle; +} + +.differential-results-table tr.differential-results-row-star th, +.differential-results-table tr.differential-results-row-star td { + font-size: 12px; + font-weight: bold; + background: #dfdfcf; +} + + +.differential-results-table tr.differential-results-row-section th { + font-weight: bold; + padding-top: 4px; + text-align: left; +} + +.differential-results-table tr.differential-results-row-excuse th { + background: #3399ff; +} + +.differential-results-table tr.differential-results-row-excuse td { + padding-top: 1em; + padding-right: 1em; + padding-bottom: 1em; +} + +.differential-results-table tr.differential-results-row-red th { + background: #ff4422; +} + +.differential-results-table tr.differential-results-row-yellow th { + background: #ffdd66; +} + +.differential-results-table tr.differential-results-row-green th { + background: #22dd44; +} + +.differential-results-table tr.differential-results-row-blue th { + background: #88bbff; +} + + +.differential-results-table tr.differential-results-row-details td { + color: #888888; +} + +.differential-results-table tr.differential-results-row-show th { + padding: 4px; + color: #888888; + font-weight: bold; + background: #dfdfcf; +} diff --git a/webroot/rsrc/css/application/differential/revision-detail.css b/webroot/rsrc/css/application/differential/revision-detail.css deleted file mode 100644 index b4a20e1042..0000000000 --- a/webroot/rsrc/css/application/differential/revision-detail.css +++ /dev/null @@ -1,68 +0,0 @@ -/** - * @provides differential-revision-detail-css - */ - -.differential-unit-block, -.differential-lint-block { - padding: .5em; - background: #fcfcec; - margin: .5em 0; - font-size: 11px; - line-height: 1.5em; -} - -.differential-lint-block .lint-severity-warning { - background: #ffff66; - padding: 0 0.5em; - font-weight: bold; -} - -.differential-lint-block .lint-severity-error { - background: #ff3333; - padding: 0 0.5em; - font-weight: bold; -} - -.differential-lint-block .lint-file-block { -} - -.differential-lint-block li li { - margin-left: 1.5em; -} - -.differential-lint-block li li p { - margin-left: 1em; - color: #666666; -} - -.differential-unit-block li { - margin-left: 1.5em; -} - -.differential-unit-block li p { - margin-left: 1em; - color: #666666; -} - -.differential-unit-block .unit-test-result { - padding: 0; -} - -.differential-unit-block .unit-result-fail { - background: #ff3333; - padding: 0 0.5em; - font-weight: bold; -} - -.differential-unit-block .unit-result-unsound { - background: #cc33cc; - padding: 0 0.5em; - font-weight: bold; -} - -.differential-lint-block .lint-excuse, -.differential-unit-block .unit-excuse { - background: #808080; - color: #ffff66; - font-size: 12px; -} diff --git a/webroot/rsrc/js/application/differential/behavior-show-field-details.js b/webroot/rsrc/js/application/differential/behavior-show-field-details.js index af1abb26f8..b788b7a6d6 100644 --- a/webroot/rsrc/js/application/differential/behavior-show-field-details.js +++ b/webroot/rsrc/js/application/differential/behavior-show-field-details.js @@ -5,27 +5,36 @@ * javelin-dom */ -JX.behavior('differential-show-field-details', function() { +JX.behavior('differential-show-field-details', function(config) { JX.Stratcom.listen( 'click', - 'differential-show-field-details', + ['differential-results-row-show', 'tag:a'], function(e) { - var node = e.getNode('tag:td'); - var data = JX.Stratcom.getData(node); - var details = JX.DOM.scry( - node, - 'div', - 'differential-field-detail'); - for (var i=0; i < details.length; i++) { - if (!data.detailsShown) { - JX.DOM.show(details[i]); - } else { - JX.DOM.hide(details[i]); - } - } - data.detailsShown = !data.detailsShown; - e.kill(); + toggle(e, true); }); + JX.Stratcom.listen( + 'click', + ['differential-results-row-hide', 'tag:a'], + function(e) { + toggle(e, false); + }); + + function toggle(e, show) { + e.kill(); + + var f = show ? JX.DOM.show : JX.DOM.hide; + var g = show ? JX.DOM.hide : JX.DOM.show; + + var table = e.getNode('differential-results-table'); + var rows = JX.DOM.scry(table, 'tr', 'differential-results-row-toggle'); + for (var ii = 0; ii < rows.length; ii++) { + f(rows[ii]); + } + + g(JX.DOM.find(table, 'tr', 'differential-results-row-show')); + f(JX.DOM.find(table, 'tr', 'differential-results-row-hide')); + } + });