Skip to content

Commit

Permalink
Tab Organization: Show WebUI on chip click
Browse files Browse the repository at this point in the history
Bug: 1469126
Change-Id: I35ff70b9fea5e4aca2628cfbd10f41f8c03451d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4944208
Commit-Queue: Emily Shack <emshack@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Charles Meng <charlesmeng@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1211238}
  • Loading branch information
Emily Shack authored and Chromium LUCI CQ committed Oct 18, 2023
1 parent 12e10e4 commit 2b3f388
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 3 deletions.
3 changes: 2 additions & 1 deletion chrome/browser/resources/tab_search/app.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
<cr-tabs
tab-names="[[tabNames_]]"
tab-icons="[[tabIcons_]]"
selected="{{selectedTabIndex_}}">
selected="{{selectedTabIndex_}}"
on-selected-changed="onSelectedTabChanged_">
</cr-tabs>
<iron-pages selected="[[selectedTabIndex_]]">
<tab-search-page></tab-search-page>
Expand Down
8 changes: 7 additions & 1 deletion chrome/browser/resources/tab_search/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
import {PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {getTemplate} from './app.html.js';
import {TabSearchApiProxy, TabSearchApiProxyImpl} from './tab_search_api_proxy.js';

export class TabSearchAppElement extends PolymerElement {
static get is() {
Expand All @@ -22,7 +23,7 @@ export class TabSearchAppElement extends PolymerElement {
return {
selectedTabIndex_: {
type: Number,
value: 0,
value: loadTimeData.getInteger('tabIndex'),
},

tabNames_: {
Expand All @@ -47,6 +48,7 @@ export class TabSearchAppElement extends PolymerElement {
};
}

private apiProxy_: TabSearchApiProxy = TabSearchApiProxyImpl.getInstance();
private selectedTabIndex_: number;
private tabNames_: string[];
private tabIcons_: string[];
Expand All @@ -55,6 +57,10 @@ export class TabSearchAppElement extends PolymerElement {
static get template() {
return getTemplate();
}

private onSelectedTabChanged_(event: CustomEvent<{value: number}>) {
this.apiProxy_.setTabIndex(event.detail.value);
}
}

declare global {
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/resources/tab_search/tab_search_api_proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export interface TabSearchApiProxy {

saveRecentlyClosedExpandedPref(expanded: boolean): void;

setTabIndex(index: number): void;

showUi(): void;
}

Expand Down Expand Up @@ -104,6 +106,10 @@ export class TabSearchApiProxyImpl implements TabSearchApiProxy {
this.handler.saveRecentlyClosedExpandedPref(expanded);
}

setTabIndex(index: number) {
this.handler.setTabIndex(index);
}

showUi() {
this.handler.showUI();
}
Expand Down
20 changes: 19 additions & 1 deletion chrome/browser/ui/views/tabs/tab_organization_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
#include "chrome/browser/ui/views/tabs/tab_organization_button.h"

#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_element_identifiers.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/tabs/organization/tab_organization_session.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
#include "chrome/browser/ui/views/tab_search_bubble_host.h"
#include "chrome/browser/ui/views/tabs/tab_strip_controller.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_prefs.h"
#include "chrome/grit/generated_resources.h"
#include "components/vector_icons/vector_icons.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -36,7 +41,8 @@ TabOrganizationButton::TabOrganizationButton(
base::Unretained(this)),
l10n_util::GetStringUTF16(IDS_TAB_ORGANIZE),
flat_edge),
pressed_callback_(std::move(pressed_callback)) {
pressed_callback_(std::move(pressed_callback)),
browser_(tab_strip_controller->GetBrowser()) {
auto* const layout_manager =
SetLayoutManager(std::make_unique<views::BoxLayout>());
layout_manager->set_main_axis_alignment(
Expand Down Expand Up @@ -88,6 +94,18 @@ void TabOrganizationButton::ButtonPressed(const ui::Event& event) {
if (session_->request()->state() ==
TabOrganizationRequest::State::NOT_STARTED) {
session_->StartRequest();

// TODO(emshack/dpenning): Move the rest of the body of this if into the
// method that executes when StartRequest() is called on a session, once
// such a method exists.
if (browser_) {
browser_->profile()->GetPrefs()->SetInteger(
tab_search_prefs::kTabSearchTabIndex, 1);
BrowserView* const browser_view =
BrowserView::GetBrowserViewForBrowser(browser_);
auto* tab_search_host = browser_view->GetTabSearchBubbleHost();
tab_search_host->ShowTabSearchBubble(false);
}
}
pressed_callback_.Run(event);
}
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/tabs/tab_organization_button.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "chrome/browser/ui/views/tabs/tab_strip_control_button.h"
#include "ui/base/metadata/metadata_header_macros.h"

class Browser;
class TabOrganizationSession;
class TabStripController;

Expand Down Expand Up @@ -46,6 +47,7 @@ class TabOrganizationButton : public TabStripControlButton {
raw_ptr<TabOrganizationSession, DanglingUntriaged> session_ = nullptr;
PressedCallback pressed_callback_;
raw_ptr<views::LabelButton> close_button_;
raw_ptr<const Browser> browser_;
};

#endif // CHROME_BROWSER_UI_VIEWS_TABS_TAB_ORGANIZATION_BUTTON_H_
4 changes: 4 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ interface PageHandler {
// list section.
SaveRecentlyClosedExpandedPref(bool expanded);

// Set the user preference for which tab the tab search UI should display
// when next shown.
SetTabIndex(int32 index);

// Notify the backend that the UI is ready to be shown.
ShowUI();
};
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search_page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,11 @@ void TabSearchPageHandler::SaveRecentlyClosedExpandedPref(bool expanded) {
: TabSearchRecentlyClosedToggleAction::kCollapse);
}

void TabSearchPageHandler::SetTabIndex(int32_t index) {
Profile::FromWebUI(web_ui_)->GetPrefs()->SetInteger(
tab_search_prefs::kTabSearchTabIndex, index);
}

void TabSearchPageHandler::ShowUI() {
auto embedder = webui_controller_->embedder();
if (embedder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
void OpenRecentlyClosedEntry(int32_t session_id) override;
void RequestTabOrganization() override;
void SaveRecentlyClosedExpandedPref(bool expanded) override;
void SetTabIndex(int32_t index) override;
void ShowUI() override;

// TabStripModelObserver:
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,13 @@ namespace tab_search_prefs {
const char kTabSearchRecentlyClosedSectionExpanded[] =
"tab_search.recently_closed_expanded";

// Integer pref indicating which tab the Tab Search bubble should open to
// when shown.
const char kTabSearchTabIndex[] = "tab_search.tab_index";

void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterBooleanPref(kTabSearchRecentlyClosedSectionExpanded, true);
registry->RegisterIntegerPref(kTabSearchTabIndex, 0);
}

} // namespace tab_search_prefs
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search_prefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ namespace tab_search_prefs {

extern const char kTabSearchRecentlyClosedSectionExpanded[];

extern const char kTabSearchTabIndex[];

void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);

} // namespace tab_search_prefs
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/browser/ui/webui/favicon_source.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_prefs.h"
#include "chrome/browser/ui/webui/webui_util.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/grit/tab_search_resources.h"
#include "chrome/grit/tab_search_resources_map.h"
#include "components/favicon_base/favicon_url_parser.h"
#include "components/prefs/pref_service.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
Expand Down Expand Up @@ -112,6 +114,7 @@ TabSearchUI::TabSearchUI(content::WebUI* web_ui)
features::kTabSearchRecentlyClosedDefaultItemDisplayCount.Get());

source->AddBoolean("tabOrganizationEnabled", features::IsTabOrganization());
source->AddInteger("tabIndex", TabIndex());

ui::Accelerator accelerator(ui::VKEY_A,
ui::EF_SHIFT_DOWN | ui::EF_PLATFORM_ACCELERATOR);
Expand Down Expand Up @@ -174,3 +177,8 @@ void TabSearchUI::CreatePageHandler(
page_handler_ = std::make_unique<TabSearchPageHandler>(
std::move(receiver), std::move(page), web_ui(), this, &metrics_reporter_);
}

int TabSearchUI::TabIndex() {
PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
return prefs->GetInteger(tab_search_prefs::kTabSearchTabIndex);
}
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class TabSearchUI : public ui::MojoBubbleWebUIController,
mojo::PendingRemote<tab_search::mojom::Page> page,
mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver) override;

int TabIndex();

std::unique_ptr<ui::ColorChangeHandler> color_provider_handler_;
std::unique_ptr<TabSearchPageHandler> page_handler_;
MetricsReporter metrics_reporter_;
Expand Down
1 change: 1 addition & 0 deletions chrome/test/data/webui/tab_search/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ build_webui_tests("build") {
"fuzzy_search_test.ts",
"infinite_list_test.ts",
"tab_organization_page_test.ts",
"tab_search_app_test.ts",
"tab_search_item_test.ts",
"tab_search_media_tabs_test.ts",
"tab_search_page_focus_test.ts",
Expand Down
48 changes: 48 additions & 0 deletions chrome/test/data/webui/tab_search/tab_search_app_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import {loadTimeData} from 'chrome://resources/js/load_time_data.js';
import {TabSearchApiProxyImpl, TabSearchAppElement} from 'chrome://tab-search.top-chrome/tab_search.js';
import {assertEquals, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';

import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js';

suite('TabOrganizationPageTest', () => {
let tabSearchApp: TabSearchAppElement;
let testProxy: TestTabSearchApiProxy;

setup(async () => {
testProxy = new TestTabSearchApiProxy();
TabSearchApiProxyImpl.setInstance(testProxy);

loadTimeData.overrideValues({
tabOrganizationEnabled: true,
});

tabSearchApp = document.createElement('tab-search-app');

document.body.innerHTML = window.trustedTypes!.emptyHTML;
document.body.appendChild(tabSearchApp);
await flushTasks();
});

test('Switching tabs calls setTabIndex', async () => {
assertEquals(0, testProxy.getCallCount('setTabIndex'));

const crTabs = tabSearchApp.shadowRoot!.querySelector('cr-tabs');
assertTrue(!!crTabs);
assertEquals(0, crTabs.selected);

const allTabs = crTabs.shadowRoot!.querySelectorAll<HTMLElement>('.tab');
assertEquals(2, allTabs.length);
const newTabIndex = 1;
const unselectedTab = allTabs[newTabIndex]!;
unselectedTab.click();

const [tabIndex] = await testProxy.whenCalled('setTabIndex');
assertEquals(newTabIndex, tabIndex);
assertEquals(newTabIndex, crTabs.selected);
});
});
4 changes: 4 additions & 0 deletions chrome/test/data/webui/tab_search/tab_search_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ class TabSearchTest : public WebUIMochaBrowserTest {
TabSearchTest() { set_test_loader_host(chrome::kChromeUITabSearchHost); }
};

IN_PROC_BROWSER_TEST_F(TabSearchTest, App) {
RunTest("tab_search/tab_search_app_test.js", "mocha.run()");
}

IN_PROC_BROWSER_TEST_F(TabSearchTest, Page) {
RunTest("tab_search/tab_search_page_test.js", "mocha.run()");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export class TestTabSearchApiProxy extends TestBrowserProxy implements
'requestTabOrganization',
'switchToTab',
'saveRecentlyClosedExpandedPref',
'setTabIndex',
'showUi',
]);

Expand Down Expand Up @@ -75,6 +76,10 @@ export class TestTabSearchApiProxy extends TestBrowserProxy implements
this.methodCalled('saveRecentlyClosedExpandedPref', [expanded]);
}

setTabIndex(index: number) {
this.methodCalled('setTabIndex', [index]);
}

showUi() {
this.methodCalled('showUi');
}
Expand Down

0 comments on commit 2b3f388

Please sign in to comment.