From b28ceafa382d2be2f90090c214a49206926016ea Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 1 Jul 2013 12:37:54 -0700 Subject: [PATCH] Update Differential diff view Summary: Ref T603. - Primarily, this gets rid of a `DifferentialRevisionListData` callsite. - Also modernize and clean up some UI stuff. Test Plan: {F48260} {F48261} Reviewers: btrahan, chad Reviewed By: chad CC: aran Maniphest Tasks: T603 Differential Revision: https://secure.phabricator.com/D6334 --- src/__celerity_resource_map__.php | 90 +++++++++---------- .../DifferentialDiffViewController.php | 87 +++++++++--------- webroot/rsrc/css/aphront/panel-view.css | 5 -- 3 files changed, 89 insertions(+), 93 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 6db78bfb48..2d1ba4b3af 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -859,7 +859,7 @@ celerity_register_resource_map(array( ), 'aphront-panel-view-css' => array( - 'uri' => '/res/4031ea99/rsrc/css/aphront/panel-view.css', + 'uri' => '/res/f55024c3/rsrc/css/aphront/panel-view.css', 'type' => 'css', 'requires' => array( @@ -4073,7 +4073,7 @@ celerity_register_resource_map(array( ), array( 'packages' => array( - 'b939ab89' => + '60bbab00' => array( 'name' => 'core.pkg.css', 'symbols' => @@ -4121,7 +4121,7 @@ celerity_register_resource_map(array( 40 => 'phabricator-property-list-view-css', 41 => 'phabricator-tag-view-css', ), - 'uri' => '/res/pkg/b939ab89/core.pkg.css', + 'uri' => '/res/pkg/60bbab00/core.pkg.css', 'type' => 'css', ), 'f2ad0683' => @@ -4315,16 +4315,16 @@ celerity_register_resource_map(array( 'reverse' => array( 'aphront-attached-file-view-css' => 'adc3c36d', - 'aphront-dialog-view-css' => 'b939ab89', - 'aphront-error-view-css' => 'b939ab89', - 'aphront-form-view-css' => 'b939ab89', - 'aphront-list-filter-view-css' => 'b939ab89', - 'aphront-pager-view-css' => 'b939ab89', - 'aphront-panel-view-css' => 'b939ab89', - 'aphront-table-view-css' => 'b939ab89', - 'aphront-tokenizer-control-css' => 'b939ab89', - 'aphront-tooltip-css' => 'b939ab89', - 'aphront-typeahead-control-css' => 'b939ab89', + 'aphront-dialog-view-css' => '60bbab00', + 'aphront-error-view-css' => '60bbab00', + 'aphront-form-view-css' => '60bbab00', + 'aphront-list-filter-view-css' => '60bbab00', + 'aphront-pager-view-css' => '60bbab00', + 'aphront-panel-view-css' => '60bbab00', + 'aphront-table-view-css' => '60bbab00', + 'aphront-tokenizer-control-css' => '60bbab00', + 'aphront-tooltip-css' => '60bbab00', + 'aphront-typeahead-control-css' => '60bbab00', 'differential-changeset-view-css' => 'dd27a69b', 'differential-core-view-css' => 'dd27a69b', 'differential-inline-comment-editor' => '9488bb69', @@ -4338,7 +4338,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => 'dd27a69b', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'global-drag-and-drop-css' => 'b939ab89', + 'global-drag-and-drop-css' => '60bbab00', 'inline-comment-summary-css' => 'dd27a69b', 'javelin-aphlict' => 'f2ad0683', 'javelin-behavior' => 'a9f14d76', @@ -4412,55 +4412,55 @@ celerity_register_resource_map(array( 'javelin-util' => 'a9f14d76', 'javelin-vector' => 'a9f14d76', 'javelin-workflow' => 'a9f14d76', - 'lightbox-attachment-css' => 'b939ab89', + 'lightbox-attachment-css' => '60bbab00', 'maniphest-task-summary-css' => 'adc3c36d', 'maniphest-transaction-detail-css' => 'adc3c36d', - 'phabricator-action-list-view-css' => 'b939ab89', - 'phabricator-application-launch-view-css' => 'b939ab89', + 'phabricator-action-list-view-css' => '60bbab00', + 'phabricator-application-launch-view-css' => '60bbab00', 'phabricator-busy' => 'f2ad0683', 'phabricator-content-source-view-css' => 'dd27a69b', - 'phabricator-core-css' => 'b939ab89', - 'phabricator-crumbs-view-css' => 'b939ab89', + 'phabricator-core-css' => '60bbab00', + 'phabricator-crumbs-view-css' => '60bbab00', 'phabricator-drag-and-drop-file-upload' => '9488bb69', 'phabricator-dropdown-menu' => 'f2ad0683', 'phabricator-file-upload' => 'f2ad0683', - 'phabricator-filetree-view-css' => 'b939ab89', - 'phabricator-flag-css' => 'b939ab89', - 'phabricator-form-view-css' => 'b939ab89', - 'phabricator-header-view-css' => 'b939ab89', + 'phabricator-filetree-view-css' => '60bbab00', + 'phabricator-flag-css' => '60bbab00', + 'phabricator-form-view-css' => '60bbab00', + 'phabricator-header-view-css' => '60bbab00', 'phabricator-hovercard' => 'f2ad0683', - 'phabricator-jump-nav' => 'b939ab89', + 'phabricator-jump-nav' => '60bbab00', 'phabricator-keyboard-shortcut' => 'f2ad0683', 'phabricator-keyboard-shortcut-manager' => 'f2ad0683', - 'phabricator-main-menu-view' => 'b939ab89', + 'phabricator-main-menu-view' => '60bbab00', 'phabricator-menu-item' => 'f2ad0683', - 'phabricator-nav-view-css' => 'b939ab89', + 'phabricator-nav-view-css' => '60bbab00', 'phabricator-notification' => 'f2ad0683', - 'phabricator-notification-css' => 'b939ab89', - 'phabricator-notification-menu-css' => 'b939ab89', - 'phabricator-object-item-list-view-css' => 'b939ab89', + 'phabricator-notification-css' => '60bbab00', + 'phabricator-notification-menu-css' => '60bbab00', + 'phabricator-object-item-list-view-css' => '60bbab00', 'phabricator-object-selector-css' => 'dd27a69b', 'phabricator-phtize' => 'f2ad0683', 'phabricator-prefab' => 'f2ad0683', 'phabricator-project-tag-css' => 'adc3c36d', - 'phabricator-property-list-view-css' => 'b939ab89', - 'phabricator-remarkup-css' => 'b939ab89', + 'phabricator-property-list-view-css' => '60bbab00', + 'phabricator-remarkup-css' => '60bbab00', 'phabricator-shaped-request' => '9488bb69', - 'phabricator-side-menu-view-css' => 'b939ab89', - 'phabricator-standard-page-view' => 'b939ab89', - 'phabricator-tag-view-css' => 'b939ab89', + 'phabricator-side-menu-view-css' => '60bbab00', + 'phabricator-standard-page-view' => '60bbab00', + 'phabricator-tag-view-css' => '60bbab00', 'phabricator-textareautils' => 'f2ad0683', 'phabricator-tooltip' => 'f2ad0683', - 'phabricator-transaction-view-css' => 'b939ab89', - 'phabricator-zindex-css' => 'b939ab89', - 'phui-button-css' => 'b939ab89', - 'phui-form-css' => 'b939ab89', - 'phui-icon-view-css' => 'b939ab89', - 'phui-spacing-css' => 'b939ab89', - 'sprite-apps-large-css' => 'b939ab89', - 'sprite-gradient-css' => 'b939ab89', - 'sprite-icons-css' => 'b939ab89', - 'sprite-menu-css' => 'b939ab89', - 'syntax-highlighting-css' => 'b939ab89', + 'phabricator-transaction-view-css' => '60bbab00', + 'phabricator-zindex-css' => '60bbab00', + 'phui-button-css' => '60bbab00', + 'phui-form-css' => '60bbab00', + 'phui-icon-view-css' => '60bbab00', + 'phui-spacing-css' => '60bbab00', + 'sprite-apps-large-css' => '60bbab00', + 'sprite-gradient-css' => '60bbab00', + 'sprite-icons-css' => '60bbab00', + 'sprite-menu-css' => '60bbab00', + 'syntax-highlighting-css' => '60bbab00', ), )); diff --git a/src/applications/differential/controller/DifferentialDiffViewController.php b/src/applications/differential/controller/DifferentialDiffViewController.php index 6503ec2087..b97bbb92aa 100644 --- a/src/applications/differential/controller/DifferentialDiffViewController.php +++ b/src/applications/differential/controller/DifferentialDiffViewController.php @@ -10,6 +10,7 @@ final class DifferentialDiffViewController extends DifferentialController { public function processRequest() { $request = $this->getRequest(); + $viewer = $request->getUser(); $diff = id(new DifferentialDiff())->load($this->id); if (!$diff) { @@ -17,31 +18,19 @@ final class DifferentialDiffViewController extends DifferentialController { } if ($diff->getRevisionID()) { - $top_panel = new AphrontPanelView(); - $top_panel->setWidth(AphrontPanelView::WIDTH_WIDE); - $link = phutil_tag( - 'a', - array( - 'href' => PhabricatorEnv::getURI('/D'.$diff->getRevisionID()), - ), - 'D'.$diff->getRevisionID()); - $top_panel->appendChild(phutil_tag( - 'h1', - array(), - pht('This diff belongs to revision %s', $link))); + $top_part = id(new AphrontErrorView()) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->appendChild( + pht( + 'This diff belongs to revision %s.', + phutil_tag( + 'a', + array( + 'href' => '/D'.$diff->getRevisionID(), + ), + 'D'.$diff->getRevisionID()))); } else { - $action_panel = new AphrontPanelView(); - $action_panel->setHeader('Preview Diff'); - $action_panel->setWidth(AphrontPanelView::WIDTH_WIDE); - $action_panel->appendChild(hsprintf( - '

