Skip to content

Commit

Permalink
Tab Organization: Add failure state FRE
Browse files Browse the repository at this point in the history
Also remove GetBrowserForWebUI from tab_search_page_handler.cc as it
proved unreliable. Switching to chrome::FindLastActive for now, as is
used by the rest of tab_search_page_handler.cc.

Bug: 1469126
Change-Id: I402cf043138e43a469f0375f1e7b9be412794e20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974126
Commit-Queue: Emily Shack <emshack@chromium.org>
Reviewed-by: Charles Meng <charlesmeng@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1215803}
  • Loading branch information
Emily Shack authored and Chromium LUCI CQ committed Oct 26, 2023
1 parent ce11f52 commit 4cb76ae
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 30 deletions.
9 changes: 9 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -10465,6 +10465,15 @@ Check your passwords anytime in <ph name="GOOGLE_PASSWORD_MANAGER">$1<ex>Google
<message name="IDS_TAB_ORGANIZATION_FAILURE_BODY_GROUPING" desc="The body text for the grouping failure state in the tab organization UI" translateable="false">
You'll get more opportunities for tab organization after you open new related tabs
</message>
<message name="IDS_TAB_ORGANIZATION_TIP_TITLE" desc="The bolded text at the start of the tip in the tab organization UI" translateable="false">
Tip:
</message>
<message name="IDS_TAB_ORGANIZATION_TIP_BODY" desc="The main text of the tip in the tab organization UI" translateable="false">
You can organize your own tabs anytime.
</message>
<message name="IDS_TAB_ORGANIZATION_TIP_ACTION" desc="The actionable text of the tip in the tab organization UI" translateable="false">
Show me how
</message>

<!-- Strings for Window Titles in Menus -->
<if expr="not use_titlecase">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

