Commit ccd95c55 authored by James Cook's avatar James Cook Committed by Commit Bot

UX updates to OS settings search/assistant section

Latest revision to PRD specifies:
* Rename section to "Assistant"
* Remove the search engine link to browser settings
* Hide the whole section when assistant isn't available

See bug for screenshots.

This CL plumbs "showAssistant" from settings UI to main to page
similarly to how showApps is handled.

TODO: Once we're sure there won't be other search settings here,
rename the files/section to "os_assistant_page".

Bug: 960937
Test: manual on device (with assistant) and on linux-chromeos (without)
Change-Id: I97e4409f6751123bad2ce364230adb17e7d7e9ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1650525Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667371}
parent f35d04e5
...@@ -20,9 +20,9 @@ ...@@ -20,9 +20,9 @@
Open the wallpaper app Open the wallpaper app
</message> </message>
<!-- Search and Assistant section. --> <!-- Assistant section. -->
<message name="IDS_OS_SETTINGS_SEARCH_ENGINE_LABEL" desc="Label in OS settings describing search engine behavior."> <message name="IDS_OS_SETTINGS_ASSISTANT" desc="Name of the settings section for the Google Assistant.">
Searches from the app launcher use your browser <ph name="BEGIN_LINK">&lt;a target="_blank" href="$1"&gt;</ph>search engine setting<ph name="END_LINK">&lt;/a&gt;</ph>. Assistant
</message> </message>
</if> </if>
</grit-part> </grit-part>
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
<link rel="import" href="chrome://resources/html/i18n_behavior.html"> <link rel="import" href="chrome://resources/html/i18n_behavior.html">
<link rel="import" href="../../google_assistant_page/google_assistant_page.html"> <link rel="import" href="../../google_assistant_page/google_assistant_page.html">
<link rel="import" href="../../google_assistant_page/google_assistant_browser_proxy.html"> <link rel="import" href="../../google_assistant_page/google_assistant_browser_proxy.html">
<link rel="import" href="../../icons.html">
<link rel="import" href="../../i18n_setup.html"> <link rel="import" href="../../i18n_setup.html">
<link rel="import" href="../../route.html"> <link rel="import" href="../../route.html">
<link rel="import" href="../../settings_page/settings_animated_pages.html"> <link rel="import" href="../../settings_page/settings_animated_pages.html">
...@@ -14,19 +13,16 @@ ...@@ -14,19 +13,16 @@
<link rel="import" href="../../settings_shared_css.html"> <link rel="import" href="../../settings_shared_css.html">
<link rel="import" href="../../settings_vars_css.html"> <link rel="import" href="../../settings_vars_css.html">
<!-- TODO(jamescook): Rename to "os-assistant-page". -->
<dom-module id="os-settings-search-page"> <dom-module id="os-settings-search-page">
<template> <template>
<style include="settings-shared md-select"> <style include="settings-shared">
iron-icon {
padding-inline-end: 16px;
}
</style> </style>
<!-- TODO(jamescook): Rename section to "assistant". -->
<settings-animated-pages id="pages" section="search" <settings-animated-pages id="pages" section="search"
focus-config="[[focusConfig_]]"> focus-config="[[focusConfig_]]">
<div route-path="default"> <div route-path="default">
<!-- Google Assistant --> <!-- Google Assistant -->
<template is="dom-if" if="[[isAssistantAllowed_]]">
<div id="assistantSubpageTrigger" class="settings-box first two-line" <div id="assistantSubpageTrigger" class="settings-box first two-line"
on-click="onGoogleAssistantTap_" actionable> on-click="onGoogleAssistantTap_" actionable>
<div class="start"> <div class="start">
...@@ -39,19 +35,8 @@ ...@@ -39,19 +35,8 @@
<cr-icon-button class="subpage-arrow" <cr-icon-button class="subpage-arrow"
aria-label="$i18n{searchGoogleAssistant}"></cr-icon-button> aria-label="$i18n{searchGoogleAssistant}"></cr-icon-button>
</div> </div>
</template>
<!-- Label explaining that the app launcher search box uses the
browser's search engine. -->
<div class="settings-box">
<div class="start">
<iron-icon icon="cr:info-outline"></iron-icon>
$i18nRaw{osSearchEngineLabel}
</div>
</div>
</div> </div>
<template is="dom-if" if="[[isAssistantAllowed_]]">
<template is="dom-if" route-path="/googleAssistant"> <template is="dom-if" route-path="/googleAssistant">
<settings-subpage <settings-subpage
associated-control="[[$$('#assistantSubpageTrigger')]]" associated-control="[[$$('#assistantSubpageTrigger')]]"
...@@ -60,7 +45,6 @@ ...@@ -60,7 +45,6 @@
</settings-google-assistant-page> </settings-google-assistant-page>
</settings-subpage> </settings-subpage>
</template> </template>
</template>
</settings-animated-pages> </settings-animated-pages>
</template> </template>
<script src="os_search_page.js"></script> <script src="os_search_page.js"></script>
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
/** /**
* @fileoverview * @fileoverview
* 'os-settings-search-page' contains search and assistant settings. * 'os-settings-search-page' contains assistant settings.
*/ */
Polymer({ Polymer({
is: 'os-settings-search-page', is: 'os-settings-search-page',
...@@ -16,18 +16,11 @@ Polymer({ ...@@ -16,18 +16,11 @@ Polymer({
/** @type {?Map<string, string>} */ /** @type {?Map<string, string>} */
focusConfig_: Object, focusConfig_: Object,
/** @private Can be disallowed due to flag, policy, locale, etc. */
isAssistantAllowed_: {
type: Boolean,
value: function() {
return loadTimeData.getBoolean('isAssistantAllowed');
},
},
}, },
/** @override */ /** @override */
ready: function() { ready: function() {
assert(loadTimeData.getBoolean('isAssistantAllowed'));
this.focusConfig_ = new Map(); this.focusConfig_ = new Map();
if (settings.routes.GOOGLE_ASSISTANT) { if (settings.routes.GOOGLE_ASSISTANT) {
this.focusConfig_.set( this.focusConfig_.set(
...@@ -38,7 +31,6 @@ Polymer({ ...@@ -38,7 +31,6 @@ Polymer({
/** @private */ /** @private */
onGoogleAssistantTap_: function() { onGoogleAssistantTap_: function() {
assert(this.isAssistantAllowed_);
settings.navigateTo(settings.routes.GOOGLE_ASSISTANT); settings.navigateTo(settings.routes.GOOGLE_ASSISTANT);
}, },
......
...@@ -64,6 +64,7 @@ ...@@ -64,6 +64,7 @@
page-visibility="[[pageVisibility]]" page-visibility="[[pageVisibility]]"
show-apps="[[showApps]]" show-apps="[[showApps]]"
show-android-apps="[[showAndroidApps]]" show-android-apps="[[showAndroidApps]]"
show-assistant="[[showAssistant]]"
show-kiosk-next-shell="[[showKioskNextShell]]" show-kiosk-next-shell="[[showKioskNextShell]]"
show-crostini="[[showCrostini]]" show-crostini="[[showCrostini]]"
show-plugin-vm="[[showPluginVm]]" show-plugin-vm="[[showPluginVm]]"
......
...@@ -83,6 +83,8 @@ Polymer({ ...@@ -83,6 +83,8 @@ Polymer({
showAndroidApps: Boolean, showAndroidApps: Boolean,
showAssistant: Boolean,
havePlayStoreApp: Boolean, havePlayStoreApp: Boolean,
}, },
......
...@@ -128,9 +128,10 @@ ...@@ -128,9 +128,10 @@
<iron-icon icon="settings:laptop-chromebook"></iron-icon> <iron-icon icon="settings:laptop-chromebook"></iron-icon>
$i18n{devicePageTitle} $i18n{devicePageTitle}
</a> </a>
<a href="/search"> <!-- TODO(jamescook): Rename path to "/assistant". -->
<a href="/search" hidden="[[!showAssistant]]">
<iron-icon icon="cr:search"></iron-icon> <iron-icon icon="cr:search"></iron-icon>
$i18n{osSearchPageTitle} $i18n{osAssistantPageTitle}
</a> </a>
<a href="/apps" hidden="[[!showApps]]"> <a href="/apps" hidden="[[!showApps]]">
<iron-icon icon="settings:apps"></iron-icon> <iron-icon icon="settings:apps"></iron-icon>
......
...@@ -161,8 +161,8 @@ ...@@ -161,8 +161,8 @@
</settings-device-page> </settings-device-page>
</settings-section> </settings-section>
</template> </template>
<template is="dom-if" if="[[showPage_(pageVisibility.search)]]" restamp> <template is="dom-if" if="[[showAssistant]]" restamp>
<settings-section page-title="$i18n{osSearchPageTitle}" <settings-section page-title="$i18n{osAssistantPageTitle}"
section="search"> section="search">
<os-settings-search-page prefs="{{prefs}}"> <os-settings-search-page prefs="{{prefs}}">
</os-settings-search-page> </os-settings-search-page>
......
...@@ -26,6 +26,8 @@ Polymer({ ...@@ -26,6 +26,8 @@ Polymer({
showAndroidApps: Boolean, showAndroidApps: Boolean,
showAssistant: Boolean,
showCrostini: Boolean, showCrostini: Boolean,
allowCrostini_: Boolean, allowCrostini_: Boolean,
......
...@@ -122,6 +122,7 @@ ...@@ -122,6 +122,7 @@
<os-settings-menu page-visibility="[[pageVisibility_]]" <os-settings-menu page-visibility="[[pageVisibility_]]"
show-apps="[[showApps_]]" show-apps="[[showApps_]]"
show-android-apps="[[showAndroidApps_]]" show-android-apps="[[showAndroidApps_]]"
show-assistant="[[showAssistant_]]"
show-crostini="[[showCrostini_]]" show-crostini="[[showCrostini_]]"
show-parental-controls="[[showParentalControls_]]" show-parental-controls="[[showParentalControls_]]"
show-plugin-vm="[[showPluginVm_]]" show-plugin-vm="[[showPluginVm_]]"
...@@ -138,6 +139,7 @@ ...@@ -138,6 +139,7 @@
<os-settings-menu page-visibility="[[pageVisibility_]]" <os-settings-menu page-visibility="[[pageVisibility_]]"
show-apps="[[showApps_]]" show-apps="[[showApps_]]"
show-android-apps="[[showAndroidApps_]]" show-android-apps="[[showAndroidApps_]]"
show-assistant="[[showAssistant_]]"
show-crostini="[[showCrostini_]]" show-crostini="[[showCrostini_]]"
show-plugin-vm="[[showPluginVm_]]" show-plugin-vm="[[showPluginVm_]]"
show-multidevice="[[showMultidevice_]]" show-multidevice="[[showMultidevice_]]"
...@@ -151,6 +153,7 @@ ...@@ -151,6 +153,7 @@
page-visibility="[[pageVisibility_]]" page-visibility="[[pageVisibility_]]"
show-apps="[[showApps_]]" show-apps="[[showApps_]]"
show-android-apps="[[showAndroidApps_]]" show-android-apps="[[showAndroidApps_]]"
show-assistant="[[showAssistant_]]"
show-kiosk-next-shell="[[showKioskNextShell_]]" show-kiosk-next-shell="[[showKioskNextShell_]]"
show-crostini="[[showCrostini_]]" show-crostini="[[showCrostini_]]"
show-parental-controls="[[showParentalControls_]]" show-parental-controls="[[showParentalControls_]]"
......
...@@ -74,6 +74,9 @@ Polymer({ ...@@ -74,6 +74,9 @@ Polymer({
/** @private */ /** @private */
showAndroidApps_: Boolean, showAndroidApps_: Boolean,
/** @private */
showAssistant_: Boolean,
/** @private */ /** @private */
showKioskNextShell_: Boolean, showKioskNextShell_: Boolean,
...@@ -167,6 +170,8 @@ Polymer({ ...@@ -167,6 +170,8 @@ Polymer({
this.showApps_ = loadTimeData.getBoolean('showApps'); this.showApps_ = loadTimeData.getBoolean('showApps');
this.showAndroidApps_ = loadTimeData.getBoolean('androidAppsVisible'); this.showAndroidApps_ = loadTimeData.getBoolean('androidAppsVisible');
// Assistant can be disallowed due to flag, policy, locale, etc.
this.showAssistant_ = loadTimeData.getBoolean('isAssistantAllowed');
this.showKioskNextShell_ = loadTimeData.valueExists('showKioskNextShell') && this.showKioskNextShell_ = loadTimeData.valueExists('showKioskNextShell') &&
loadTimeData.getBoolean('showKioskNextShell'); loadTimeData.getBoolean('showKioskNextShell');
this.showParentalControls_ = this.showParentalControls_ =
......
...@@ -63,7 +63,6 @@ ...@@ -63,7 +63,6 @@
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
#include "ash/public/cpp/ash_switches.h" #include "ash/public/cpp/ash_switches.h"
#include "ash/public/interfaces/voice_interaction_controller.mojom.h" #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
#include "base/strings/strcat.h"
#include "base/system/sys_info.h" #include "base/system/sys_info.h"
#include "chrome/browser/chromeos/account_manager/account_manager_util.h" #include "chrome/browser/chromeos/account_manager/account_manager_util.h"
#include "chrome/browser/chromeos/arc/arc_util.h" #include "chrome/browser/chromeos/arc/arc_util.h"
...@@ -2309,9 +2308,7 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) { ...@@ -2309,9 +2308,7 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) {
#endif #endif
{"searchEnginesManage", IDS_SETTINGS_SEARCH_MANAGE_SEARCH_ENGINES}, {"searchEnginesManage", IDS_SETTINGS_SEARCH_MANAGE_SEARCH_ENGINES},
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
{"osSearchPageTitle", is_assistant_allowed {"osAssistantPageTitle", IDS_OS_SETTINGS_ASSISTANT},
? IDS_SETTINGS_SEARCH_AND_ASSISTANT
: IDS_SETTINGS_SEARCH},
{"searchGoogleAssistant", IDS_SETTINGS_SEARCH_GOOGLE_ASSISTANT}, {"searchGoogleAssistant", IDS_SETTINGS_SEARCH_GOOGLE_ASSISTANT},
{"searchGoogleAssistantEnabled", {"searchGoogleAssistantEnabled",
IDS_SETTINGS_SEARCH_GOOGLE_ASSISTANT_ENABLED}, IDS_SETTINGS_SEARCH_GOOGLE_ASSISTANT_ENABLED},
...@@ -2326,12 +2323,6 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) { ...@@ -2326,12 +2323,6 @@ void AddSearchStrings(content::WebUIDataSource* html_source, Profile* profile) {
base::ASCIIToUTF16(chrome::kOmniboxLearnMoreURL)); base::ASCIIToUTF16(chrome::kOmniboxLearnMoreURL));
html_source->AddString("searchExplanation", search_explanation_text); html_source->AddString("searchExplanation", search_explanation_text);
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
std::string settings_search_url =
base::StrCat({chrome::kChromeUISettingsURL, chrome::kSearchSubPage});
html_source->AddString(
"osSearchEngineLabel",
l10n_util::GetStringFUTF16(IDS_OS_SETTINGS_SEARCH_ENGINE_LABEL,
base::ASCIIToUTF16(settings_search_url)));
html_source->AddBoolean("isAssistantAllowed", is_assistant_allowed); html_source->AddBoolean("isAssistantAllowed", is_assistant_allowed);
#endif #endif
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment