diff --git a/browser/ui/views/frame/brave_tab_strip_region_view.cc b/browser/ui/views/frame/brave_tab_strip_region_view.cc index 11762a01855..a1f93efdb87 100644 --- a/browser/ui/views/frame/brave_tab_strip_region_view.cc +++ b/browser/ui/views/frame/brave_tab_strip_region_view.cc @@ -5,8 +5,6 @@ #include "brave/browser/ui/views/frame/brave_tab_strip_region_view.h" -#include - #include "base/feature_list.h" #include "base/functional/bind.h" #include "brave/browser/ui/tabs/brave_tab_prefs.h" @@ -29,12 +27,123 @@ #include "components/prefs/pref_service.h" #include "components/vector_icons/vector_icons.h" #include "ui/base/l10n/l10n_util.h" +#include "ui/base/metadata/metadata_header_macros.h" #include "ui/base/metadata/metadata_impl_macros.h" +#include "ui/events/event.h" +#include "ui/views/controls/button/button.h" +#include "ui/views/controls/button/button_controller.h" #include "ui/views/layout/flex_layout_types.h" +#include "ui/views/repeat_controller.h" #include "ui/views/view_class_properties.h" #include "ui/views/view_utils.h" namespace { +// Pattern from ui/views/controls/scrollbar/scroll_bar_button.h (scroll bar +// line/page repeat). Uses kOnPress so the primary action is not fired again on +// release. +class BraveTabStripScrollButton : public TabStripControlButton { + METADATA_HEADER(BraveTabStripScrollButton, TabStripControlButton) + + public: + BraveTabStripScrollButton(BrowserWindowInterface* browser_window_interface, + base::RepeatingClosure scroll_action, + const gfx::VectorIcon& icon); + + BraveTabStripScrollButton(const BraveTabStripScrollButton&) = delete; + BraveTabStripScrollButton& operator=(const BraveTabStripScrollButton&) = + delete; + + ~BraveTabStripScrollButton() override; + + // views::View + bool OnMousePressed(const ui::MouseEvent& event) override; + void OnMouseReleased(const ui::MouseEvent& event) override; + void OnMouseCaptureLost() override; + void OnGestureEvent(ui::GestureEvent* event) override; + + private: + void OnRepeaterFired(); + + base::RepeatingClosure scroll_action_; + views::RepeatController repeater_; +}; + +BraveTabStripScrollButton::BraveTabStripScrollButton( + BrowserWindowInterface* browser_window_interface, + base::RepeatingClosure scroll_action, + const gfx::VectorIcon& icon) + : TabStripControlButton(browser_window_interface, + views::Button::PressedCallback(scroll_action), + icon, + Edge::kNone, + Edge::kNone), + scroll_action_(std::move(scroll_action)), + repeater_(base::BindRepeating(&BraveTabStripScrollButton::OnRepeaterFired, + base::Unretained(this))) { + button_controller()->set_notify_action( + views::ButtonController::NotifyAction::kOnPress); +} + +BraveTabStripScrollButton::~BraveTabStripScrollButton() { + repeater_.Stop(); +} + +void BraveTabStripScrollButton::OnRepeaterFired() { + scroll_action_.Run(); +} + +bool BraveTabStripScrollButton::OnMousePressed(const ui::MouseEvent& event) { + const bool result = TabStripControlButton::OnMousePressed(event); + if (GetState() != views::Button::STATE_DISABLED && + event.IsOnlyLeftMouseButton()) { + repeater_.Start(); + } + return result; +} + +void BraveTabStripScrollButton::OnMouseReleased(const ui::MouseEvent& event) { + repeater_.Stop(); + TabStripControlButton::OnMouseReleased(event); +} + +void BraveTabStripScrollButton::OnMouseCaptureLost() { + repeater_.Stop(); + TabStripControlButton::OnMouseCaptureLost(); +} + +void BraveTabStripScrollButton::OnGestureEvent(ui::GestureEvent* event) { + if (GetState() == views::Button::STATE_DISABLED) { + TabStripControlButton::OnGestureEvent(event); + return; + } + + if (event->type() == ui::EventType::kGestureTapDown) { + TabStripControlButton::OnGestureEvent(event); + if (GetState() == views::Button::STATE_PRESSED) { + scroll_action_.Run(); + repeater_.Start(); + } + event->SetHandled(); + return; + } + + if (event->type() == ui::EventType::kGestureLongPress) { + return; + } + + repeater_.Stop(); + + if (event->type() == ui::EventType::kGestureTap) { + SetState(views::Button::STATE_HOVERED); + event->SetHandled(); + return; + } + + TabStripControlButton::OnGestureEvent(event); +} + +BEGIN_METADATA(BraveTabStripScrollButton) +END_METADATA #if BUILDFLAG(IS_LINUX) ui::DropTargetEvent ConvertRootLocation(views::View* view, @@ -69,20 +178,20 @@ void BraveHorizontalTabStripRegionView::CreateScrollButtonsIfNeeded() { // Child order for FlexLayout: leading scroll, tab strip, trailing scroll, // then (via base layout) new tab button after the strip cluster. tab_scroll_next_button_ = AddChildViewAt( - std::make_unique( + std::make_unique( bwi, base::BindRepeating( &BraveHorizontalTabStripRegionView::OnScrollNextPressed, weak_factory_.GetWeakPtr()), - vector_icons::kForwardArrowIcon, Edge::kNone, Edge::kNone), + vector_icons::kForwardArrowIcon), strip_idx.value() + 1); tab_scroll_previous_button_ = AddChildViewAt( - std::make_unique( + std::make_unique( bwi, base::BindRepeating( &BraveHorizontalTabStripRegionView::OnScrollPreviousPressed, weak_factory_.GetWeakPtr()), - vector_icons::kBackArrowIcon, Edge::kNone, Edge::kNone), + vector_icons::kBackArrowIcon), strip_idx.value()); tab_scroll_previous_button_->SetProperty(views::kCrossAxisAlignmentKey, diff --git a/browser/ui/views/tabs/BUILD.gn b/browser/ui/views/tabs/BUILD.gn index bdc3f13d22e..cb2dbce27db 100644 --- a/browser/ui/views/tabs/BUILD.gn +++ b/browser/ui/views/tabs/BUILD.gn @@ -31,6 +31,7 @@ source_set("browser_tests") { "//chrome/test:test_support", "//components/prefs", "//ui/base:test_support", + "//ui/events:test_support", ] if (use_ozone) { diff --git a/browser/ui/views/tabs/brave_tab_container_browsertest.cc b/browser/ui/views/tabs/brave_tab_container_browsertest.cc index 6a9de599530..0b0436c15a9 100644 --- a/browser/ui/views/tabs/brave_tab_container_browsertest.cc +++ b/browser/ui/views/tabs/brave_tab_container_browsertest.cc @@ -5,6 +5,11 @@ #include "brave/browser/ui/views/tabs/brave_tab_container.h" +#include + +#include "base/location.h" +#include "base/run_loop.h" +#include "base/task/sequenced_task_runner.h" #include "base/test/run_until.h" #include "base/test/scoped_feature_list.h" #include "brave/browser/ui/tabs/brave_tab_prefs.h" @@ -29,7 +34,10 @@ #include "testing/gtest/include/gtest/gtest.h" #include "ui/events/base_event_utils.h" #include "ui/events/event_constants.h" +#include "ui/events/test/event_generator.h" +#include "ui/views/repeat_controller.h" #include "ui/views/view_utils.h" +#include "ui/views/widget/widget_utils.h" #include "url/url_constants.h" class HorizontalScrollableTabStripBrowserTest : public InProcessBrowserTest { @@ -77,6 +85,27 @@ class HorizontalScrollableTabStripBrowserTest : public InProcessBrowserTest { tab_strip->parent()); } + // Grows the strip so at least |n| scroll-button steps can apply before + // clamping (viewport/4 as step is often greater than the max offset after + // the first few overflow tabs). + void GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep( + BraveTabContainer* container, + int n, + base::Location location = base::Location::Current()) { + SCOPED_TRACE(location.ToString()); + for (int i = 0; i < 100; i++) { + const int max_offset = container->GetMaxScrollOffsetForTesting(); + const int step = container->GetHorizontalTabScrollStep(); + if (step > 0 && max_offset >= n * step) { + return; + } + AppendTab(); + StopAnimatingAndLayout(); + } + GTEST_FAIL() + << "Failed to get enough scroll headroom for repeat/hold test."; + } + void SetUpOnMainThread() override { InProcessBrowserTest::SetUpOnMainThread(); // Horizontal scrolling requires kBraveScrollableTabStrip and this pref. @@ -410,3 +439,109 @@ IN_PROC_BROWSER_TEST_F(HorizontalScrollableTabStripBrowserTest, ASSERT_TRUE(region->tab_scroll_previous_for_testing()); EXPECT_FALSE(region->tab_scroll_previous_for_testing()->GetVisible()); } + +IN_PROC_BROWSER_TEST_F( + HorizontalScrollableTabStripBrowserTest, + HorizontalScrollPreviousButtonSingleClickScrollsByOneStep) { + auto* tab_strip = views::AsViewClass( + browser_view()->horizontal_tab_strip_for_testing()); + BraveTabContainer* container = views::AsViewClass( + tab_strip->GetTabContainerForTesting()); + ASSERT_TRUE(container); + + browser()->profile()->GetPrefs()->SetBoolean( + brave_tabs::kShowHorizontalTabScrollButtons, true); + StopAnimatingAndLayout(); + GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep(container, 2); + + BraveHorizontalTabStripRegionView* region = tab_strip_region(); + ASSERT_TRUE(region); + const int max_scroll_offset = container->GetMaxScrollOffsetForTesting(); + ASSERT_GT(max_scroll_offset, 0); + // Exercise the leading (back) control at max scroll. The trailing (forward) + // button is disabled at the end; growing many tabs often leaves the last + // tab active with the strip scrolled to the end. + browser()->tab_strip_model()->ActivateTabAt( + std::max(0, browser()->tab_strip_model()->count() - 1)); + container->SetScrollOffsetForTesting(max_scroll_offset); + StopAnimatingAndLayout(); + + TabStripControlButton* back = region->tab_scroll_previous_for_testing(); + ASSERT_TRUE(back); + ASSERT_TRUE(back->GetVisible()); + ASSERT_TRUE(back->GetEnabled()); + + const int step = container->GetHorizontalTabScrollStep(); + ASSERT_GT(step, 0); + + const int before = container->GetScrollOffsetForTesting(); + + ui::test::EventGenerator event_generator( + views::GetRootWindow(browser_view()->GetWidget()), + browser_view()->GetNativeWindow()); + event_generator.MoveMouseTo(back->GetBoundsInScreen().CenterPoint()); + event_generator.PressLeftButton(); + event_generator.ReleaseLeftButton(); + StopAnimatingAndLayout(); + + EXPECT_EQ(before - container->GetScrollOffsetForTesting(), step) + << "before: " << before + << " after: " << container->GetScrollOffsetForTesting() + << " One press+release should match one scroll action (not double on " + "press, " + "repeater should not have fired)."; +} + +IN_PROC_BROWSER_TEST_F( + HorizontalScrollableTabStripBrowserTest, + HorizontalScrollPreviousButtonHoldScrollsByMoreThanOneStep) { + auto* tab_strip = views::AsViewClass( + browser_view()->horizontal_tab_strip_for_testing()); + BraveTabContainer* container = views::AsViewClass( + tab_strip->GetTabContainerForTesting()); + ASSERT_TRUE(container); + + browser()->profile()->GetPrefs()->SetBoolean( + brave_tabs::kShowHorizontalTabScrollButtons, false); + browser()->profile()->GetPrefs()->SetBoolean( + brave_tabs::kShowHorizontalTabScrollButtons, true); + StopAnimatingAndLayout(); + GrowHorizontalStripUntilMaxOffsetAtLeastNTimesStep(container, 2); + + const int max_scroll_offset = container->GetMaxScrollOffsetForTesting(); + const int step = container->GetHorizontalTabScrollStep(); + ASSERT_GT(step, 0); + ASSERT_GE(max_scroll_offset, 2 * step); + + BraveHorizontalTabStripRegionView* region = tab_strip_region(); + ASSERT_TRUE(region); + browser()->tab_strip_model()->ActivateTabAt( + std::max(0, browser()->tab_strip_model()->count() - 1)); + container->SetScrollOffsetForTesting(max_scroll_offset); + StopAnimatingAndLayout(); + + TabStripControlButton* back = region->tab_scroll_previous_for_testing(); + ASSERT_TRUE(back); + ASSERT_TRUE(back->GetVisible()); + ASSERT_TRUE(back->GetEnabled()); + + const int before = container->GetScrollOffsetForTesting(); + + ui::test::EventGenerator event_generator( + views::GetRootWindow(browser_view()->GetWidget()), + browser_view()->GetNativeWindow()); + event_generator.MoveMouseTo(back->GetBoundsInScreen().CenterPoint()); + event_generator.PressLeftButton(); + base::RunLoop run_loop; + base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask( + FROM_HERE, run_loop.QuitClosure(), + views::RepeatController::GetInitialWaitForTesting() + + views::RepeatController::GetRepeatingWaitForTesting() * 2); + run_loop.Run(); + event_generator.ReleaseLeftButton(); + StopAnimatingAndLayout(); + + const int after = container->GetScrollOffsetForTesting(); + EXPECT_GT(before - after, step) + << "Holding should run the repeat timer and scroll more than one action."; +}