From d3cbfd952e1521e94b4ced6d019106af3aa96a4c Mon Sep 17 00:00:00 2001 From: Benjamin Kausch Date: Thu, 23 May 2024 11:43:13 +0200 Subject: [PATCH] Fix overflowing of AphrontSideNavFilterView on small screens and tidy up nav bar styles Summary: Switch the two column layout of AphrontSideNavFilterView (used in pretty much most views) from table to flex display. The former CSS rendering could lead to overflowing of the entire page on small screens if very wide unbreaking content is displayed (example described here: T15809). While I was at it I have found some unused code. The CSS file `phabricator-nav-view.css` consisted almost only of `.phabricator-side-menu-home` class selectors. This is an ancient class (there at least since 2013) not used anymore: https://we.phorge.it/source/phorge/browse/master/src/applications/directory/controller/PhabricatorDirectoryController.php;919bd4a03499305093d8882952a9dd3dac1c55a9$169 So I have removed this CSS component and saved the few still used declarations to the main `phui-basic-nav-view.css`. There was some unused code in `AphrontSideNavFilterView.php` too. Fixes T15809 Test Plan: Look at this video to understand the problem: F2178277 Go in a Task comment and mention a task with a very-long title. Have phd enabled to generate feeds. Visit homepage with feeds: See that you cannot reproduce anymore. Browse pages with the right nav bar with many devices and look if the layout looks fine and not broken. Tested the homepage, scaling from mobile to desktop, using: - Firefox 124 64bit on GNU/Linux (snap) - Chromium 124 64bit on GNU/Linux (snap) - ... Tested other elements, scaling from mobile to desktop, still working correctly: - popup dialogs - right floating menu - top curtain - notifications - Conpherence chats - unresolved setup issues - search omnibox Reviewers: O1 Blessed Committers, aklapper Reviewed By: O1 Blessed Committers, aklapper Subscribers: aklapper, speck, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Maniphest Tasks: T15809 Differential Revision: https://we.phorge.it/D25619 --- resources/celerity/map.php | 13 ++--- resources/celerity/packages.php | 1 - src/view/layout/AphrontSideNavFilterView.php | 8 ---- .../rsrc/css/aphront/phabricator-nav-view.css | 48 ------------------- .../application/base/standard-page-view.css | 4 -- webroot/rsrc/css/phui/phui-basic-nav-view.css | 18 ++++--- 6 files changed, 17 insertions(+), 75 deletions(-) delete mode 100644 webroot/rsrc/css/aphront/phabricator-nav-view.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 9c6c980c68..beb76af95c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3144a5e2', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '07b1d4b2', + 'core.pkg.css' => '10815c8e', 'core.pkg.js' => 'f58c3c6e', 'dark-console.pkg.js' => '187792c2', 'differential.pkg.css' => '91ac6214', @@ -31,7 +31,6 @@ return array( 'rsrc/css/aphront/multi-column.css' => 'fbc00ba3', 'rsrc/css/aphront/notification.css' => '30240bd2', 'rsrc/css/aphront/panel-view.css' => '46923d46', - 'rsrc/css/aphront/phabricator-nav-view.css' => '423f92cc', 'rsrc/css/aphront/table-view.css' => '0bb61df1', 'rsrc/css/aphront/tokenizer.css' => '194e8160', 'rsrc/css/aphront/tooltip.css' => 'e3f2412f', @@ -42,7 +41,7 @@ return array( 'rsrc/css/application/base/main-menu-view.css' => '741a6bba', 'rsrc/css/application/base/notification-menu.css' => '4df1ee30', 'rsrc/css/application/base/phui-theme.css' => '35883b37', - 'rsrc/css/application/base/standard-page-view.css' => 'ab9ca8d8', + 'rsrc/css/application/base/standard-page-view.css' => '6267874d', 'rsrc/css/application/conduit/conduit-api.css' => 'ce2cfc41', 'rsrc/css/application/config/config-options.css' => '16c920ae', 'rsrc/css/application/config/config-template.css' => '20babf50', @@ -138,7 +137,7 @@ return array( 'rsrc/css/phui/phui-action-list.css' => 'd26ccb62', 'rsrc/css/phui/phui-action-panel.css' => '6c386cbf', 'rsrc/css/phui/phui-badge.css' => 'aa49028c', - 'rsrc/css/phui/phui-basic-nav-view.css' => 'a5693cf0', + 'rsrc/css/phui/phui-basic-nav-view.css' => '6b7bd9a5', 'rsrc/css/phui/phui-big-info-view.css' => '362ad37b', 'rsrc/css/phui/phui-box.css' => '5ed3b8cb', 'rsrc/css/phui/phui-bulk-editor.css' => '374d5e30', @@ -792,7 +791,6 @@ return array( 'phabricator-keyboard-shortcut' => '1a844c06', 'phabricator-keyboard-shortcut-manager' => '81debc48', 'phabricator-main-menu-view' => '741a6bba', - 'phabricator-nav-view-css' => '423f92cc', 'phabricator-notification' => 'a9b91e3f', 'phabricator-notification-css' => '30240bd2', 'phabricator-notification-menu-css' => '4df1ee30', @@ -805,7 +803,7 @@ return array( 'phabricator-shaped-request' => '995f5102', 'phabricator-slowvote-css' => '1694baed', 'phabricator-source-code-view-css' => 'e382316a', - 'phabricator-standard-page-view' => 'ab9ca8d8', + 'phabricator-standard-page-view' => '6267874d', 'phabricator-textareautils' => 'f340a484', 'phabricator-title' => '43bc9360', 'phabricator-tooltip' => '83754533', @@ -823,7 +821,7 @@ return array( 'phriction-document-css' => '03380da0', 'phui-action-panel-css' => '6c386cbf', 'phui-badge-view-css' => 'aa49028c', - 'phui-basic-nav-view-css' => 'a5693cf0', + 'phui-basic-nav-view-css' => '6b7bd9a5', 'phui-big-info-view-css' => '362ad37b', 'phui-box-css' => '5ed3b8cb', 'phui-bulk-editor-css' => '374d5e30', @@ -2258,7 +2256,6 @@ return array( 'phui-lightbox-css', 'phui-comment-panel-css', 'phui-header-view-css', - 'phabricator-nav-view-css', 'phui-basic-nav-view-css', 'phui-crumbs-view-css', 'phui-oi-list-view-css', diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php index fc5a4f11b6..944de8cbca 100644 --- a/resources/celerity/packages.php +++ b/resources/celerity/packages.php @@ -124,7 +124,6 @@ return array( 'phui-lightbox-css', 'phui-comment-panel-css', 'phui-header-view-css', - 'phabricator-nav-view-css', 'phui-basic-nav-view-css', 'phui-crumbs-view-css', 'phui-oi-list-view-css', diff --git a/src/view/layout/AphrontSideNavFilterView.php b/src/view/layout/AphrontSideNavFilterView.php index c7c8b5b534..a030bb04e9 100644 --- a/src/view/layout/AphrontSideNavFilterView.php +++ b/src/view/layout/AphrontSideNavFilterView.php @@ -17,7 +17,6 @@ */ final class AphrontSideNavFilterView extends AphrontView { - private $items = array(); private $baseURI; private $selectedFilter = false; private $menu; @@ -199,23 +198,16 @@ final class AphrontSideNavFilterView extends AphrontView { } private function renderFlexNav() { - require_celerity_resource('phabricator-nav-view-css'); - $nav_classes = array(); $nav_classes[] = 'phabricator-nav'; - $nav_id = null; - $drag_id = null; $content_id = celerity_generate_unique_node_id(); $local_id = null; - $background_id = null; $local_menu = null; $main_id = $this->getMainID(); - $nav_menu = null; if ($this->menu->getItems()) { $local_id = celerity_generate_unique_node_id(); - $background_id = celerity_generate_unique_node_id(); $nav_classes[] = 'has-local-nav'; diff --git a/webroot/rsrc/css/aphront/phabricator-nav-view.css b/webroot/rsrc/css/aphront/phabricator-nav-view.css deleted file mode 100644 index 16e6c7ef1c..0000000000 --- a/webroot/rsrc/css/aphront/phabricator-nav-view.css +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @provides phabricator-nav-view-css - */ - -.device .phui-navigation-shell div.phabricator-nav-local { - display: none; -} - -.device-desktop .has-local-nav .phabricator-nav-local { - display: block; -} - -.device-phone .phabricator-side-menu-home .phabricator-nav-local { - display: block; -} - -/* Home Sidenav */ -.phui-basic-nav.phui-navigation-shell - .phabricator-side-menu-home .phabricator-nav-local { - padding-top: 16px; - padding-right: 0; - background-color: transparent; - width: 205px; - max-width: 205px; -} - -.device-phone .phui-basic-nav.phui-navigation-shell - .phabricator-side-menu-home .phabricator-nav-local { - padding-top: 0; - padding-right: 0; - background-color: transparent; - width: auto; -} - - -.phabricator-nav-content { - overflow: hidden; -} - -.device-phone .phabricator-side-menu-home .phabricator-nav-content { - display: none; -} - -.device-phone .phabricator-side-menu-home .phabricator-nav-local { - width: 100%; - padding-top: 0; - margin-top: 0; -} diff --git a/webroot/rsrc/css/application/base/standard-page-view.css b/webroot/rsrc/css/application/base/standard-page-view.css index 2b710dbd94..2938796bff 100644 --- a/webroot/rsrc/css/application/base/standard-page-view.css +++ b/webroot/rsrc/css/application/base/standard-page-view.css @@ -45,10 +45,6 @@ body.white-background { margin-left: 221px; } -.device .phabricator-side-menu-home + .phabricator-standard-page-footer { - display: none; -} - .keyboard-shortcut-help { margin: 4px 12px; } diff --git a/webroot/rsrc/css/phui/phui-basic-nav-view.css b/webroot/rsrc/css/phui/phui-basic-nav-view.css index f718c673ab..d87a1da8a0 100644 --- a/webroot/rsrc/css/phui/phui-basic-nav-view.css +++ b/webroot/rsrc/css/phui/phui-basic-nav-view.css @@ -4,21 +4,19 @@ .device-desktop .phui-navigation-shell, .phabricator-home.device .phui-navigation-shell { - display: table; width: 100%; height: calc(100vh - {$menu.main.height}); } .device-desktop .phui-navigation-shell .phabricator-nav, .phabricator-home.device .phui-navigation-shell .phabricator-nav { - display: table-row; + display: flex; + min-height: 100%; } .device-desktop .phui-navigation-shell .phabricator-nav-local, .phabricator-home.device .phui-navigation-shell .phabricator-nav-local { - display: table-cell; position: relative; - vertical-align: top; margin-top: 0; overflow: hidden; } @@ -35,12 +33,20 @@ } .phui-basic-nav.phui-navigation-shell .phabricator-nav-local { - width: 205px; - max-width: 205px; + flex: 0 0 205px; padding-top: 12px; padding-right: 8px; } +.device .phui-navigation-shell .phabricator-nav-local { + display: none; +} + +.phabricator-nav-content { + flex-grow: 1; + overflow: hidden; +} + .phui-two-column-view .phui-basic-nav.phui-navigation-shell .phabricator-nav-local { padding-right: 0; -- 2.51.2