<div class="tab-organization-container">
<div class="tab-organization-text-container">
<div class="tab-organization-header">[[getTitle_(showFRE_)]]</div>
<div class="tab-organization-body">[[getBody_(showFRE_)]]</div>
<div class="tab-organization-header">[[getTitle_(showFre)]]</div>
<div class="tab-organization-body">[[getBody_(showFre)]]</div>
</div>
<cr-button class="action-button" on-click="onOrganizeTabsClick_">
$i18n{notStartedButton}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,26 @@ export class TabOrganizationNotStartedElement extends PolymerElement {

static get properties() {
return {
showFRE_: {
type: Boolean,
value: loadTimeData.getBoolean('showTabOrganizationFRE'),
},
showFre: Boolean,
};
}

private showFRE_: boolean;
showFre: boolean;

static get template() {
return getTemplate();
}

private getTitle_(): string {
if (this.showFRE_) {
if (this.showFre) {
return loadTimeData.getString('notStartedTitleFRE');
} else {
return loadTimeData.getString('notStartedTitle');
}
}

private getBody_(): string {
if (this.showFRE_) {
if (this.showFre) {
return loadTimeData.getString('notStartedBodyFRE');
} else {
return loadTimeData.getString('notStartedBody');
Expand Down
22 changes: 20 additions & 2 deletions chrome/browser/resources/tab_search/tab_organization_page.html
Original file line number Diff line number Diff line change
@@ -1,13 +1,25 @@
<style>
<style include="tab-organization-shared-style">
.body {
margin: 16px var(--mwb-list-item-horizontal-margin);
}

.footer {
background-color: var(--color-bubble-footer-background);
padding: 16px var(--mwb-list-item-horizontal-margin);
}

.link {
color: var(--color-link-foreground-on-bubble-footer);
cursor: pointer;
text-decoration: underline;
}
</style>

<div class="body">
<tab-organization-not-started
hidden="[[!isState_(tabOrganizationStateEnum_.kNotStarted, state_)]]"
on-organize-tabs-click="onOrganizeTabsClick_">
on-organize-tabs-click="onOrganizeTabsClick_"
show-fre="[[showFRE_]]">
</tab-organization-not-started>
<tab-organization-in-progress
hidden="[[!isState_(tabOrganizationStateEnum_.kInProgress, state_)]]">
Expand All @@ -23,3 +35,9 @@
error="[[error_]]">
</tab-organization-failure>
</div>
<div class="footer" hidden="[[!showFooter_(state_, showFRE_)]]">
<div class="tab-organization-body"><b>$i18n{tipTitle}</b> $i18n{tipBody}</div>
<div class="tab-organization-body link" on-click="onTipClick_">
$i18n{tipAction}
</div>
</div>
17 changes: 17 additions & 0 deletions chrome/browser/resources/tab_search/tab_organization_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ import 'chrome://resources/cr_elements/cr_button/cr_button.js';
import 'chrome://resources/cr_elements/cr_icon_button/cr_icon_button.js';
import 'chrome://resources/cr_elements/cr_icons.css.js';
import 'chrome://resources/cr_elements/mwb_shared_style.css.js';
import './strings.m.js';
import './tab_organization_failure.js';
import './tab_organization_in_progress.js';
import './tab_organization_not_started.js';
import './tab_organization_results.js';
import './tab_organization_shared_style.css.js';

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 './tab_organization_page.html.js';
Expand All @@ -33,6 +36,11 @@ export class TabOrganizationPageElement extends PolymerElement {
type: Object,
value: TabOrganizationState,
},

showFRE_: {
type: Boolean,
value: loadTimeData.getBoolean('showTabOrganizationFRE'),
},
};
}

Expand All @@ -44,6 +52,7 @@ export class TabOrganizationPageElement extends PolymerElement {
private error_: TabOrganizationError = TabOrganizationError.kNone;
private sessionId_: number = -1;
private organizationId_: number = -1;
private showFRE_: boolean;

static get template() {
return getTemplate();
Expand Down Expand Up @@ -88,6 +97,10 @@ export class TabOrganizationPageElement extends PolymerElement {
return this.state_ === state;
}

private showFooter_(): boolean {
return this.state_ === TabOrganizationState.kFailure && this.showFRE_;
}

private onOrganizeTabsClick_() {
this.apiProxy_.requestTabOrganization();
}
Expand All @@ -99,6 +112,10 @@ export class TabOrganizationPageElement extends PolymerElement {
this.apiProxy_.acceptTabOrganization(
this.sessionId_, this.organizationId_, this.name_, this.tabs_);
}

private onTipClick_() {
this.apiProxy_.startTabGroupTutorial();
}
}

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 @@ -39,6 +39,8 @@ export interface TabSearchApiProxy {

setTabIndex(index: number): void;

startTabGroupTutorial(): void;

showUi(): void;
}

Expand Down Expand Up @@ -110,6 +112,10 @@ export class TabSearchApiProxyImpl implements TabSearchApiProxy {
this.handler.setTabIndex(index);
}

startTabGroupTutorial() {
this.handler.startTabGroupTutorial();
}

showUi() {
this.handler.showUI();
}
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ui/webui/tab_search/tab_search.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ interface PageHandler {
// when next shown.
SetTabIndex(int32 index);

// Force trigger the tab group tutorial.
StartTabGroupTutorial();

// Notify the backend that the UI is ready to be shown.
ShowUI();
};
Expand Down
46 changes: 27 additions & 19 deletions chrome/browser/ui/webui/tab_search/tab_search_page_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@
#include "chrome/browser/ui/webui/metrics_reporter/metrics_reporter.h"
#include "chrome/browser/ui/webui/tab_search/tab_search_prefs.h"
#include "chrome/browser/ui/webui/util/image_util.h"
#include "chrome/browser/user_education/user_education_service.h"
#include "chrome/browser/user_education/user_education_service_factory.h"
#include "chrome/common/webui_url_constants.h"
#include "components/user_education/common/tutorial_identifier.h"
#include "components/user_education/common/tutorial_service.h"
#include "ui/base/l10n/time_format.h"
#include "ui/color/color_provider.h"

Expand Down Expand Up @@ -107,24 +111,6 @@ gfx::ImageSkia ThemeFavicon(const gfx::ImageSkia& source,
provider.GetColor(kColorTabSearchBackground));
}

Browser* GetBrowserForWebUI(content::WebUI* web_ui) {
// get the browser from the web_contents
BrowserWindow* browser_window =
BrowserWindow::FindBrowserWindowWithWebContents(web_ui->GetWebContents());
if (!browser_window) {
return nullptr;
}
Browser* browser = nullptr;
for (auto* browser_in_list : *BrowserList::GetInstance()) {
if (browser_in_list->window() == browser_window) {
browser = browser_in_list;
break;
}
}

return browser;
}

} // namespace

TabSearchPageHandler::TabSearchPageHandler(
Expand Down Expand Up @@ -181,7 +167,7 @@ void TabSearchPageHandler::AcceptTabOrganization(
const std::string& name,
std::vector<tab_search::mojom::TabPtr> tabs) {
// TODO(dpenning): Implement this
Browser* browser = GetBrowserForWebUI(web_ui_);
Browser* browser = chrome::FindLastActive();
browser->profile()->GetPrefs()->SetBoolean(
tab_search_prefs::kTabOrganizationShowFRE, false);
}
Expand Down Expand Up @@ -302,6 +288,28 @@ void TabSearchPageHandler::SetTabIndex(int32_t index) {
tab_search_prefs::kTabSearchTabIndex, index);
}

void TabSearchPageHandler::StartTabGroupTutorial() {
// Close the tab search bubble if showing.
auto embedder = webui_controller_->embedder();
if (embedder) {
embedder->CloseUI();
}

const Browser* const browser = chrome::FindLastActive();
auto* const user_education_service =
UserEducationServiceFactory::GetForBrowserContext(browser->profile());
user_education::TutorialService* const tutorial_service =
user_education_service ? &user_education_service->tutorial_service()
: nullptr;
CHECK(tutorial_service);

const ui::ElementContext context = browser->window()->GetElementContext();
CHECK(context);

user_education::TutorialIdentifier tutorial_id = kTabGroupTutorialId;
tutorial_service->StartTutorial(tutorial_id, context);
}

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

// TabStripModelObserver:
Expand Down
3 changes: 3 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 @@ -81,6 +81,9 @@ TabSearchUI::TabSearchUI(content::WebUI* web_ui)
{"notStartedTitleFRE", IDS_TAB_ORGANIZATION_NOT_STARTED_TITLE_FRE},
{"successTitle", IDS_TAB_ORGANIZATION_SUCCESS_TITLE},
{"tabOrganizationTabName", IDS_TAB_ORGANIZATION_TAB_NAME},
{"tipTitle", IDS_TAB_ORGANIZATION_TIP_TITLE},
{"tipBody", IDS_TAB_ORGANIZATION_TIP_BODY},
{"tipAction", IDS_TAB_ORGANIZATION_TIP_ACTION},
};
webui::SetupChromeRefresh2023(source);
source->AddLocalizedStrings(kStrings);
Expand Down
25 changes: 25 additions & 0 deletions chrome/test/data/webui/tab_search/tab_organization_page_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// 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 {Tab, TabOrganizationError, TabOrganizationPageElement, TabOrganizationResultsElement, TabOrganizationSession, TabOrganizationState, TabSearchApiProxyImpl} from 'chrome://tab-search.top-chrome/tab_search.js';
import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {flushTasks} from 'chrome://webui-test/polymer_test_util.js';
Expand Down Expand Up @@ -146,4 +147,28 @@ suite('TabOrganizationPageTest', () => {

assertEquals(1, testProxy.getCallCount('acceptTabOrganization'));
});

test('Tip action starts tutorial', async () => {
loadTimeData.overrideValues({
showTabOrganizationFRE: true,
});

await tabOrganizationPageSetup();

testProxy.getCallbackRouterRemote().tabOrganizationSessionUpdated(
createSession({
state: TabOrganizationState.kFailure,
error: TabOrganizationError.kGeneric,
}));

assertEquals(0, testProxy.getCallCount('startTabGroupTutorial'));

const tipAction =
tabOrganizationPage.shadowRoot!.querySelector<HTMLElement>('.link');
assertTrue(!!tipAction);
tipAction.click();
await flushTasks();

assertEquals(1, testProxy.getCallCount('startTabGroupTutorial'));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export class TestTabSearchApiProxy extends TestBrowserProxy implements
'switchToTab',
'saveRecentlyClosedExpandedPref',
'setTabIndex',
'startTabGroupTutorial',
'showUi',
]);

Expand Down Expand Up @@ -80,6 +81,10 @@ export class TestTabSearchApiProxy extends TestBrowserProxy implements
this.methodCalled('setTabIndex', [index]);
}

startTabGroupTutorial() {
this.methodCalled('startTabGroupTutorial');
}

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

0 comments on commit 4cb76ae

Please sign in to comment.