From 09cdf0de4897d8a072e88fe7f2e2a7a27a807064 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 19 Sep 2013 11:57:33 -0700 Subject: [PATCH] Modernize Diffusion browse views Summary: Broadly, I'm trying to modernize these views and fix UI and at least mitigate mobile problems. See discussion. Test Plan: See screenshots. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D7042 --- .../ConduitAPI_diffusion_tagsquery_Method.php | 3 +- .../controller/DiffusionBrowseController.php | 178 +++++++++++++++--- .../controller/DiffusionController.php | 69 +++---- .../DiffusionRepositoryController.php | 7 +- 4 files changed, 199 insertions(+), 58 deletions(-) diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php index 8d2448ff7e..037c1a6de6 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_tagsquery_Method.php @@ -26,7 +26,8 @@ final class ConduitAPI_diffusion_tagsquery_Method protected function getGitResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); $repository = $drequest->getRepository(); - $commit = $drequest->getCommit(); + $commit = $drequest->getRawCommit(); + $offset = $request->getValue('offset'); $limit = $request->getValue('limit'); diff --git a/src/applications/diffusion/controller/DiffusionBrowseController.php b/src/applications/diffusion/controller/DiffusionBrowseController.php index a2a73f3c44..6d5f320a29 100644 --- a/src/applications/diffusion/controller/DiffusionBrowseController.php +++ b/src/applications/diffusion/controller/DiffusionBrowseController.php @@ -29,22 +29,14 @@ final class DiffusionBrowseController extends DiffusionController { $content = array(); - if ($drequest->getTagContent()) { - $title = pht('Tag: %s', $drequest->getSymbolicCommit()); - - $tag_view = new AphrontPanelView(); - $tag_view->setHeader($title); - $tag_view->appendChild( - $this->markupText($drequest->getTagContent())); - - $content[] = $tag_view; - } + $content[] = $this->buildHeaderView($drequest); + $content[] = $this->buildActionView($drequest); + $content[] = $this->buildPropertyView($drequest); $content[] = $this->renderSearchForm(); if ($this->getRequest()->getStr('grep') != '') { $content[] = $this->renderSearchResults(); - } else { if (!$results->isValidResults()) { $empty_result = new DiffusionEmptyResultView(); @@ -52,9 +44,7 @@ final class DiffusionBrowseController extends DiffusionController { $empty_result->setDiffusionBrowseResultSet($results); $empty_result->setView($this->getRequest()->getStr('view')); $content[] = $empty_result; - } else { - $phids = array(); foreach ($results->getPaths() as $result) { $data = $result->getLastCommitData(); @@ -103,21 +93,20 @@ final class DiffusionBrowseController extends DiffusionController { $box, ); } - } - $nav = $this->buildSideNav('browse', false); - $nav->appendChild($content); - $crumbs = $this->buildCrumbs( array( 'branch' => true, 'path' => true, 'view' => 'browse', )); - $nav->setCrumbs($crumbs); + return $this->buildApplicationPage( - $nav, + array( + $crumbs, + $content, + ), array( 'device' => true, 'title' => array( @@ -146,16 +135,24 @@ final class DiffusionBrowseController extends DiffusionController { ->setLabel(pht('Search Here')) ->setName('grep') ->setValue($this->getRequest()->getStr('grep')) - ->setCaption(pht('Regular expression'))) + ->setCaption(pht('Enter a regular expression.'))) ->appendChild( id(new AphrontFormSubmitControl()) - ->setValue(pht('Grep'))); + ->setValue(pht('Search File Content'))); break; } $filter = new AphrontListFilterView(); $filter->appendChild($form); + if (!strlen($this->getRequest()->getStr('grep'))) { + $filter->setCollapsed( + pht('Show Search'), + pht('Hide Search'), + pht('Search for file content in this directory.'), + '#'); + } + return $filter; } @@ -249,7 +246,7 @@ final class DiffusionBrowseController extends DiffusionController { ->setNoDataString($no_data); return id(new AphrontPanelView()) - ->setHeader(pht('Search Results')) + ->setNoBackground(true) ->appendChild($table) ->appendChild($pager); } @@ -270,4 +267,141 @@ final class DiffusionBrowseController extends DiffusionController { return $text; } + private function buildHeaderView(DiffusionRequest $drequest) { + $viewer = $this->getRequest()->getUser(); + + $header = id(new PHUIHeaderView()) + ->setUser($viewer) + ->setHeader($this->renderPathLinks($drequest)) + ->setPolicyObject($drequest->getRepository()); + + return $header; + } + + private function buildActionView(DiffusionRequest $drequest) { + $viewer = $this->getRequest()->getUser(); + + $view = id(new PhabricatorActionListView()) + ->setUser($viewer); + + $history_uri = $drequest->generateURI( + array( + 'action' => 'history', + )); + + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('View History')) + ->setHref($history_uri) + ->setIcon('perflab')); + + $behind_head = $drequest->getRawCommit(); + $head_uri = $drequest->generateURI( + array( + 'commit' => '', + 'action' => 'browse', + )); + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Jump to HEAD')) + ->setHref($head_uri) + ->setIcon('home') + ->setDisabled(!$behind_head)); + + // TODO: Ideally, this should live in Owners and be event-triggered, but + // there's no reasonable object for it to react to right now. + + $owners_uri = id(new PhutilURI('/owners/view/search/')) + ->setQueryParams( + array( + 'repository' => $drequest->getCallsign(), + 'path' => '/'.$drequest->getPath(), + )); + + $view->addAction( + id(new PhabricatorActionView()) + ->setName(pht('Find Owners')) + ->setHref((string)$owners_uri) + ->setIcon('preview')); + + return $view; + } + + private function buildPropertyView(DiffusionRequest $drequest) { + $viewer = $this->getRequest()->getUser(); + + $view = id(new PhabricatorPropertyListView()) + ->setUser($viewer); + + $stable_commit = $drequest->getStableCommitName(); + $callsign = $drequest->getRepository()->getCallsign(); + + $view->addProperty( + pht('Commit'), + phutil_tag( + 'a', + array( + 'href' => $drequest->generateURI( + array( + 'action' => 'commit', + 'commit' => $stable_commit, + )), + ), + $drequest->getRepository()->formatCommitName($stable_commit))); + + if ($drequest->getTagContent()) { + $view->addProperty( + pht('Tag'), + $drequest->getSymbolicCommit()); + + $view->addSectionHeader(pht('Tag Content')); + $view->addTextContent($this->markupText($drequest->getTagContent())); + } + + return $view; + } + + private function renderPathLinks(DiffusionRequest $drequest) { + $path = $drequest->getPath(); + $path_parts = array_filter(explode('/', trim($path, '/'))); + + $links = array(); + if ($path_parts) { + $links[] = phutil_tag( + 'a', + array( + 'href' => $drequest->generateURI( + array( + 'action' => 'browse', + 'path' => '', + )), + ), + 'r'.$drequest->getRepository()->getCallsign().'/'); + $accum = ''; + $last_key = last_key($path_parts); + foreach ($path_parts as $key => $part) { + $links[] = ' '; + $accum .= '/'.$part; + if ($key === $last_key) { + $links[] = $part; + } else { + $links[] = phutil_tag( + 'a', + array( + 'href' => $drequest->generateURI( + array( + 'action' => 'browse', + 'path' => $accum, + )), + ), + $part.'/'); + } + } + } else { + $links[] = 'r'.$drequest->getRepository()->getCallsign().'/'; + } + + return $links; + } + } diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php index f2ad87c859..fbb7868969 100644 --- a/src/applications/diffusion/controller/DiffusionController.php +++ b/src/applications/diffusion/controller/DiffusionController.php @@ -259,43 +259,44 @@ abstract class DiffusionController extends PhabricatorController { $crumb = id(new PhabricatorCrumbView()) ->setName($view_name); - if (!strlen($path)) { + + if ($view == 'browse') { $crumb_list[] = $crumb; - } else { - - $crumb->setHref($drequest->generateURI( - array( - 'path' => '', - ) + $uri_params)); - $crumb_list[] = $crumb; - - $path_parts = explode('/', $path); - do { - $last = array_pop($path_parts); - } while ($last == ''); - - $path_sections = array(); - $thus_far = ''; - foreach ($path_parts as $path_part) { - $thus_far .= $path_part.'/'; - $path_sections[] = '/'; - $path_sections[] = phutil_tag( - 'a', - array( - 'href' => $drequest->generateURI( - array( - 'path' => $thus_far, - ) + $uri_params), - ), - $path_part); - } - - $path_sections[] = '/'.$last; - - $crumb_list[] = id(new PhabricatorCrumbView()) - ->setName($path_sections); + return $crumb_list; } + $crumb->setHref($drequest->generateURI( + array( + 'path' => '', + ) + $uri_params)); + $crumb_list[] = $crumb; + + $path_parts = explode('/', $path); + do { + $last = array_pop($path_parts); + } while (count($path_parts) && $last == ''); + + $path_sections = array(); + $thus_far = ''; + foreach ($path_parts as $path_part) { + $thus_far .= $path_part.'/'; + $path_sections[] = '/'; + $path_sections[] = phutil_tag( + 'a', + array( + 'href' => $drequest->generateURI( + array( + 'path' => $thus_far, + ) + $uri_params), + ), + $path_part); + } + + $path_sections[] = '/'.$last; + + $crumb_list[] = id(new PhabricatorCrumbView()) + ->setName($path_sections); + $last_crumb = array_pop($crumb_list); if ($raw_commit) { diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index 5e0b6a606c..8d0c3faf7f 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -238,7 +238,11 @@ final class DiffusionRepositoryController extends DiffusionController { $tags = DiffusionRepositoryTag::newFromConduit( $this->callConduitWithDiffusionRequest( 'diffusion.tagsquery', - array('limit' => $tag_limit + 1))); + array( + // On the home page, we want to find tags on any branch. + 'commit' => null, + 'limit' => $tag_limit + 1, + ))); } catch (ConduitException $e) { if ($e->getMessage() != 'ERR-UNSUPPORTED-VCS') { throw $e; @@ -271,6 +275,7 @@ final class DiffusionRepositoryController extends DiffusionController { $panel = new AphrontPanelView(); $panel->setHeader(pht('Tags')); + $panel->setNoBackground(true); if ($more_tags) { $panel->setCaption(pht('Showing the %d most recent tags.', $tag_limit));