Fix tab hover card position in vertical tabs (#35246)
Resolves brave/brave-browser#54199 The hover card should sit to the right of the strip in vertical-tab mode (LEFT_TOP). Brave only set the bubble arrow in BraveTabHoverCardController when the card was created or when orientation changed. Chromium keeps the arrow in sync from anchor_target->GetAnchorPosition() on every update, so BraveTab still reporting TOP_LEFT could overwrite the correct arrow. Override BraveTab::GetAnchorPosition() to return LEFT_TOP when vertical tabs are enabled, otherwise use Tab::GetAnchorPosition(). Remove the controller’s SetIsVerticalTabs / UpdateHoverCardArrow path and the BraveTabStrip call. TEST=BraveTabTest.GetAnchorPositionReflectsTabOrientation
This commit is contained in:
@@ -28,6 +28,7 @@
|
||||
#include "ui/base/models/image_model.h"
|
||||
#include "ui/gfx/favicon_size.h"
|
||||
#include "ui/views/animation/ink_drop.h"
|
||||
#include "ui/views/bubble/bubble_border.h"
|
||||
#include "ui/views/controls/button/image_button.h"
|
||||
#include "ui/views/controls/label.h"
|
||||
#include "ui/views/view_class_properties.h"
|
||||
@@ -63,6 +64,15 @@ int BraveTab::GetTreeHeight() const {
|
||||
return 0;
|
||||
}
|
||||
|
||||
views::BubbleBorder::Arrow BraveTab::GetAnchorPosition() const {
|
||||
if (tabs::utils::ShouldShowBraveVerticalTabs(
|
||||
controller()->GetBrowserWindowInterface())) {
|
||||
return views::BubbleBorder::Arrow::LEFT_TOP;
|
||||
}
|
||||
|
||||
return Tab::GetAnchorPosition();
|
||||
}
|
||||
|
||||
std::u16string BraveTab::GetRenderedTooltipText(const gfx::Point& p) const {
|
||||
auto* browser = controller_->GetBrowserWindowInterface();
|
||||
if (browser &&
|
||||
|
||||
@@ -51,6 +51,7 @@ class BraveTab : public Tab {
|
||||
void UpdateTreeToggleButtonIcon();
|
||||
|
||||
// Tab:
|
||||
views::BubbleBorder::Arrow GetAnchorPosition() const override;
|
||||
std::u16string GetRenderedTooltipText(const gfx::Point& p) const override;
|
||||
|
||||
// Overridden because we moved alert button to left side in the tab whereas
|
||||
|
||||
@@ -16,25 +16,9 @@
|
||||
#include "chrome/browser/ui/views/tabs/tab_hover_card_controller.h"
|
||||
#include "chrome/browser/ui/views/tabs/tab_hover_card_thumbnail_observer.h"
|
||||
#include "chrome/browser/ui/views/tabs/tab_strip.h"
|
||||
#include "ui/views/bubble/bubble_border.h"
|
||||
|
||||
BraveTabHoverCardController::~BraveTabHoverCardController() = default;
|
||||
|
||||
void BraveTabHoverCardController::SetIsVerticalTabs(bool is_vertical_tabs) {
|
||||
if (std::exchange(is_vertical_tabs_, is_vertical_tabs) == is_vertical_tabs) {
|
||||
return;
|
||||
}
|
||||
|
||||
UpdateHoverCardArrow();
|
||||
}
|
||||
|
||||
void BraveTabHoverCardController::UpdateHoverCardArrow() {
|
||||
if (hover_card_) {
|
||||
hover_card_->SetArrow(is_vertical_tabs_ ? views::BubbleBorder::LEFT_TOP
|
||||
: views::BubbleBorder::TOP_CENTER);
|
||||
}
|
||||
}
|
||||
|
||||
void BraveTabHoverCardController::CreateHoverCard(
|
||||
HoverCardAnchorTarget* anchor_target) {
|
||||
hover_card_image_previews_enabled_ =
|
||||
@@ -43,8 +27,6 @@ void BraveTabHoverCardController::CreateHoverCard(
|
||||
browser_window_interface_->GetProfile()->GetPrefs());
|
||||
|
||||
TabHoverCardController::CreateHoverCard(anchor_target);
|
||||
|
||||
UpdateHoverCardArrow();
|
||||
}
|
||||
|
||||
void BraveTabHoverCardController::OnHovercardImagesEnabledChanged() {
|
||||
|
||||
@@ -15,17 +15,11 @@ class BraveTabHoverCardController : public TabHoverCardController {
|
||||
using TabHoverCardController::TabHoverCardController;
|
||||
~BraveTabHoverCardController() override;
|
||||
|
||||
void SetIsVerticalTabs(bool is_vertical_tabs);
|
||||
|
||||
protected:
|
||||
void UpdateHoverCardArrow();
|
||||
|
||||
void OnHovercardImagesEnabledChanged() override;
|
||||
|
||||
// TabHoverCardController:
|
||||
void CreateHoverCard(HoverCardAnchorTarget* anchor_target) override;
|
||||
|
||||
bool is_vertical_tabs_ = false;
|
||||
};
|
||||
|
||||
#endif // BRAVE_BROWSER_UI_VIEWS_TABS_BRAVE_TAB_HOVER_CARD_CONTROLLER_H_
|
||||
|
||||
@@ -326,8 +326,6 @@ void BraveTabStrip::UpdateOrientation() {
|
||||
SetAvailableWidthCallback(base::NullCallback());
|
||||
}
|
||||
|
||||
hover_card_controller_->SetIsVerticalTabs(using_vertical_tabs);
|
||||
|
||||
if (const auto active_index = GetActiveIndex(); active_index) {
|
||||
// In order to update shadow state, call ActiveStateChanged().
|
||||
tab_at(active_index.value())->ActiveStateChanged();
|
||||
|
||||
@@ -28,6 +28,7 @@
|
||||
#include "ui/gfx/geometry/insets.h"
|
||||
#include "ui/gfx/geometry/rect.h"
|
||||
#include "ui/gfx/geometry/skia_conversions.h"
|
||||
#include "ui/views/bubble/bubble_border.h"
|
||||
#include "ui/views/test/views_test_utils.h"
|
||||
|
||||
class MockTabSlotController : public FakeTabSlotController {
|
||||
@@ -521,3 +522,36 @@ TEST_F(BraveTabTest, GetTabSizeInfoFullMinWidthModeUsesStandardWidth) {
|
||||
EXPECT_EQ(info.min_active_width, std_w);
|
||||
EXPECT_EQ(info.min_inactive_width, std_w);
|
||||
}
|
||||
|
||||
// BraveTab::GetAnchorPosition() must return LEFT_TOP in vertical tab mode so
|
||||
// that the hover card appears to the right of the tab strip, and must
|
||||
// delegate to Tab::GetAnchorPosition() (TOP_LEFT) in horizontal mode.
|
||||
TEST_F(BraveTabTest, GetAnchorPositionReflectsTabOrientation) {
|
||||
TestingProfile profile;
|
||||
|
||||
testing::NiceMock<MockBrowserWindowInterface> mock_browser_window;
|
||||
EXPECT_CALL(mock_browser_window, GetProfile())
|
||||
.WillRepeatedly(testing::Return(&profile));
|
||||
EXPECT_CALL(testing::Const(mock_browser_window), GetProfile())
|
||||
.WillRepeatedly(testing::Return(&profile));
|
||||
EXPECT_CALL(mock_browser_window, GetType())
|
||||
.WillRepeatedly(
|
||||
testing::Return(BrowserWindowInterface::Type::TYPE_NORMAL));
|
||||
|
||||
testing::NiceMock<MockTabSlotController> tab_slot_controller;
|
||||
EXPECT_CALL(tab_slot_controller, GetBrowserWindowInterface())
|
||||
.WillRepeatedly(testing::Return(&mock_browser_window));
|
||||
|
||||
BraveTab tab(tabs::TabHandle(1), &tab_slot_controller);
|
||||
|
||||
// Horizontal tabs: delegate to Tab::GetAnchorPosition() which returns
|
||||
// TOP_LEFT.
|
||||
profile.GetPrefs()->SetBoolean(brave_tabs::kVerticalTabsEnabled, false);
|
||||
EXPECT_EQ(views::BubbleBorder::Arrow::TOP_LEFT, tab.GetAnchorPosition());
|
||||
|
||||
// Vertical tabs: must return LEFT_TOP so the hover card appears to the
|
||||
// right of the tab strip for both the first hover (card creation) and
|
||||
// subsequent hovers (card update via UpdateOrShowCard).
|
||||
profile.GetPrefs()->SetBoolean(brave_tabs::kVerticalTabsEnabled, true);
|
||||
EXPECT_EQ(views::BubbleBorder::Arrow::LEFT_TOP, tab.GetAnchorPosition());
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user