Skip to content

Commit

Permalink
Fixed contextual panel item is not activated after attaching to window
Browse files Browse the repository at this point in the history
fix brave/brave-browser#41782

We called SidebarContainerView::WillShowSidePanel() to observer
tab's contextual entry/global entry when panel starts to show
from some places. But there was corner-case that WillShowSidePanel()
is not called when panel is shown. It happened when tab is attached
to another window. And realized that PopulateSidePanel() is always
called whenever panel is shown in whatever situation.
Fixed by calling WillShowSidePanel() from PopulateSidePanel().
  • Loading branch information
simonhong committed Oct 22, 2024
1 parent 74374a2 commit 3eb965f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 18 deletions.
15 changes: 15 additions & 0 deletions browser/ui/sidebar/sidebar_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,21 @@ IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat,
EXPECT_TRUE(model()->active_index().has_value());
EXPECT_EQ(model()->active_index(),
model()->GetIndexOf(SidebarItem::BuiltInItemType::kBookmarks));

// Check tab specific panel item is still activated after moving to another
// browser.
tab_model()->ActivateTabAt(1);
EXPECT_EQ(model()->active_index(), tab_specific_item_index);

auto* browser2 = CreateBrowser(browser()->profile());
auto* browser2_model =
static_cast<BraveBrowser*>(browser2)->sidebar_controller()->model();
auto* browser2_tab_model = browser2->tab_strip_model();

auto detached_tab = tab_model()->DetachTabAtForInsertion(1);
browser2_tab_model->AppendTab(std::move(detached_tab), /* foreground */ true);
EXPECT_EQ(browser2_model->active_index(),
browser2_model->GetIndexOf(SidebarItem::BuiltInItemType::kChatUI));
}

IN_PROC_BROWSER_TEST_F(SidebarBrowserTestWithAIChat,
Expand Down
22 changes: 5 additions & 17 deletions browser/ui/views/side_panel/brave_side_panel_coordinator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ void BraveSidePanelCoordinator::Show(
sidebar::SetLastUsedSidePanel(browser_view_->GetProfile()->GetPrefs(),
entry_key.id());

// Notify to give opportunity to observe another panel entries from
// global or active tab's contextual registry.
auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
brave_browser_view->WillShowSidePanel();

SidePanelCoordinator::Show(entry_key, open_trigger);
}

Expand Down Expand Up @@ -91,13 +86,6 @@ void BraveSidePanelCoordinator::Toggle() {
void BraveSidePanelCoordinator::Toggle(
SidePanelEntryKey key,
SidePanelUtil::SidePanelOpenTrigger open_trigger) {
// Notify to give opportunity to observe another panel entries from
// global or active tab's contextual registry.
if (!IsSidePanelShowing()) {
auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
brave_browser_view->WillShowSidePanel();
}

SidePanelCoordinator::Toggle(key, open_trigger);
}

Expand Down Expand Up @@ -160,18 +148,18 @@ void BraveSidePanelCoordinator::PopulateSidePanel(
return;
}

// Notify to give opportunity to observe another panel entries from
// global or active tab's contextual registry.
auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
brave_browser_view->WillShowSidePanel();

SidePanelCoordinator::PopulateSidePanel(supress_animations, unique_key, entry,
std::move(content_view));
}

void BraveSidePanelCoordinator::NotifyPinnedContainerOfActiveStateChange(
SidePanelEntryKey key,
bool is_active) {
// // Notify to give opportunity to observe another panel entries from
// // global or active tab's contextual registry.
// auto* brave_browser_view = static_cast<BraveBrowserView*>(browser_view_);
// brave_browser_view->WillShowSidePanel();

if (!browser_view_->toolbar()->pinned_toolbar_actions_container()) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion browser/ui/views/side_panel/brave_side_panel_coordinator.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include <memory>
#include <optional>

#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h" // IWYU pragma: export
#include "chrome/browser/ui/views/side_panel/side_panel_coordinator.h"

class BraveSidePanelCoordinator : public SidePanelCoordinator {
public:
Expand Down

0 comments on commit 3eb965f

Please sign in to comment.