From 4c7370a1a3b0b59f4ce6fc731a59b637f75c0ed2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 22 Feb 2018 13:31:20 -0800 Subject: [PATCH] Make the filetree view width sticky across show/hide and reload Summary: Ref T13090. The default width changed recently to become much wider, but the behavior on this control isn't great. Instead: - Pick a default width somewhere between the two. - Make the width sticky across show/hide (pressing "f" twice remembers your width instead of resetting it). - Make the width sticky across reloads (dragging the bar, then reloading the page keeps the bar in the same place). Test Plan: - Without settings, loaded page: got medium-width bar. - Dragged bar wide/narrow, toggled on/off with "f", got persistent width. - Dragged bar wide/narrow, reloaded page, got persistent width. - Dragged bar wide/narrow, toggled it off, reloaded page, toggled it on, got persistent width. Maniphest Tasks: T13090 Differential Revision: https://secure.phabricator.com/D19129 --- resources/celerity/map.php | 22 ++++---- src/__phutil_library_map__.php | 2 + .../DifferentialRevisionViewController.php | 4 ++ ...rentialChangesetFileTreeSideNavBuilder.php | 15 +++-- .../controller/DiffusionCommitController.php | 12 ++-- .../PhabricatorFiletreeWidthSetting.php | 12 ++++ src/view/layout/AphrontSideNavFilterView.php | 55 ++++++++++++++----- .../rsrc/css/aphront/phabricator-nav-view.css | 6 +- .../rsrc/js/core/behavior-phabricator-nav.js | 35 +++++++++++- 9 files changed, 124 insertions(+), 39 deletions(-) create mode 100644 src/applications/settings/setting/PhabricatorFiletreeWidthSetting.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 32ccb9e5f9..01dd75c901 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,8 +9,8 @@ return array( 'names' => array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', - 'core.pkg.css' => 'e4f098a5', - 'core.pkg.js' => 'bd19de1c', + 'core.pkg.css' => '2fa91e14', + 'core.pkg.js' => 'dc13d4b7', 'darkconsole.pkg.js' => '1f9a31bc', 'differential.pkg.css' => '113e692c', 'differential.pkg.js' => 'f6d809c0', @@ -31,7 +31,7 @@ return array( 'rsrc/css/aphront/multi-column.css' => '84cc6640', 'rsrc/css/aphront/notification.css' => '457861ec', 'rsrc/css/aphront/panel-view.css' => '8427b78d', - 'rsrc/css/aphront/phabricator-nav-view.css' => '028126f6', + 'rsrc/css/aphront/phabricator-nav-view.css' => 'a9e3e6d5', 'rsrc/css/aphront/table-view.css' => '8c9bbafe', 'rsrc/css/aphront/tokenizer.css' => '15d5ff71', 'rsrc/css/aphront/tooltip.css' => '173b9431', @@ -498,7 +498,7 @@ return array( 'rsrc/js/core/behavior-more.js' => 'a80d0378', 'rsrc/js/core/behavior-object-selector.js' => '77c1f0b0', 'rsrc/js/core/behavior-oncopy.js' => '2926fff2', - 'rsrc/js/core/behavior-phabricator-nav.js' => '81144dfa', + 'rsrc/js/core/behavior-phabricator-nav.js' => '836f966d', 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => 'acd29eee', 'rsrc/js/core/behavior-read-only-warning.js' => 'ba158207', 'rsrc/js/core/behavior-refresh-csrf.js' => 'ab2f381b', @@ -657,7 +657,7 @@ return array( 'javelin-behavior-phabricator-keyboard-pager' => 'a8da01f0', 'javelin-behavior-phabricator-keyboard-shortcuts' => '01fca1f0', 'javelin-behavior-phabricator-line-linker' => '1499a8cb', - 'javelin-behavior-phabricator-nav' => '81144dfa', + 'javelin-behavior-phabricator-nav' => '836f966d', 'javelin-behavior-phabricator-notification-example' => '8ce821c5', 'javelin-behavior-phabricator-object-selector' => '77c1f0b0', 'javelin-behavior-phabricator-oncopy' => '2926fff2', @@ -789,7 +789,7 @@ return array( 'phabricator-keyboard-shortcut' => '1ae869f2', 'phabricator-keyboard-shortcut-manager' => 'c19dd9b9', 'phabricator-main-menu-view' => '1802a242', - 'phabricator-nav-view-css' => '028126f6', + 'phabricator-nav-view-css' => 'a9e3e6d5', 'phabricator-notification' => '008faf9c', 'phabricator-notification-css' => '457861ec', 'phabricator-notification-menu-css' => '10685bd4', @@ -1563,7 +1563,11 @@ return array( 'phuix-icon-view', 'phabricator-prefab', ), - '81144dfa' => array( + '834a1173' => array( + 'javelin-behavior', + 'javelin-scrollbar', + ), + '836f966d' => array( 'javelin-behavior', 'javelin-behavior-device', 'javelin-stratcom', @@ -1573,10 +1577,6 @@ return array( 'javelin-request', 'javelin-util', ), - '834a1173' => array( - 'javelin-behavior', - 'javelin-scrollbar', - ), '8499b6ab' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 6fd36a1e38..4455ad139a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3046,6 +3046,7 @@ phutil_register_library_map(array( 'PhabricatorFilesOnDiskBuiltinFile' => 'applications/files/builtin/PhabricatorFilesOnDiskBuiltinFile.php', 'PhabricatorFilesOutboundRequestAction' => 'applications/files/action/PhabricatorFilesOutboundRequestAction.php', 'PhabricatorFiletreeVisibleSetting' => 'applications/settings/setting/PhabricatorFiletreeVisibleSetting.php', + 'PhabricatorFiletreeWidthSetting' => 'applications/settings/setting/PhabricatorFiletreeWidthSetting.php', 'PhabricatorFlag' => 'applications/flag/storage/PhabricatorFlag.php', 'PhabricatorFlagAddFlagHeraldAction' => 'applications/flag/herald/PhabricatorFlagAddFlagHeraldAction.php', 'PhabricatorFlagColor' => 'applications/flag/constants/PhabricatorFlagColor.php', @@ -8613,6 +8614,7 @@ phutil_register_library_map(array( 'PhabricatorFilesOnDiskBuiltinFile' => 'PhabricatorFilesBuiltinFile', 'PhabricatorFilesOutboundRequestAction' => 'PhabricatorSystemAction', 'PhabricatorFiletreeVisibleSetting' => 'PhabricatorInternalSetting', + 'PhabricatorFiletreeWidthSetting' => 'PhabricatorInternalSetting', 'PhabricatorFlag' => array( 'PhabricatorFlagDAO', 'PhabricatorPolicyInterface', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index e1da5ed32e..d7280cb0b6 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -473,10 +473,14 @@ final class DifferentialRevisionViewController extends DifferentialController { $collapsed_key = PhabricatorFiletreeVisibleSetting::SETTINGKEY; $collapsed_value = $viewer->getUserSetting($collapsed_key); + $width_key = PhabricatorFiletreeWidthSetting::SETTINGKEY; + $width_value = $viewer->getUserSetting($width_key); + $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setTitle($monogram) ->setBaseURI(new PhutilURI($revision->getURI())) ->setCollapsed((bool)$collapsed_value) + ->setWidth((int)$width_value) ->build($changesets); } diff --git a/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php index 1f699be8eb..e8852a5c52 100644 --- a/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php +++ b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php @@ -6,6 +6,7 @@ final class DifferentialChangesetFileTreeSideNavBuilder extends Phobject { private $baseURI; private $anchorName; private $collapsed = false; + private $width; public function setAnchorName($anchor_name) { $this->anchorName = $anchor_name; @@ -36,13 +37,19 @@ final class DifferentialChangesetFileTreeSideNavBuilder extends Phobject { return $this; } + public function setWidth($width) { + $this->width = $width; + return $this; + } + public function build(array $changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI($this->getBaseURI()); - $nav->setFlexible(true); - $nav->setCollapsed($this->collapsed); + $nav = id(new AphrontSideNavFilterView()) + ->setBaseURI($this->getBaseURI()) + ->setFlexible(true) + ->setCollapsed($this->collapsed) + ->setWidth($this->width); $anchor = $this->getAnchorName(); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index b4d06989c2..67ffb73357 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -415,17 +415,21 @@ final class DiffusionCommitController extends DiffusionController { PhabricatorShowFiletreeSetting::SETTINGKEY, PhabricatorShowFiletreeSetting::VALUE_ENABLE_FILETREE); - $pref_collapse = PhabricatorFiletreeVisibleSetting::SETTINGKEY; - $collapsed = $viewer->getUserSetting($pref_collapse); - $nav = null; if ($show_changesets && $filetree_on) { + $pref_collapse = PhabricatorFiletreeVisibleSetting::SETTINGKEY; + $collapsed = $viewer->getUserSetting($pref_collapse); + + $pref_width = PhabricatorFiletreeWidthSetting::SETTINGKEY; + $width = $viewer->getUserSetting($pref_width); + $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setTitle($commit->getDisplayName()) ->setBaseURI(new PhutilURI($commit->getURI())) ->build($changesets) ->setCrumbs($crumbs) - ->setCollapsed((bool)$collapsed); + ->setCollapsed((bool)$collapsed) + ->setWidth((int)$width); } $view = id(new PHUITwoColumnView()) diff --git a/src/applications/settings/setting/PhabricatorFiletreeWidthSetting.php b/src/applications/settings/setting/PhabricatorFiletreeWidthSetting.php new file mode 100644 index 0000000000..5cfc9f3fe7 --- /dev/null +++ b/src/applications/settings/setting/PhabricatorFiletreeWidthSetting.php @@ -0,0 +1,12 @@ +menuID = $menu_id; @@ -82,6 +83,11 @@ final class AphrontSideNavFilterView extends AphrontView { return $this; } + public function setWidth($width) { + $this->width = $width; + return $this; + } + public function getMenuView() { return $this->menu; } @@ -216,6 +222,24 @@ final class AphrontSideNavFilterView extends AphrontView { $local_menu = null; $main_id = $this->getMainID(); + $width = $this->width; + if ($width) { + $width = min($width, 600); + $width = max($width, 150); + } else { + $width = null; + } + + if ($width && !$this->collapsed) { + $width_drag_style = 'left: '.$width.'px'; + $width_panel_style = 'width: '.$width.'px'; + $width_margin_style = 'margin-left: '.($width + 7).'px'; + } else { + $width_drag_style = null; + $width_panel_style = null; + $width_margin_style = null; + } + if ($this->flexible) { $drag_id = celerity_generate_unique_node_id(); $flex_bar = phutil_tag( @@ -223,6 +247,7 @@ final class AphrontSideNavFilterView extends AphrontView { array( 'class' => 'phabricator-nav-drag', 'id' => $drag_id, + 'style' => $width_drag_style, ), ''); } else { @@ -238,14 +263,14 @@ final class AphrontSideNavFilterView extends AphrontView { $nav_classes[] = 'has-local-nav'; } - $local_menu = - phutil_tag( - 'div', - array( - 'class' => 'phabricator-nav-local phabricator-side-menu', - 'id' => $local_id, - ), - $this->menu->setID($this->getMenuID())); + $local_menu = phutil_tag( + 'div', + array( + 'class' => 'phabricator-nav-local phabricator-side-menu', + 'id' => $local_id, + 'style' => $width_panel_style, + ), + $this->menu->setID($this->getMenuID())); } $crumbs = null; @@ -264,12 +289,13 @@ final class AphrontSideNavFilterView extends AphrontView { Javelin::initBehavior( 'phabricator-nav', array( - 'mainID' => $main_id, - 'localID' => $local_id, - 'dragID' => $drag_id, - 'contentID' => $content_id, - 'backgroundID' => $background_id, - 'collapsed' => $this->collapsed, + 'mainID' => $main_id, + 'localID' => $local_id, + 'dragID' => $drag_id, + 'contentID' => $content_id, + 'backgroundID' => $background_id, + 'collapsed' => $this->collapsed, + 'width' => $width, )); if ($this->active) { @@ -297,6 +323,7 @@ final class AphrontSideNavFilterView extends AphrontView { array( 'class' => 'phabricator-nav-content plb', 'id' => $content_id, + 'style' => $width_margin_style, ), array( $crumbs, diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css index f3320e3eae..6dbd0931a6 100644 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ b/webroot/rsrc/css/aphront/phabricator-nav-view.css @@ -44,7 +44,7 @@ position: fixed; top: 0; bottom: 0; - left: 410px; + left: 310px; width: 7px; cursor: col-resize; @@ -66,7 +66,7 @@ .device-desktop .phabricator-standard-page-body .has-drag-nav .phabricator-nav-content { - margin-left: 417px; + margin-left: 317px; } .device-desktop .phabricator-standard-page-body .has-drag-nav @@ -81,7 +81,7 @@ } .device-desktop .phui-navigation-shell .has-drag-nav .phabricator-nav-local { - width: 410px; + width: 310px; padding: 0; background: transparent; } diff --git a/webroot/rsrc/js/core/behavior-phabricator-nav.js b/webroot/rsrc/js/core/behavior-phabricator-nav.js index 74909e447d..50d8e1c820 100644 --- a/webroot/rsrc/js/core/behavior-phabricator-nav.js +++ b/webroot/rsrc/js/core/behavior-phabricator-nav.js @@ -18,7 +18,6 @@ JX.behavior('phabricator-nav', function(config) { var main = JX.$(config.mainID); var drag = JX.$(config.dragID); - // - Flexible Navigation Column ------------------------------------------------ @@ -98,22 +97,52 @@ JX.behavior('phabricator-nav', function(config) { } JX.DOM.alterClass(document.body, 'jx-drag-col', false); dragging = false; + + new JX.Request('/settings/adjust/', JX.bag) + .setData( + { + key: 'filetree.width', + value: JX.$V(drag).x + }) + .send(); }); - function resetdrag() { + var saved_width = config.width; + function savedrag() { + saved_width = JX.$V(drag).x; + local.style.width = ''; drag.style.left = ''; content.style.marginLeft = ''; } + function restoredrag() { + if (!saved_width) { + return; + } + + local.style.width = saved_width + 'px'; + drag.style.left = saved_width + 'px'; + content.style.marginLeft = (saved_width + JX.Vector.getDim(drag).x) + 'px'; + } + var collapsed = config.collapsed; JX.Stratcom.listen('differential-filetree-toggle', null, function() { collapsed = !collapsed; + + if (collapsed) { + savedrag(); + } + JX.DOM.alterClass(main, 'has-local-nav', !collapsed); JX.DOM.alterClass(main, 'has-drag-nav', !collapsed); JX.DOM.alterClass(main, 'has-closed-nav', collapsed); - resetdrag(); + + if (!collapsed) { + restoredrag(); + } + new JX.Request('/settings/adjust/', JX.bag) .setData({ key : 'nav-collapsed', value : (collapsed ? 1 : 0) }) .send();