%s

', - pht( - 'Review the diff for correctness. When you are satisfied, either '. - 'create a new revision or update '. - 'an existing revision.', - hsprintf('')))); - - // TODO: implmenent optgroup support in AphrontFormSelectControl? + // TODO: implement optgroup support in AphrontFormSelectControl? $select = array(); $select[] = hsprintf('', pht('Create New Revision')); $select[] = hsprintf( @@ -49,10 +38,11 @@ final class DifferentialDiffViewController extends DifferentialController { pht('Create a new Revision...')); $select[] = hsprintf(''); - $revision_data = new DifferentialRevisionListData( - DifferentialRevisionListData::QUERY_OPEN_OWNED, - array($request->getUser()->getPHID())); - $revisions = $revision_data->loadRevisions(); + $revisions = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withAuthors(array($viewer->getPHID())) + ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) + ->execute(); if ($revisions) { $select[] = hsprintf( @@ -64,7 +54,7 @@ final class DifferentialDiffViewController extends DifferentialController { array( 'value' => $revision->getID(), ), - $revision->getTitle()); + 'D'.$revision->getID().' '.$revision->getTitle()); } $select[] = hsprintf(''); } @@ -74,12 +64,16 @@ final class DifferentialDiffViewController extends DifferentialController { array('name' => 'revisionID'), $select); - $action_form = new AphrontFormView(); - $action_form + $form = id(new AphrontFormView()) ->setUser($request->getUser()) ->setAction('/differential/revision/edit/') ->addHiddenInput('diffID', $diff->getID()) ->addHiddenInput('viaDiffView', 1) + ->setFlexible(true) + ->appendRemarkupInstructions( + pht( + 'Review the diff for correctness. When you are satisfied, either '. + '**create a new revision** or **update an existing revision**.')) ->appendChild( id(new AphrontFormMarkupControl()) ->setLabel(pht('Attach To')) @@ -88,9 +82,7 @@ final class DifferentialDiffViewController extends DifferentialController { id(new AphrontFormSubmitControl()) ->setValue(pht('Continue'))); - $action_panel->appendChild($action_form); - - $top_panel = $action_panel; + $top_part = $form; } $props = id(new DifferentialDiffProperty())->loadAllWhere( @@ -120,6 +112,9 @@ final class DifferentialDiffViewController extends DifferentialController { } } + $property_head = id(new PhabricatorHeaderView()) + ->setHeader(pht('Properties')); + $property_view = new PhabricatorPropertyListView(); foreach ($dict as $key => $value) { $property_view->addProperty($key, $value); @@ -147,17 +142,23 @@ final class DifferentialDiffViewController extends DifferentialController { ->setTitle(pht('Diff %d', $diff->getID())) ->setUser($request->getUser()); - return $this->buildStandardPageResponse( - id(new DifferentialPrimaryPaneView()) - ->appendChild( - array( - $top_panel->render(), - $property_view, - $table_of_contents->render(), - $details->render(), - )), + $crumbs = $this->buildApplicationCrumbs(); + $crumbs->addCrumb( + id(new PhabricatorCrumbView()) + ->setName(pht('Diff %d', $diff->getID()))); + + return $this->buildApplicationPage( + array( + $crumbs, + $top_part, + $property_head, + $property_view, + $table_of_contents, + $details, + ), array( 'title' => pht('Diff View'), + 'dust' => true, )); } diff --git a/webroot/rsrc/css/aphront/panel-view.css b/webroot/rsrc/css/aphront/panel-view.css index d634dae299..a5bd4d39ea 100644 --- a/webroot/rsrc/css/aphront/panel-view.css +++ b/webroot/rsrc/css/aphront/panel-view.css @@ -52,11 +52,6 @@ float: right; } -.aphront-panel-view p.aphront-panel-instructions { - margin: .5em 2em .75em; - font-size: 13px; -} - .device-desktop .aphront-panel-width-form { width: 720px; margin-right: auto;