Skip to content

Commit

Permalink
Makes sidebar item handles window disposition event
Browse files Browse the repository at this point in the history
fix brave/brave-browser#21157

Same with bookmarks button behavior.
* Ctrl + click, middle click - opens in new background tab
* Shift + click - opens in new window.
  • Loading branch information
simonhong authored and bsclifton committed Feb 22, 2022
1 parent f8ef5fc commit e0118b6
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 8 deletions.
12 changes: 11 additions & 1 deletion browser/ui/sidebar/sidebar_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ bool SidebarController::DoesBrowserHaveOpenedTabForItem(
return !GetAllExistingTabIndexForHost(browser_, item.url.host()).empty();
}

void SidebarController::ActivateItemAt(int index) {
void SidebarController::ActivateItemAt(int index,
WindowOpenDisposition disposition) {
// -1 means there is no active item.
DCHECK_GE(index, -1);
if (index == -1) {
Expand All @@ -76,6 +77,15 @@ void SidebarController::ActivateItemAt(int index) {
return;
}

if (disposition != WindowOpenDisposition::UNKNOWN) {
NavigateParams params(browser_->profile(), item.url,
ui::PAGE_TRANSITION_AUTO_BOOKMARK);
params.disposition = disposition;
params.browser = browser_;
Navigate(&params);
return;
}

// Iterate whenever builtin panel icon clicks.
if (IsBuiltInType(item)) {
IterateOrLoadAtActiveTab(item.url);
Expand Down
12 changes: 9 additions & 3 deletions browser/ui/sidebar/sidebar_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
#include <memory>
#include <string>

#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "brave/components/sidebar/sidebar_service.h"
#include "ui/base/window_open_disposition.h"

class BraveBrowser;
class GURL;
Expand All @@ -37,7 +39,11 @@ class SidebarController : public SidebarService::Observer {
SidebarController(const SidebarController&) = delete;
SidebarController& operator=(const SidebarController&) = delete;

void ActivateItemAt(int index);
// |disposition| is only valid for shortcut type.
// If it's unknown, it's activated at active tab.
void ActivateItemAt(
int index,
WindowOpenDisposition disposition = WindowOpenDisposition::UNKNOWN);
void AddItemWithCurrentTab();
// If current browser doesn't have a tab for |url|, active tab will load
// |url|. Otherwise, existing tab will be activated.
Expand Down Expand Up @@ -66,9 +72,9 @@ class SidebarController : public SidebarService::Observer {
// Otherwise, load URL in the active tab.
void IterateOrLoadAtActiveTab(const GURL& url);

BraveBrowser* browser_ = nullptr;
raw_ptr<BraveBrowser> browser_ = nullptr;
// Interface to view.
Sidebar* sidebar_ = nullptr;
raw_ptr<Sidebar> sidebar_ = nullptr;

std::unique_ptr<SidebarModel> sidebar_model_;
base::ScopedObservation<SidebarService, SidebarService::Observer>
Expand Down
7 changes: 7 additions & 0 deletions browser/ui/views/sidebar/sidebar_item_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "brave/browser/themes/theme_properties.h"
#include "brave/grit/brave_theme_resources.h"
#include "chrome/browser/ui/views/event_utils.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/theme_provider.h"
#include "ui/gfx/canvas.h"
Expand Down Expand Up @@ -60,6 +61,12 @@ void SidebarItemView::OnPaintBorder(gfx::Canvas* canvas) {
}
}

bool SidebarItemView::IsTriggerableEvent(const ui::Event& e) {
return e.type() == ui::ET_GESTURE_TAP ||
e.type() == ui::ET_GESTURE_TAP_DOWN ||
event_utils::IsPossibleDispositionEvent(e);
}

void SidebarItemView::OnPaintBackground(gfx::Canvas* canvas) {
SidebarButtonView::OnPaintBackground(canvas);

Expand Down
1 change: 1 addition & 0 deletions browser/ui/views/sidebar/sidebar_item_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class SidebarItemView : public SidebarButtonView {
// views::ImageButton overrides:
void OnPaintBackground(gfx::Canvas* canvas) override;
void OnPaintBorder(gfx::Canvas* canvas) override;
bool IsTriggerableEvent(const ui::Event& e) override;

private:
bool draw_highlight_ = false;
Expand Down
12 changes: 10 additions & 2 deletions browser/ui/views/sidebar/sidebar_items_contents_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@
#include "brave/grit/brave_theme_resources.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/views/event_utils.h"
#include "components/prefs/pref_service.h"
#include "ui/base/default_style.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/base/theme_provider.h"
#include "ui/base/window_open_disposition.h"
#include "ui/compositor/compositor.h"
#include "ui/events/event.h"
#include "ui/gfx/canvas.h"
#include "ui/gfx/image/image_skia_operations.h"
#include "ui/gfx/paint_vector_icon.h"
Expand Down Expand Up @@ -383,7 +386,8 @@ void SidebarItemsContentsView::UpdateItemViewStateAt(int index, bool active) {
}
}

void SidebarItemsContentsView::OnItemPressed(const views::View* item) {
void SidebarItemsContentsView::OnItemPressed(const views::View* item,
const ui::Event& event) {
auto* controller = browser_->sidebar_controller();
const int index = GetIndexOf(item);
if (controller->IsActiveIndex(index)) {
Expand All @@ -393,7 +397,11 @@ void SidebarItemsContentsView::OnItemPressed(const views::View* item) {
return;
}

controller->ActivateItemAt(index);
WindowOpenDisposition open_disposition = WindowOpenDisposition::UNKNOWN;
if (event_utils::IsPossibleDispositionEvent(event))
open_disposition = ui::DispositionFromEventFlags(event.flags());

controller->ActivateItemAt(index, open_disposition);
}

gfx::ImageSkia SidebarItemsContentsView::GetImageForBuiltInItems(
Expand Down
3 changes: 1 addition & 2 deletions browser/ui/views/sidebar/sidebar_items_contents_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ class SidebarItemsContentsView : public views::View,
bool IsBuiltInTypeItemView(views::View* view) const;

// Called when each item is pressed.
void OnItemPressed(const views::View* item);

void OnItemPressed(const views::View* item, const ui::Event& event);
void OnContextMenuClosed();

gfx::ImageSkia GetImageForBuiltInItems(
Expand Down

0 comments on commit e0118b6

Please sign in to comment.