Merge pull request #21989 from brave/ksmith-first-tab-hit

Allow first tab to be selected at edge of screen in fullscreen
brave · Feb 12, 2024 · a1c9b64 · a1c9b64
2 parents 136b79d + 2997f1b
commit a1c9b64
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git a/browser/ui/views/frame/brave_browser_view_layout.cc b/browser/ui/views/frame/brave_browser_view_layout.cc
@@ -9,6 +9,8 @@
 #include <limits>
 
 #include "brave/browser/ui/brave_browser.h"
+#include "brave/browser/ui/tabs/brave_tab_layout_constants.h"
+#include "brave/browser/ui/tabs/features.h"
 #include "brave/browser/ui/views/frame/brave_contents_view_util.h"
 #include "brave/browser/ui/views/sidebar/sidebar_container_view.h"
 #include "brave/browser/ui/views/tabs/vertical_tab_utils.h"
@@ -20,6 +22,7 @@
 #include "chrome/browser/ui/views/bookmarks/bookmark_bar_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/browser_view_layout_delegate.h"
+#include "chrome/browser/ui/views/frame/tab_strip_region_view.h"
 #include "chrome/browser/ui/views/infobars/infobar_container_view.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "ui/views/border.h"
@@ -141,7 +144,28 @@ int BraveBrowserViewLayout::LayoutTabStripRegion(int top) {
     return top;
   }
 
-  return BrowserViewLayout::LayoutTabStripRegion(top);
+  top = BrowserViewLayout::LayoutTabStripRegion(top);
+
+  if (tabs::features::HorizontalTabsUpdateEnabled()) {
+    // If we are not maximized or fullscreen, then insert a medium sized space
+    // between the tab strip and the left (or right, depending upon RTL) edge of
+    // the browser. In maximized or fullscreen mode, we don't want a margin
+    // because the user should be able to select the first tab by clicking the
+    // edge of the screen.
+    if (!browser_view_->IsFullscreen() && !browser_view_->IsMaximized()) {
+      constexpr int kTabStripMargin =
+          brave_tabs::kHorizontalTabStripLeftMargin -
+          brave_tabs::kHorizontalTabInset;
+      static_assert(kTabStripMargin >= 0,
+                    "The tabstrip margin cannot be less than 0.");
+      auto tab_strip_region_view_bounds = tab_strip_region_view_->bounds();
+      tab_strip_region_view_bounds.Inset(
+          gfx::Insets::TLBR(0, kTabStripMargin, 0, 0));
+      tab_strip_region_view_->SetBoundsRect(tab_strip_region_view_bounds);
+    }
+  }
+
+  return top;
 }
 
 int BraveBrowserViewLayout::LayoutBookmarkAndInfoBars(int top,

diff --git a/browser/ui/views/tabs/brave_tab_strip.cc b/browser/ui/views/tabs/brave_tab_strip.cc
@@ -12,7 +12,6 @@
 #include "brave/browser/profiles/profile_util.h"
 #include "brave/browser/themes/brave_dark_mode_utils.h"
 #include "brave/browser/ui/color/brave_color_id.h"
-#include "brave/browser/ui/tabs/brave_tab_layout_constants.h"
 #include "brave/browser/ui/tabs/brave_tab_prefs.h"
 #include "brave/browser/ui/tabs/features.h"
 #include "brave/browser/ui/tabs/shared_pinned_tab_service.h"
@@ -167,8 +166,6 @@ void BraveTabStrip::MaybeStartDrag(
 void BraveTabStrip::AddedToWidget() {
   TabStrip::AddedToWidget();
 
-  UpdateTabStripMargins();
-
   if (BrowserView::GetBrowserViewForBrowser(GetBrowser())) {
     UpdateTabContainer();
   } else {
@@ -381,25 +378,6 @@ void BraveTabStrip::UpdateTabContainer() {
   }
 }
 
-void BraveTabStrip::UpdateTabStripMargins() {
-  if (!tabs::features::HorizontalTabsUpdateEnabled()) {
-    return;
-  }
-
-  gfx::Insets margins;
-
-  if (!ShouldShowVerticalTabs()) {
-    // There should be a medium size gap between the left edge of the tabstrip
-    // and the visual left edge of the first tab. Set a left margin that takes
-    // into account the visual tab inset.
-    margins.set_left(brave_tabs::kHorizontalTabStripLeftMargin -
-                     brave_tabs::kHorizontalTabInset);
-    DCHECK_GE(margins.left(), 0);
-  }
-
-  SetProperty(views::kMarginsKey, margins);
-}
-
 bool BraveTabStrip::ShouldShowVerticalTabs() const {
   return tabs::utils::ShouldShowVerticalTabs(GetBrowser());
 }

diff --git a/browser/ui/views/tabs/brave_tab_strip.h b/browser/ui/views/tabs/brave_tab_strip.h
@@ -38,7 +38,6 @@ class BraveTabStrip : public TabStrip {
   FRIEND_TEST_ALL_PREFIXES(VerticalTabStripBrowserTest, ScrollBarVisibility);
 
   void UpdateTabContainer();
-  void UpdateTabStripMargins();
   bool ShouldShowVerticalTabs() const;
 
   TabContainer* GetTabContainerForTesting();

diff --git a/browser/ui/views/tabs/brave_tab_style_views.inc.cc b/browser/ui/views/tabs/brave_tab_style_views.inc.cc
@@ -129,6 +129,17 @@ SkPath BraveVerticalTabStyle::GetPath(
         hit_test_outsets.set_top(0);
       }
 
+      // We also want the first tab (taking RTL into account) to be selectable
+      // in maximized or fullscreen mode by clicking at the very edge of the
+      // screen.
+      if (ShouldExtendHitTest() && tab()->controller()->IsTabFirst(tab())) {
+        if (tab()->GetMirrored()) {
+          hit_test_outsets.set_right(brave_tabs::kHorizontalTabInset * scale);
+        } else {
+          hit_test_outsets.set_left(brave_tabs::kHorizontalTabInset * scale);
+        }
+      }
+
       aligned_bounds.Outset(hit_test_outsets);
     }
   }