diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -360,7 +360,7 @@ 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '82439934', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', - 'rsrc/js/application/differential/ChangesetViewManager.js' => '5eb5b98c', + 'rsrc/js/application/differential/ChangesetViewManager.js' => 'c024db3d', 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2441746', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', @@ -510,7 +510,7 @@ 'aphront-two-column-view-css' => '16ab3ad2', 'aphront-typeahead-control-css' => '0e403212', 'auth-css' => '1e655982', - 'changeset-view-manager' => '5eb5b98c', + 'changeset-view-manager' => 'c024db3d', 'config-options-css' => '7fedf08b', 'config-welcome-css' => '6abd79be', 'conpherence-durable-column-view' => '3b836442', @@ -1180,16 +1180,6 @@ 'javelin-dom', 'javelin-vector', ), - '5eb5b98c' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - ), '5fefb143' => array( 'javelin-behavior', 'javelin-dom', @@ -1714,6 +1704,16 @@ 'javelin-util', 'phabricator-shaped-request', ), + 'c024db3d' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + ), 'c1700f6f' => array( 'javelin-install', 'javelin-util', diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -240,10 +240,11 @@ // undergoing like six kinds of refactoring anyway. $output = phutil_safe_html($output); - $detail = new DifferentialChangesetDetailView(); - $detail->setChangeset($changeset); - $detail->appendChild($output); - $detail->setVsChangesetID($left_source); + $detail = id(new DifferentialChangesetDetailView()) + ->setUser($this->getViewer()) + ->setChangeset($changeset) + ->appendChild($output) + ->setVsChangesetID($left_source); $panel = new DifferentialPrimaryPaneView(); $panel->appendChild( diff --git a/src/applications/differential/view/DifferentialChangesetDetailView.php b/src/applications/differential/view/DifferentialChangesetDetailView.php --- a/src/applications/differential/view/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/DifferentialChangesetDetailView.php @@ -188,6 +188,19 @@ $icon = id(new PHUIIconView()) ->setIconFont($display_icon); + $renderer = null; + + // If the viewer prefers unified diffs, always set the renderer to unified. + // Otherwise, we leave it unspecified and the client will choose a + // renderer based on the screen size. + + $viewer = $this->getUser(); + $prefs = $viewer->loadPreferences(); + $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; + if ($prefs->getPreference($pref_unified) == 'unified') { + $renderer = '1up'; + } + return javelin_tag( 'div', array( @@ -200,7 +213,7 @@ 'renderURI' => $this->getRenderURI(), 'whitespace' => $this->getWhitespace(), 'highlight' => null, - 'renderer' => null, + 'renderer' => $renderer, 'ref' => $this->getRenderingRef(), 'autoload' => $this->getAutoload(), ), diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -130,7 +130,7 @@ 'Configure Editor' => pht('Configure Editor'), 'Load Changes' => pht('Load Changes'), 'View Side-by-Side' => pht('View Side-by-Side'), - 'View Unified' => pht('View Unified (Barely Works!)'), + 'View Unified' => pht('View Unified'), 'Change Text Encoding...' => pht('Change Text Encoding...'), 'Highlight As...' => pht('Highlight As...'), ), @@ -148,7 +148,8 @@ $ref = $this->references[$key]; - $detail = new DifferentialChangesetDetailView(); + $detail = id(new DifferentialChangesetDetailView()) + ->setUser($this->getUser()); $uniq_id = 'diff-'.$changeset->getAnchorName(); $detail->setID($uniq_id); diff --git a/src/applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php --- a/src/applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorDiffPreferencesSettingsPanel.php @@ -19,6 +19,7 @@ $user = $request->getUser(); $preferences = $user->loadPreferences(); + $pref_unified = PhabricatorUserPreferences::PREFERENCE_DIFF_UNIFIED; $pref_filetree = PhabricatorUserPreferences::PREFERENCE_DIFF_FILETREE; if ($request->isFormPost()) { @@ -32,6 +33,9 @@ $preferences->setPreference($pref_filetree, $filetree); + $unified = $request->getStr($pref_unified); + $preferences->setPreference($pref_unified, $unified); + $preferences->save(); return id(new AphrontRedirectResponse()) ->setURI($this->getPanelURI('?saved=true')); @@ -39,6 +43,23 @@ $form = id(new AphrontFormView()) ->setUser($user) + ->appendRemarkupInstructions( + pht( + 'Phabricator normally shows diffs in a side-by-side layout on '. + 'large screens, and automatically switches to a unified '. + 'view on small screens (like mobile phones). If you prefer '. + 'unified diffs even on large screens, you can select them as '. + 'the default layout.')) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Show Unified Diffs')) + ->setName($pref_unified) + ->setValue($preferences->getPreference($pref_unified)) + ->setOptions( + array( + 'default' => pht('On Small Screens'), + 'unified' => pht('Always'), + ))) ->appendChild( id(new AphrontFormSelectControl()) ->setLabel(pht('Show Filetree')) diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -28,6 +28,7 @@ const PREFERENCE_APP_TILES = 'app-tiles'; const PREFERENCE_APP_PINNED = 'app-pinned'; + const PREFERENCE_DIFF_UNIFIED = 'diff-unified'; const PREFERENCE_DIFF_FILETREE = 'diff-filetree'; const PREFERENCE_CONPH_NOTIFICATIONS = 'conph-notifications'; diff --git a/webroot/rsrc/js/application/differential/ChangesetViewManager.js b/webroot/rsrc/js/application/differential/ChangesetViewManager.js --- a/webroot/rsrc/js/application/differential/ChangesetViewManager.js +++ b/webroot/rsrc/js/application/differential/ChangesetViewManager.js @@ -172,17 +172,12 @@ return this._renderer; } - // TODO: This is a big pile of TODOs. - // NOTE: If you load the page at one device resolution and then resize to // a different one we don't re-render the diffs, because it's a // complicated mess and you could lose inline comments, cursor positions, // etc. var renderer = (JX.Device.getDevice() == 'desktop') ? '2up' : '1up'; - // TODO: Once 1up works better, figure out when to show it. - renderer = '2up'; - return renderer; },