From af1f57b37ac3e96eaa157ab11e3564a6fb19e16b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 4 Feb 2013 17:00:27 -0800 Subject: [PATCH] Add a preference to completely disable the file tree Summary: See D4812. - This preference disables the file tree completely. - It defaults off, so users who want it will have to go turn it on. - Maybe slightly cleaner would be doing this if the tree was hidden and then ajaxing it in if you press "F", but that's complicated and I don't want to bother. - Generally, I think this element is useful to something like 5% of users and not useful to 95%. Test Plan: Enabled and disabled file tree. Looked at commits and revisions; verified they reflected the setting correctly. Reviewers: vrana Reviewed By: vrana CC: aran Differential Revision: https://secure.phabricator.com/D4813 --- src/__celerity_resource_map__.php | 128 ++++++++++-------- src/__phutil_library_map__.php | 2 + .../DifferentialRevisionViewController.php | 48 ++++--- ...rentialChangesetFileTreeSideNavBuilder.php | 2 + .../controller/DiffusionCommitController.php | 9 +- ...habricatorSettingsPanelDiffPreferences.php | 72 ++++++++++ .../storage/PhabricatorUserPreferences.php | 2 + .../js/application/core/behavior-file-tree.js | 16 +++ .../differential/behavior-keyboard-nav.js | 6 - 9 files changed, 200 insertions(+), 85 deletions(-) create mode 100644 src/applications/settings/panel/PhabricatorSettingsPanelDiffPreferences.php create mode 100644 webroot/rsrc/js/application/core/behavior-file-tree.js diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 3df75ec35a..7693a108a5 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1292,7 +1292,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-keyboard-navigation' => array( - 'uri' => '/res/a7798465/rsrc/js/application/differential/behavior-keyboard-nav.js', + 'uri' => '/res/89e93cc9/rsrc/js/application/differential/behavior-keyboard-nav.js', 'type' => 'js', 'requires' => array( @@ -1664,6 +1664,18 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/behavior-autofocus.js', ), + 'javelin-behavior-phabricator-file-tree' => + array( + 'uri' => '/res/e9c96597/rsrc/js/application/core/behavior-file-tree.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'phabricator-keyboard-shortcut', + 2 => 'javelin-stratcom', + ), + 'disk' => '/rsrc/js/application/core/behavior-file-tree.js', + ), 'javelin-behavior-phabricator-home-reveal-tiles' => array( 'uri' => '/res/7230ca0c/rsrc/js/application/core/behavior-home-reveal-tiles.js', @@ -1704,7 +1716,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-phabricator-nav' => array( - 'uri' => '/res/cec31f3f/rsrc/js/application/core/behavior-phabricator-nav.js', + 'uri' => '/res/222b9329/rsrc/js/application/core/behavior-phabricator-nav.js', 'type' => 'js', 'requires' => array( @@ -3431,7 +3443,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/7d347135/core.pkg.css', 'type' => 'css', ), - '8a3f0fbd' => + '22373a47' => array( 'name' => 'core.pkg.js', 'symbols' => @@ -3470,7 +3482,7 @@ celerity_register_resource_map(array( 31 => 'javelin-behavior-global-drag-and-drop', 32 => 'javelin-behavior-phabricator-home-reveal-tiles', ), - 'uri' => '/res/pkg/8a3f0fbd/core.pkg.js', + 'uri' => '/res/pkg/22373a47/core.pkg.js', 'type' => 'js', ), '8edbada5' => @@ -3508,7 +3520,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/ec01d039/differential.pkg.css', 'type' => 'css', ), - '1b2c991b' => + '95d0d865' => array( 'name' => 'differential.pkg.js', 'symbols' => @@ -3533,7 +3545,7 @@ celerity_register_resource_map(array( 17 => 'javelin-behavior-differential-toggle-files', 18 => 'javelin-behavior-differential-user-select', ), - 'uri' => '/res/pkg/1b2c991b/differential.pkg.js', + 'uri' => '/res/pkg/95d0d865/differential.pkg.js', 'type' => 'js', ), 'c8ce2d88' => @@ -3633,7 +3645,7 @@ celerity_register_resource_map(array( 'aphront-typeahead-control-css' => '7d347135', 'differential-changeset-view-css' => 'ec01d039', 'differential-core-view-css' => 'ec01d039', - 'differential-inline-comment-editor' => '1b2c991b', + 'differential-inline-comment-editor' => '95d0d865', 'differential-local-commits-view-css' => 'ec01d039', 'differential-results-table-css' => 'ec01d039', 'differential-revision-add-comment-css' => 'ec01d039', @@ -3646,56 +3658,56 @@ celerity_register_resource_map(array( 'diffusion-icons-css' => 'c8ce2d88', 'global-drag-and-drop-css' => '7d347135', 'inline-comment-summary-css' => 'ec01d039', - 'javelin-aphlict' => '8a3f0fbd', + 'javelin-aphlict' => '22373a47', 'javelin-behavior' => '88225b70', - 'javelin-behavior-aphlict-dropdown' => '8a3f0fbd', - 'javelin-behavior-aphlict-listen' => '8a3f0fbd', - 'javelin-behavior-aphront-basic-tokenizer' => '8a3f0fbd', - 'javelin-behavior-aphront-drag-and-drop' => '1b2c991b', - 'javelin-behavior-aphront-drag-and-drop-textarea' => '1b2c991b', - 'javelin-behavior-aphront-form-disable-on-submit' => '8a3f0fbd', + 'javelin-behavior-aphlict-dropdown' => '22373a47', + 'javelin-behavior-aphlict-listen' => '22373a47', + 'javelin-behavior-aphront-basic-tokenizer' => '22373a47', + 'javelin-behavior-aphront-drag-and-drop' => '95d0d865', + 'javelin-behavior-aphront-drag-and-drop-textarea' => '95d0d865', + 'javelin-behavior-aphront-form-disable-on-submit' => '22373a47', 'javelin-behavior-audit-preview' => 'f96657b8', 'javelin-behavior-dark-console' => '8edbada5', 'javelin-behavior-dark-console-ajax' => '8edbada5', - 'javelin-behavior-device' => '8a3f0fbd', - 'javelin-behavior-differential-accept-with-errors' => '1b2c991b', - 'javelin-behavior-differential-add-reviewers-and-ccs' => '1b2c991b', - 'javelin-behavior-differential-comment-jump' => '1b2c991b', - 'javelin-behavior-differential-diff-radios' => '1b2c991b', - 'javelin-behavior-differential-dropdown-menus' => '1b2c991b', - 'javelin-behavior-differential-edit-inline-comments' => '1b2c991b', - 'javelin-behavior-differential-feedback-preview' => '1b2c991b', - 'javelin-behavior-differential-keyboard-navigation' => '1b2c991b', - 'javelin-behavior-differential-populate' => '1b2c991b', - 'javelin-behavior-differential-show-more' => '1b2c991b', - 'javelin-behavior-differential-toggle-files' => '1b2c991b', - 'javelin-behavior-differential-user-select' => '1b2c991b', + 'javelin-behavior-device' => '22373a47', + 'javelin-behavior-differential-accept-with-errors' => '95d0d865', + 'javelin-behavior-differential-add-reviewers-and-ccs' => '95d0d865', + 'javelin-behavior-differential-comment-jump' => '95d0d865', + 'javelin-behavior-differential-diff-radios' => '95d0d865', + 'javelin-behavior-differential-dropdown-menus' => '95d0d865', + 'javelin-behavior-differential-edit-inline-comments' => '95d0d865', + 'javelin-behavior-differential-feedback-preview' => '95d0d865', + 'javelin-behavior-differential-keyboard-navigation' => '95d0d865', + 'javelin-behavior-differential-populate' => '95d0d865', + 'javelin-behavior-differential-show-more' => '95d0d865', + 'javelin-behavior-differential-toggle-files' => '95d0d865', + 'javelin-behavior-differential-user-select' => '95d0d865', 'javelin-behavior-diffusion-commit-graph' => 'f96657b8', 'javelin-behavior-diffusion-pull-lastmodified' => 'f96657b8', 'javelin-behavior-error-log' => '8edbada5', - 'javelin-behavior-global-drag-and-drop' => '8a3f0fbd', - 'javelin-behavior-konami' => '8a3f0fbd', - 'javelin-behavior-lightbox-attachments' => '8a3f0fbd', + 'javelin-behavior-global-drag-and-drop' => '22373a47', + 'javelin-behavior-konami' => '22373a47', + 'javelin-behavior-lightbox-attachments' => '22373a47', 'javelin-behavior-maniphest-batch-selector' => '7707de41', 'javelin-behavior-maniphest-subpriority-editor' => '7707de41', 'javelin-behavior-maniphest-transaction-controls' => '7707de41', 'javelin-behavior-maniphest-transaction-expand' => '7707de41', 'javelin-behavior-maniphest-transaction-preview' => '7707de41', - 'javelin-behavior-phabricator-active-nav' => '8a3f0fbd', - 'javelin-behavior-phabricator-autofocus' => '8a3f0fbd', - 'javelin-behavior-phabricator-home-reveal-tiles' => '8a3f0fbd', - 'javelin-behavior-phabricator-keyboard-shortcuts' => '8a3f0fbd', - 'javelin-behavior-phabricator-nav' => '8a3f0fbd', - 'javelin-behavior-phabricator-object-selector' => '1b2c991b', - 'javelin-behavior-phabricator-oncopy' => '8a3f0fbd', - 'javelin-behavior-phabricator-remarkup-assist' => '8a3f0fbd', - 'javelin-behavior-phabricator-search-typeahead' => '8a3f0fbd', - 'javelin-behavior-phabricator-tooltips' => '8a3f0fbd', - 'javelin-behavior-phabricator-watch-anchor' => '8a3f0fbd', - 'javelin-behavior-refresh-csrf' => '8a3f0fbd', - 'javelin-behavior-repository-crossreference' => '1b2c991b', - 'javelin-behavior-toggle-class' => '8a3f0fbd', - 'javelin-behavior-workflow' => '8a3f0fbd', + 'javelin-behavior-phabricator-active-nav' => '22373a47', + 'javelin-behavior-phabricator-autofocus' => '22373a47', + 'javelin-behavior-phabricator-home-reveal-tiles' => '22373a47', + 'javelin-behavior-phabricator-keyboard-shortcuts' => '22373a47', + 'javelin-behavior-phabricator-nav' => '22373a47', + 'javelin-behavior-phabricator-object-selector' => '95d0d865', + 'javelin-behavior-phabricator-oncopy' => '22373a47', + 'javelin-behavior-phabricator-remarkup-assist' => '22373a47', + 'javelin-behavior-phabricator-search-typeahead' => '22373a47', + 'javelin-behavior-phabricator-tooltips' => '22373a47', + 'javelin-behavior-phabricator-watch-anchor' => '22373a47', + 'javelin-behavior-refresh-csrf' => '22373a47', + 'javelin-behavior-repository-crossreference' => '95d0d865', + 'javelin-behavior-toggle-class' => '22373a47', + 'javelin-behavior-workflow' => '22373a47', 'javelin-dom' => '88225b70', 'javelin-event' => '88225b70', 'javelin-install' => '88225b70', @@ -3717,39 +3729,39 @@ celerity_register_resource_map(array( 'lightbox-attachment-css' => '7d347135', 'maniphest-task-summary-css' => 'e30a3fa8', 'maniphest-transaction-detail-css' => 'e30a3fa8', - 'phabricator-busy' => '8a3f0fbd', + 'phabricator-busy' => '22373a47', 'phabricator-content-source-view-css' => 'ec01d039', 'phabricator-core-buttons-css' => '7d347135', 'phabricator-core-css' => '7d347135', 'phabricator-crumbs-view-css' => '7d347135', 'phabricator-directory-css' => '7d347135', - 'phabricator-drag-and-drop-file-upload' => '1b2c991b', - 'phabricator-dropdown-menu' => '8a3f0fbd', - 'phabricator-file-upload' => '8a3f0fbd', + 'phabricator-drag-and-drop-file-upload' => '95d0d865', + 'phabricator-dropdown-menu' => '22373a47', + 'phabricator-file-upload' => '22373a47', 'phabricator-filetree-view-css' => '7d347135', 'phabricator-flag-css' => '7d347135', 'phabricator-form-view-css' => '7d347135', 'phabricator-header-view-css' => '7d347135', 'phabricator-jump-nav' => '7d347135', - 'phabricator-keyboard-shortcut' => '8a3f0fbd', - 'phabricator-keyboard-shortcut-manager' => '8a3f0fbd', + 'phabricator-keyboard-shortcut' => '22373a47', + 'phabricator-keyboard-shortcut-manager' => '22373a47', 'phabricator-main-menu-view' => '7d347135', - 'phabricator-menu-item' => '8a3f0fbd', + 'phabricator-menu-item' => '22373a47', 'phabricator-nav-view-css' => '7d347135', - 'phabricator-notification' => '8a3f0fbd', + 'phabricator-notification' => '22373a47', 'phabricator-notification-css' => '7d347135', 'phabricator-notification-menu-css' => '7d347135', 'phabricator-object-item-list-view-css' => '7d347135', 'phabricator-object-selector-css' => 'ec01d039', - 'phabricator-paste-file-upload' => '8a3f0fbd', - 'phabricator-prefab' => '8a3f0fbd', + 'phabricator-paste-file-upload' => '22373a47', + 'phabricator-prefab' => '22373a47', 'phabricator-project-tag-css' => 'e30a3fa8', 'phabricator-remarkup-css' => '7d347135', - 'phabricator-shaped-request' => '1b2c991b', + 'phabricator-shaped-request' => '95d0d865', 'phabricator-side-menu-view-css' => '7d347135', 'phabricator-standard-page-view' => '7d347135', - 'phabricator-textareautils' => '8a3f0fbd', - 'phabricator-tooltip' => '8a3f0fbd', + 'phabricator-textareautils' => '22373a47', + 'phabricator-tooltip' => '22373a47', 'phabricator-transaction-view-css' => '7d347135', 'phabricator-zindex-css' => '7d347135', 'sprite-apps-large-css' => '7d347135', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 83d988c22d..b40b32e2a1 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1228,6 +1228,7 @@ phutil_register_library_map(array( 'PhabricatorSettingsPanel' => 'applications/settings/panel/PhabricatorSettingsPanel.php', 'PhabricatorSettingsPanelAccount' => 'applications/settings/panel/PhabricatorSettingsPanelAccount.php', 'PhabricatorSettingsPanelConduit' => 'applications/settings/panel/PhabricatorSettingsPanelConduit.php', + 'PhabricatorSettingsPanelDiffPreferences' => 'applications/settings/panel/PhabricatorSettingsPanelDiffPreferences.php', 'PhabricatorSettingsPanelDisplayPreferences' => 'applications/settings/panel/PhabricatorSettingsPanelDisplayPreferences.php', 'PhabricatorSettingsPanelEmailAddresses' => 'applications/settings/panel/PhabricatorSettingsPanelEmailAddresses.php', 'PhabricatorSettingsPanelEmailPreferences' => 'applications/settings/panel/PhabricatorSettingsPanelEmailPreferences.php', @@ -2628,6 +2629,7 @@ phutil_register_library_map(array( 'PhabricatorSettingsMainController' => 'PhabricatorController', 'PhabricatorSettingsPanelAccount' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelConduit' => 'PhabricatorSettingsPanel', + 'PhabricatorSettingsPanelDiffPreferences' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelDisplayPreferences' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelEmailAddresses' => 'PhabricatorSettingsPanel', 'PhabricatorSettingsPanelEmailPreferences' => 'PhabricatorSettingsPanel', diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php index d55e5d3ce9..a5f8c41095 100644 --- a/src/applications/differential/controller/DifferentialRevisionViewController.php +++ b/src/applications/differential/controller/DifferentialRevisionViewController.php @@ -408,34 +408,42 @@ final class DifferentialRevisionViewController extends DifferentialController { ->setAnchorName('top') ->setNavigationMarker(true); - $collapsed = $user->loadPreferences()->getPreference( - PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED, - false); - - $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) - ->setAnchorName('top') - ->setTitle('D'.$revision->getID()) - ->setBaseURI(new PhutilURI('/D'.$revision->getID())) - ->setCollapsed((bool)$collapsed) - ->build($changesets); - $nav->appendChild( - array( - $reviewer_warning, - $top_anchor, - $revision_detail, - $page_pane, - )); - + $content = array( + $reviewer_warning, + $top_anchor, + $revision_detail, + $page_pane, + ); $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( id(new PhabricatorCrumbView()) ->setName($object_id) ->setHref('/'.$object_id)); - $nav->setCrumbs($crumbs); + + $prefs = $user->loadPreferences(); + + $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; + if ($prefs->getPreference($pref_filetree)) { + $collapsed = $prefs->getPreference( + PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED, + false); + + $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) + ->setAnchorName('top') + ->setTitle('D'.$revision->getID()) + ->setBaseURI(new PhutilURI('/D'.$revision->getID())) + ->setCollapsed((bool)$collapsed) + ->build($changesets); + $nav->appendChild($content); + $nav->setCrumbs($crumbs); + $content = $nav; + } else { + array_unshift($content, $crumbs); + } return $this->buildApplicationPage( - $nav, + $content, array( 'title' => $object_id.' '.$revision->getTitle(), )); diff --git a/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php index ecb3e80504..d3ae1ad744 100644 --- a/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php +++ b/src/applications/differential/view/DifferentialChangesetFileTreeSideNavBuilder.php @@ -123,6 +123,8 @@ final class DifferentialChangesetFileTreeSideNavBuilder { } $tree->destroy(); + Javelin::initBehavior('phabricator-file-tree', array()); + $filetree = '
'. implode("\n", $filetree). diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 69f96d2537..d9f2bee1ea 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -324,13 +324,20 @@ final class DiffusionCommitController extends DiffusionController { 'commit' => true, )); - if ($changesets) { + $prefs = $user->loadPreferences(); + $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; + $pref_collapse = PhabricatorUserPreferences::PREFERENCE_NAV_COLLAPSED; + $show_filetree = $prefs->getPreference($pref_filetree); + $collapsed = $prefs->getPreference($pref_collapse); + + if ($changesets && $show_filetree) { $nav = id(new DifferentialChangesetFileTreeSideNavBuilder()) ->setAnchorName('top') ->setTitle($short_name) ->setBaseURI(new PhutilURI('/'.$commit_id)) ->build($changesets) ->setCrumbs($crumbs) + ->setCollapsed((bool)$collapsed) ->appendChild($content); $content = $nav; } else { diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelDiffPreferences.php b/src/applications/settings/panel/PhabricatorSettingsPanelDiffPreferences.php new file mode 100644 index 0000000000..b6dc0afc3c --- /dev/null +++ b/src/applications/settings/panel/PhabricatorSettingsPanelDiffPreferences.php @@ -0,0 +1,72 @@ +getUser(); + $preferences = $user->loadPreferences(); + + $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; + + if ($request->isFormPost()) { + $preferences->setPreference( + $pref_filetree, + $request->getInt($pref_filetree)); + + $preferences->save(); + return id(new AphrontRedirectResponse()) + ->setURI($this->getPanelURI('?saved=true')); + } + + $form = id(new AphrontFormView()) + ->setUser($user) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Show Filetree')) + ->setName($pref_filetree) + ->setValue($preferences->getPreference($pref_filetree)) + ->setOptions( + array( + 0 => pht('Disable Filetree'), + 1 => pht('Enable Filetree'), + )) + ->setCaption( + pht("When looking at a revision or commit, show affected files ". + "in a sidebar."))) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Save Preferences'))); + + $panel = new AphrontPanelView(); + $panel->setHeader(pht('Diff Preferences')); + $panel->appendChild($form); + $panel->setNoBackground(); + + $error_view = null; + if ($request->getBool('saved')) { + $error_view = id(new AphrontErrorView()) + ->setTitle(pht('Preferences Saved')) + ->setSeverity(AphrontErrorView::SEVERITY_NOTICE) + ->setErrors(array(pht('Your preferences have been saved.'))); + } + + return array( + $error_view, + $panel, + ); + } +} + diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 1b23ee082d..9f5196a215 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -23,6 +23,8 @@ final class PhabricatorUserPreferences extends PhabricatorUserDAO { const PREFERENCE_NAV_WIDTH = 'nav-width'; const PREFERENCE_APP_TILES = 'app-tiles'; + const PREFERENCE_DIFF_FILETREE = 'diff-filetree'; + protected $userPHID; protected $preferences = array(); diff --git a/webroot/rsrc/js/application/core/behavior-file-tree.js b/webroot/rsrc/js/application/core/behavior-file-tree.js new file mode 100644 index 0000000000..d5b908eb75 --- /dev/null +++ b/webroot/rsrc/js/application/core/behavior-file-tree.js @@ -0,0 +1,16 @@ +/** + * @provides javelin-behavior-phabricator-file-tree + * @requires javelin-behavior + * phabricator-keyboard-shortcut + * javelin-stratcom + */ + +JX.behavior('phabricator-file-tree', function(config) { + + new JX.KeyboardShortcut('f', 'Toggle file tree.') + .setHandler(function(manager) { + JX.Stratcom.invoke('differential-filetree-toggle'); + }) + .register(); + +}); diff --git a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js b/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js index 46695a43a8..ef9baae675 100644 --- a/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js +++ b/webroot/rsrc/js/application/differential/behavior-keyboard-nav.js @@ -264,12 +264,6 @@ JX.behavior('differential-keyboard-navigation', function(config) { }) .register(); - new JX.KeyboardShortcut('f', 'Toggle file tree.') - .setHandler(function(manager) { - JX.Stratcom.invoke('differential-filetree-toggle'); - }) - .register(); - if (config.haunt) { new JX.KeyboardShortcut('z', 'Cycle comment panel haunting modes.') .setHandler(haunt)