From 57f9450bcfc0f173bb497aaf97bf04b1f559019e Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 12 Jul 2020 14:07:17 -0700 Subject: [PATCH] Improve desktop and mobile layouts for new "CommitGridView" Summary: Ref T13552. The current layout doesn't work particularly well on desktops or devices. We have some device/desktop table layout code, but it isn't generic. We also have property list layout code, but it isn't generic either. Provide generic layout elements ("Fuel", from "Phabricator UI Layout" to "PHUIL"?) and narrowly specialize their display behavior. Then swap the ListItemView stuff to use it. Test Plan: Saw slightly better responsive behavior: {F7637457} Maniphest Tasks: T13552 Differential Revision: https://secure.phabricator.com/D21418 --- resources/celerity/map.php | 4 ++ src/__phutil_library_map__.php | 14 ++++ .../view/DiffusionCommitGraphView.php | 20 +++--- src/view/fuel/FuelComponentView.php | 35 +++++++++ src/view/fuel/FuelGridCellView.php | 28 ++++++++ src/view/fuel/FuelGridRowView.php | 32 +++++++++ src/view/fuel/FuelGridView.php | 41 +++++++++++ src/view/fuel/FuelMapItemView.php | 54 ++++++++++++++ src/view/fuel/FuelMapView.php | 45 ++++++++++++ src/view/fuel/FuelView.php | 10 +++ src/view/phui/PHUIObjectItemView.php | 29 +++++--- webroot/rsrc/css/fuel/fuel-grid.css | 29 ++++++++ webroot/rsrc/css/fuel/fuel-map.css | 71 +++++++++++++++++++ 13 files changed, 392 insertions(+), 20 deletions(-) create mode 100644 src/view/fuel/FuelComponentView.php create mode 100644 src/view/fuel/FuelGridCellView.php create mode 100644 src/view/fuel/FuelGridRowView.php create mode 100644 src/view/fuel/FuelGridView.php create mode 100644 src/view/fuel/FuelMapItemView.php create mode 100644 src/view/fuel/FuelMapView.php create mode 100644 src/view/fuel/FuelView.php create mode 100644 webroot/rsrc/css/fuel/fuel-grid.css create mode 100644 webroot/rsrc/css/fuel/fuel-map.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index fb63b68822..447d96d7e1 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -121,6 +121,8 @@ return array( 'rsrc/css/font/font-awesome.css' => '3883938a', 'rsrc/css/font/font-lato.css' => '23631304', 'rsrc/css/font/phui-font-icon-base.css' => '303c9b87', + 'rsrc/css/fuel/fuel-grid.css' => 'd87031e7', + 'rsrc/css/fuel/fuel-map.css' => 'd6e31510', 'rsrc/css/layout/phabricator-source-code-view.css' => '03d7ac28', 'rsrc/css/phui/button/phui-button-bar.css' => 'a4aa75c4', 'rsrc/css/phui/button/phui-button-simple.css' => '1ff278aa', @@ -574,6 +576,8 @@ return array( 'diviner-shared-css' => '4bd263b0', 'font-fontawesome' => '3883938a', 'font-lato' => '23631304', + 'fuel-grid-css' => 'd87031e7', + 'fuel-map-css' => 'd6e31510', 'global-drag-and-drop-css' => '1d2713a4', 'harbormaster-css' => '8dfe16b2', 'herald-css' => '648d39e2', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9828fca099..1578a69868 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1305,6 +1305,13 @@ phutil_register_library_map(array( 'FlagDeleteConduitAPIMethod' => 'applications/flag/conduit/FlagDeleteConduitAPIMethod.php', 'FlagEditConduitAPIMethod' => 'applications/flag/conduit/FlagEditConduitAPIMethod.php', 'FlagQueryConduitAPIMethod' => 'applications/flag/conduit/FlagQueryConduitAPIMethod.php', + 'FuelComponentView' => 'view/fuel/FuelComponentView.php', + 'FuelGridCellView' => 'view/fuel/FuelGridCellView.php', + 'FuelGridRowView' => 'view/fuel/FuelGridRowView.php', + 'FuelGridView' => 'view/fuel/FuelGridView.php', + 'FuelMapItemView' => 'view/fuel/FuelMapItemView.php', + 'FuelMapView' => 'view/fuel/FuelMapView.php', + 'FuelView' => 'view/fuel/FuelView.php', 'FundBacker' => 'applications/fund/storage/FundBacker.php', 'FundBackerCart' => 'applications/fund/phortune/FundBackerCart.php', 'FundBackerEditor' => 'applications/fund/editor/FundBackerEditor.php', @@ -7439,6 +7446,13 @@ phutil_register_library_map(array( 'FlagDeleteConduitAPIMethod' => 'FlagConduitAPIMethod', 'FlagEditConduitAPIMethod' => 'FlagConduitAPIMethod', 'FlagQueryConduitAPIMethod' => 'FlagConduitAPIMethod', + 'FuelComponentView' => 'FuelView', + 'FuelGridCellView' => 'FuelView', + 'FuelGridRowView' => 'FuelView', + 'FuelGridView' => 'FuelComponentView', + 'FuelMapItemView' => 'AphrontView', + 'FuelMapView' => 'FuelComponentView', + 'FuelView' => 'AphrontView', 'FundBacker' => array( 'FundDAO', 'PhabricatorPolicyInterface', diff --git a/src/applications/diffusion/view/DiffusionCommitGraphView.php b/src/applications/diffusion/view/DiffusionCommitGraphView.php index 124a94fc69..6682e1bf66 100644 --- a/src/applications/diffusion/view/DiffusionCommitGraphView.php +++ b/src/applications/diffusion/view/DiffusionCommitGraphView.php @@ -164,21 +164,23 @@ final class DiffusionCommitGraphView $this->addAuditAction($item_view, $hash); if ($show_auditors) { - $auditor_list = $item_view->newPropertyList(); + $auditor_list = $item_view->newMapView(); if ($commit) { $auditors = $this->newAuditorList($commit, $handles); - $auditor_list->addProperty(pht('Auditors'), $auditors); + $auditor_list->newItem() + ->setName(pht('Auditors')) + ->setValue($auditors); } } - $property_list = $item_view->newPropertyList(); + $property_list = $item_view->newMapView(); if ($commit) { $author_view = $this->getCommitAuthorView($commit); if ($author_view) { - $property_list->addProperty( - pht('Author'), - $this->getCommitAuthorView($commit)); + $property_list->newItem() + ->setName(pht('Author')) + ->setValue($author_view); } } @@ -189,9 +191,9 @@ final class DiffusionCommitGraphView $revision = head($revisions); $handle = $handles[$revision->getPHID()]; - $property_list->addProperty( - pht('Revision'), - $handle->renderLink()); + $property_list->newItem() + ->setName(pht('Revision')) + ->setValue($handle->renderLink()); } } } diff --git a/src/view/fuel/FuelComponentView.php b/src/view/fuel/FuelComponentView.php new file mode 100644 index 0000000000..dda96a7bd5 --- /dev/null +++ b/src/view/fuel/FuelComponentView.php @@ -0,0 +1,35 @@ +classes[] = $class; + return $this; + } + + private function getClasses() { + return $this->classes; + } + + final protected function newComponentTag( + $tag, + array $attributes, + $content) { + + $classes = $this->getClasses(); + if (isset($attributes['class'])) { + $classes[] = $attributes['class']; + } + + if ($classes) { + $classes = implode(' ', $classes); + $attributes['class'] = $classes; + } + + return javelin_tag($tag, $attributes, $content); + } + +} diff --git a/src/view/fuel/FuelGridCellView.php b/src/view/fuel/FuelGridCellView.php new file mode 100644 index 0000000000..edb6391843 --- /dev/null +++ b/src/view/fuel/FuelGridCellView.php @@ -0,0 +1,28 @@ +content = $content; + return $this; + } + + public function getContent() { + return $this->content; + } + + public function render() { + $content = $this->getContent(); + + return phutil_tag( + 'div', + array( + 'class' => 'fuel-grid-cell', + ), + $content); + } + +} diff --git a/src/view/fuel/FuelGridRowView.php b/src/view/fuel/FuelGridRowView.php new file mode 100644 index 0000000000..f8fd8b73d7 --- /dev/null +++ b/src/view/fuel/FuelGridRowView.php @@ -0,0 +1,32 @@ +cells[] = $cell; + return $cell; + } + + public function render() { + $cells = $this->cells; + + $classes = array(); + $classes[] = 'fuel-grid-row'; + + $classes[] = sprintf( + 'fuel-grid-cell-count-%d', + count($cells)); + + return phutil_tag( + 'div', + array( + 'class' => implode(' ', $classes), + ), + $cells); + } + +} diff --git a/src/view/fuel/FuelGridView.php b/src/view/fuel/FuelGridView.php new file mode 100644 index 0000000000..105a7ac6b4 --- /dev/null +++ b/src/view/fuel/FuelGridView.php @@ -0,0 +1,41 @@ +rows[] = $row; + return $row; + } + + public function render() { + require_celerity_resource('fuel-grid-css'); + + $rows = $this->rows; + + $body = phutil_tag( + 'div', + array( + 'class' => 'fuel-grid-body', + ), + $rows); + + $grid = phutil_tag( + 'div', + array( + 'class' => 'fuel-grid', + ), + $body); + + return $this->newComponentTag( + 'div', + array( + 'class' => 'fuel-grid-component', + ), + $grid); + } + +} diff --git a/src/view/fuel/FuelMapItemView.php b/src/view/fuel/FuelMapItemView.php new file mode 100644 index 0000000000..abf64c1d84 --- /dev/null +++ b/src/view/fuel/FuelMapItemView.php @@ -0,0 +1,54 @@ +name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setValue($value) { + $this->value = $value; + return $this; + } + + public function getValue() { + return $this->value; + } + + public function render() { + $value = $this->getValue(); + + $view = array(); + + $view[] = phutil_tag( + 'div', + array( + 'class' => 'fuel-map-name', + ), + $this->getName()); + + $view[] = phutil_tag( + 'div', + array( + 'class' => 'fuel-map-value', + ), + $value); + + return phutil_tag( + 'div', + array( + 'class' => 'fuel-map-pair', + ), + $view); + } + +} diff --git a/src/view/fuel/FuelMapView.php b/src/view/fuel/FuelMapView.php new file mode 100644 index 0000000000..d3a677a99f --- /dev/null +++ b/src/view/fuel/FuelMapView.php @@ -0,0 +1,45 @@ +items[] = $item; + return $item; + } + + public function render() { + require_celerity_resource('fuel-map-css'); + + $items = $this->items; + + if (!$items) { + return null; + } + + $body = phutil_tag( + 'div', + array( + 'class' => 'fuel-map-items', + ), + $items); + + $map = phutil_tag( + 'div', + array( + 'class' => 'fuel-map', + ), + $body); + + return $this->newComponentTag( + 'div', + array( + 'class' => 'fuel-map-component', + ), + $map); + } + +} diff --git a/src/view/fuel/FuelView.php b/src/view/fuel/FuelView.php new file mode 100644 index 0000000000..f0e6b795ec --- /dev/null +++ b/src/view/fuel/FuelView.php @@ -0,0 +1,10 @@ +propertyLists[] = $list; + public function newMapView() { + $list = id(new FuelMapView()) + ->addClass('fuel-map-property-list'); + $this->mapViews[] = $list; return $list; } @@ -612,12 +613,18 @@ final class PHUIObjectItemView extends AphrontTagView { ''); } - $property_lists = null; - if ($this->propertyLists) { - $property_lists[] = phutil_tag( - 'div', - array(), - $this->propertyLists); + $map_views = null; + if ($this->mapViews) { + $grid = id(new FuelGridView()) + ->addClass('fuel-grid-property-list'); + + $row = $grid->newRow(); + foreach ($this->mapViews as $map_view) { + $row->newCell() + ->setContent($map_view); + } + + $map_views = $grid; } $content = phutil_tag( @@ -628,7 +635,7 @@ final class PHUIObjectItemView extends AphrontTagView { array( $subhead, $attrs, - $property_lists, + $map_views, $this->renderChildren(), )); diff --git a/webroot/rsrc/css/fuel/fuel-grid.css b/webroot/rsrc/css/fuel/fuel-grid.css new file mode 100644 index 0000000000..5014bc5cb4 --- /dev/null +++ b/webroot/rsrc/css/fuel/fuel-grid.css @@ -0,0 +1,29 @@ +/** + * @provides fuel-grid-css + */ + +.device-desktop .fuel-grid { + display: table; + table-layout: fixed; +} + +.device-desktop .fuel-grid-body { + display: table-row-group; +} + +.device-desktop .fuel-grid-row { + display: table-row; +} + +.device-desktop .fuel-grid-cell { + display: table-cell; + vertical-align: top; +} + +.device-desktop .fuel-grid-cell-count-2 > .fuel-grid-cell { + width: 50%; +} + +.fuel-grid-property-list > .fuel-grid { + width: 100%; +} diff --git a/webroot/rsrc/css/fuel/fuel-map.css b/webroot/rsrc/css/fuel/fuel-map.css new file mode 100644 index 0000000000..b878689cc0 --- /dev/null +++ b/webroot/rsrc/css/fuel/fuel-map.css @@ -0,0 +1,71 @@ +/** + * @provides fuel-map-css + */ + +.device-desktop .fuel-map { + display: table; + table-layout: fixed; +} + +.device-desktop .fuel-map-items { + display: table-row-group; +} + +.device-desktop .fuel-map-pair { + display: table-row; +} + +.device-desktop .fuel-map-name, +.device-desktop .fuel-map-value { + display: table-cell; + vertical-align: top; +} + +.fuel-map-property-list { + margin: 4px; +} + +.fuel-map-property-list > .fuel-map { + overflow: hidden; + width: 100%; + max-width: 100%; +} + +.fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-name, +.fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-value { + padding: 2px 4px; + white-space: nowrap; +} + +.fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-name { + color: {$bluetext}; +} + +.fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-value { + color: {$lightgreytext}; +} + +.device-desktop + .fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-name { + text-align: right; + min-width: 80px; + width: 80px; +} + +.device-desktop + .fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-value { + text-overflow: ellipsis; + overflow: hidden; +} + +.device + .fuel-map-property-list > .fuel-map > .fuel-map-items > .fuel-map-pair > + .fuel-map-value { + padding-left: 12px; +}