Commit e4736989 authored by Theo Johnson-kanu's avatar Theo Johnson-kanu Committed by Chromium LUCI CQ

[Cros Cellular] Add custom sublabel to quick settings cellular networks

- This CL adds a sub label to cellular networks which have not yet been
  activated to indicate the user can click and activate that network.
- Opens new cellular setup dialog when "click to activate" is clicked

Screenshot: https://screenshot.googleplex.com/3hmXeFo7gPQhcBZ.png

Bug: 1093185
Change-Id: I5f7df05123be56e97c74125313263ff50d0a0124
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629989
Commit-Queue: Nnamdi Theodore Johnson-kanu <tjohnsonkanu@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarAzeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845388}
parent d08c3302
...@@ -1526,6 +1526,9 @@ This file contains the strings for ash. ...@@ -1526,6 +1526,9 @@ This file contains the strings for ash.
<message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING" desc="The label used when a network connection is connecting. [CHAR_LIMIT=14]"> <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING" desc="The label used when a network connection is connecting. [CHAR_LIMIT=14]">
Connecting... Connecting...
</message> </message>
<message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CLICK_TO_ACTIVATE" desc="The label used in quick settings to indicate an unactivated cellular network that can be clicked to launch activation through setup flow. [CHAR_LIMIT=14]">
Click to activate
</message>
<message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SECURED" desc="a11y label used when a Wi-Fi network is secured. [CHAR_LIMIT=14]"> <message name="IDS_ASH_STATUS_TRAY_NETWORK_STATUS_SECURED" desc="a11y label used when a Wi-Fi network is secured. [CHAR_LIMIT=14]">
Secured Secured
</message> </message>
......
7094d028d02bcb7f8b1827db6ebb162cb030f5dd
\ No newline at end of file
...@@ -93,6 +93,9 @@ class ASH_PUBLIC_EXPORT SystemTrayClient { ...@@ -93,6 +93,9 @@ class ASH_PUBLIC_EXPORT SystemTrayClient {
// (see onc_spec.md). TODO(stevenjb): Use NetworkType from onc.mojo (TBD). // (see onc_spec.md). TODO(stevenjb): Use NetworkType from onc.mojo (TBD).
virtual void ShowNetworkCreate(const std::string& type) = 0; virtual void ShowNetworkCreate(const std::string& type) = 0;
// Opens the physical SIM cellular setup flow in OS Settings.
virtual void ShowSettingsCellularSetupPsimFlow() = 0;
// Shows the "add network" UI to create a third-party extension-backed VPN // Shows the "add network" UI to create a third-party extension-backed VPN
// connection (e.g. Cisco AnyConnect). // connection (e.g. Cisco AnyConnect).
virtual void ShowThirdPartyVpnCreate(const std::string& extension_id) = 0; virtual void ShowThirdPartyVpnCreate(const std::string& extension_id) = 0;
......
...@@ -63,6 +63,8 @@ void TestSystemTrayClient::ShowNetworkConfigure(const std::string& network_id) { ...@@ -63,6 +63,8 @@ void TestSystemTrayClient::ShowNetworkConfigure(const std::string& network_id) {
void TestSystemTrayClient::ShowNetworkCreate(const std::string& type) {} void TestSystemTrayClient::ShowNetworkCreate(const std::string& type) {}
void TestSystemTrayClient::ShowSettingsCellularSetupPsimFlow() {}
void TestSystemTrayClient::ShowThirdPartyVpnCreate( void TestSystemTrayClient::ShowThirdPartyVpnCreate(
const std::string& extension_id) {} const std::string& extension_id) {}
......
...@@ -43,6 +43,7 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient { ...@@ -43,6 +43,7 @@ class ASH_PUBLIC_EXPORT TestSystemTrayClient : public SystemTrayClient {
void ShowEnterpriseInfo() override; void ShowEnterpriseInfo() override;
void ShowNetworkConfigure(const std::string& network_id) override; void ShowNetworkConfigure(const std::string& network_id) override;
void ShowNetworkCreate(const std::string& type) override; void ShowNetworkCreate(const std::string& type) override;
void ShowSettingsCellularSetupPsimFlow() override;
void ShowThirdPartyVpnCreate(const std::string& extension_id) override; void ShowThirdPartyVpnCreate(const std::string& extension_id) override;
void ShowArcVpnCreate(const std::string& app_id) override; void ShowArcVpnCreate(const std::string& app_id) override;
void ShowNetworkSettings(const std::string& network_id) override; void ShowNetworkSettings(const std::string& network_id) override;
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/numerics/ranges.h" #include "base/numerics/ranges.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h" #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h" #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
#include "components/onc/onc_constants.h" #include "components/onc/onc_constants.h"
...@@ -559,6 +560,11 @@ base::string16 GetLabelForNetworkList(const NetworkStateProperties* network) { ...@@ -559,6 +560,11 @@ base::string16 GetLabelForNetworkList(const NetworkStateProperties* network) {
} }
if (activation_state == ActivationStateType::kNotActivated || if (activation_state == ActivationStateType::kNotActivated ||
activation_state == ActivationStateType::kPartiallyActivated) { activation_state == ActivationStateType::kPartiallyActivated) {
if (base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi)) {
return base::UTF8ToUTF16(network->name);
}
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
IDS_ASH_STATUS_TRAY_NETWORK_LIST_ACTIVATE, IDS_ASH_STATUS_TRAY_NETWORK_LIST_ACTIVATE,
base::UTF8ToUTF16(network->name)); base::UTF8ToUTF16(network->name));
......
...@@ -15,7 +15,9 @@ NetworkInfo::NetworkInfo(const std::string& guid) ...@@ -15,7 +15,9 @@ NetworkInfo::NetworkInfo(const std::string& guid)
connection_state( connection_state(
chromeos::network_config::mojom::ConnectionStateType::kNotConnected), chromeos::network_config::mojom::ConnectionStateType::kNotConnected),
type(chromeos::network_config::mojom::NetworkType::kWiFi), type(chromeos::network_config::mojom::NetworkType::kWiFi),
source(chromeos::network_config::mojom::OncSource::kNone) {} source(chromeos::network_config::mojom::OncSource::kNone),
activation_state(
chromeos::network_config::mojom::ActivationStateType::kUnknown) {}
NetworkInfo::~NetworkInfo() = default; NetworkInfo::~NetworkInfo() = default;
...@@ -24,6 +26,7 @@ bool NetworkInfo::operator==(const NetworkInfo& other) const { ...@@ -24,6 +26,7 @@ bool NetworkInfo::operator==(const NetworkInfo& other) const {
tooltip == other.tooltip && image.BackedBySameObjectAs(other.image) && tooltip == other.tooltip && image.BackedBySameObjectAs(other.image) &&
type == other.type && disable == other.disable && type == other.type && disable == other.disable &&
connection_state == other.connection_state && source == other.source && connection_state == other.connection_state && source == other.source &&
activation_state == other.activation_state &&
battery_percentage == other.battery_percentage; battery_percentage == other.battery_percentage;
} }
......
...@@ -39,6 +39,9 @@ struct NetworkInfo { ...@@ -39,6 +39,9 @@ struct NetworkInfo {
chromeos::network_config::mojom::ConnectionStateType connection_state; chromeos::network_config::mojom::ConnectionStateType connection_state;
chromeos::network_config::mojom::NetworkType type; chromeos::network_config::mojom::NetworkType type;
chromeos::network_config::mojom::OncSource source; chromeos::network_config::mojom::OncSource source;
// Used by cellular networks, for other network types, activation_status is
// set to a default value of kUnknown.
chromeos::network_config::mojom::ActivationStateType activation_state;
int battery_percentage = 0; int battery_percentage = 0;
int signal_strength = 0; int signal_strength = 0;
}; };
......
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "ash/system/tray/tri_view.h" #include "ash/system/tray/tri_view.h"
#include "base/i18n/number_formatting.h" #include "base/i18n/number_formatting.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h" #include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h" #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
#include "components/device_event_log/device_event_log.h" #include "components/device_event_log/device_event_log.h"
...@@ -79,6 +80,13 @@ bool IsManagedByPolicy(const NetworkInfo& info) { ...@@ -79,6 +80,13 @@ bool IsManagedByPolicy(const NetworkInfo& info) {
info.source == OncSource::kUserPolicy; info.source == OncSource::kUserPolicy;
} }
bool ShouldShowActivateCellularNetwork(const NetworkInfo& info) {
return NetworkTypeMatchesType(info.type, NetworkType::kCellular) &&
info.activation_state == ActivationStateType::kNotActivated &&
base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi);
}
} // namespace } // namespace
// NetworkListView: // NetworkListView:
...@@ -142,6 +150,7 @@ void NetworkListView::OnGetNetworkStateList( ...@@ -142,6 +150,7 @@ void NetworkListView::OnGetNetworkStateList(
case NetworkType::kCellular: case NetworkType::kCellular:
activation_state = activation_state =
network->type_state->get_cellular()->activation_state; network->type_state->get_cellular()->activation_state;
info->activation_state = activation_state;
// If cellular is not enabled, skip cellular networks with no service. // If cellular is not enabled, skip cellular networks with no service.
if (model()->GetDeviceState(NetworkType::kCellular) != if (model()->GetDeviceState(NetworkType::kCellular) !=
DeviceStateType::kEnabled && DeviceStateType::kEnabled &&
...@@ -372,10 +381,15 @@ void NetworkListView::UpdateViewForNetwork(HoverHighlightView* view, ...@@ -372,10 +381,15 @@ void NetworkListView::UpdateViewForNetwork(HoverHighlightView* view,
network_image = info.image; network_image = info.image;
} }
view->AddIconAndLabel(network_image, info.label); view->AddIconAndLabel(network_image, info.label);
if (StateIsConnected(info.connection_state)) if (ShouldShowActivateCellularNetwork(info)) {
SetupUnactivatedCellularNetworkListItem(
view, l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CLICK_TO_ACTIVATE));
} else if (StateIsConnected(info.connection_state)) {
SetupConnectedScrollListItem(view); SetupConnectedScrollListItem(view);
else if (info.connection_state == ConnectionStateType::kConnecting) } else if (info.connection_state == ConnectionStateType::kConnecting) {
SetupConnectingScrollListItem(view); SetupConnectingScrollListItem(view);
}
view->SetTooltipText(info.tooltip); view->SetTooltipText(info.tooltip);
// Add an additional icon to the right of the label for networks // Add an additional icon to the right of the label for networks
...@@ -398,6 +412,17 @@ void NetworkListView::UpdateViewForNetwork(HoverHighlightView* view, ...@@ -398,6 +412,17 @@ void NetworkListView::UpdateViewForNetwork(HoverHighlightView* view,
needs_relayout_ = true; needs_relayout_ = true;
} }
void NetworkListView::SetupUnactivatedCellularNetworkListItem(
HoverHighlightView* view,
const base::string16& sub_text) {
DCHECK(view->is_populated());
view->SetSubText(sub_text);
view->sub_text_label()->SetEnabledColor(
AshColorProvider::Get()->GetContentLayerColor(
AshColorProvider::ContentLayerType::kTextColorWarning));
}
base::string16 NetworkListView::GenerateAccessibilityLabel( base::string16 NetworkListView::GenerateAccessibilityLabel(
const NetworkInfo& info) { const NetworkInfo& info) {
if (CanNetworkConnect(info.connection_state, info.type, info.connectable)) { if (CanNetworkConnect(info.connection_state, info.type, info.connectable)) {
...@@ -461,6 +486,12 @@ base::string16 NetworkListView::GenerateAccessibilityDescription( ...@@ -461,6 +486,12 @@ base::string16 NetworkListView::GenerateAccessibilityDescription(
base::FormatPercent(info.signal_strength)); base::FormatPercent(info.signal_strength));
} }
case NetworkType::kCellular: case NetworkType::kCellular:
if (info.activation_state == ActivationStateType::kNotActivated &&
base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi)) {
return l10n_util::GetStringUTF16(
IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CLICK_TO_ACTIVATE);
}
if (!connection_status.empty()) { if (!connection_status.empty()) {
if (IsManagedByPolicy(info)) { if (IsManagedByPolicy(info)) {
return l10n_util::GetStringFUTF16( return l10n_util::GetStringFUTF16(
......
...@@ -83,6 +83,12 @@ class NetworkListView : public NetworkStateListDetailedView, ...@@ -83,6 +83,12 @@ class NetworkListView : public NetworkStateListDetailedView,
// not managed by policy. // not managed by policy.
views::View* CreatePolicyView(const NetworkInfo& info); views::View* CreatePolicyView(const NetworkInfo& info);
// Adds a custom sub label using |sub_text| to the |view| with warning color
// and updates accessibility label. Used when cellular network is not
// activiated.
void SetupUnactivatedCellularNetworkListItem(HoverHighlightView* view,
const base::string16& sub_text);
// Adds or updates child views representing the network connections when // Adds or updates child views representing the network connections when
// |is_wifi| is matching the attribute of a network connection starting at // |is_wifi| is matching the attribute of a network connection starting at
// |child_index|. Returns a set of guids for the added network // |child_index|. Returns a set of guids for the added network
......
...@@ -114,6 +114,19 @@ bool IsArcVpn(const std::string& network_id) { ...@@ -114,6 +114,19 @@ bool IsArcVpn(const std::string& network_id) {
network_state->GetVpnProviderType() == shill::kProviderArcVpn; network_state->GetVpnProviderType() == shill::kProviderArcVpn;
} }
bool ShouldOpenCellularSetupPsimFlowOnClick(const std::string& network_id) {
// |kActivationStateNotActivated| is only set in physical SIM networks,
// checking a networks activation state is |kActivationStateNotActivated|
// ensures the current network is a phyical SIM network.
const chromeos::NetworkState* network_state = GetNetworkState(network_id);
return network_state && network_state->type() == shill::kTypeCellular &&
network_state->activation_state() ==
shill::kActivationStateNotActivated &&
base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi);
}
} // namespace } // namespace
SystemTrayClient::SystemTrayClient() SystemTrayClient::SystemTrayClient()
...@@ -371,11 +384,6 @@ void SystemTrayClient::ShowNetworkConfigure(const std::string& network_id) { ...@@ -371,11 +384,6 @@ void SystemTrayClient::ShowNetworkConfigure(const std::string& network_id) {
void SystemTrayClient::ShowNetworkCreate(const std::string& type) { void SystemTrayClient::ShowNetworkCreate(const std::string& type) {
if (type == ::onc::network_type::kCellular) { if (type == ::onc::network_type::kCellular) {
if (base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi)) {
ShowSettingsCellularSetupFlow();
return;
}
const chromeos::NetworkState* cellular = const chromeos::NetworkState* cellular =
chromeos::NetworkHandler::Get() chromeos::NetworkHandler::Get()
->network_state_handler() ->network_state_handler()
...@@ -388,10 +396,11 @@ void SystemTrayClient::ShowNetworkCreate(const std::string& type) { ...@@ -388,10 +396,11 @@ void SystemTrayClient::ShowNetworkCreate(const std::string& type) {
chromeos::InternetConfigDialog::ShowDialogForNetworkType(type); chromeos::InternetConfigDialog::ShowDialogForNetworkType(type);
} }
void SystemTrayClient::ShowSettingsCellularSetupFlow() { void SystemTrayClient::ShowSettingsCellularSetupPsimFlow() {
// TODO(crbug.com/1093185) Add metrics action recorder // TODO(crbug.com/1093185) Add metrics action recorder
std::string page = chromeos::settings::mojom::kCellularNetworksSubpagePath; std::string page = chromeos::settings::mojom::kCellularNetworksSubpagePath;
page += "&showCellularSetup=true"; page += "&showCellularSetup=true";
page += "&showPsimFlow=true";
ShowSettingsSubPageForActiveUser(page); ShowSettingsSubPageForActiveUser(page);
} }
...@@ -445,6 +454,15 @@ void SystemTrayClient::ShowNetworkSettingsHelper(const std::string& network_id, ...@@ -445,6 +454,15 @@ void SystemTrayClient::ShowNetworkSettingsHelper(const std::string& network_id,
return; return;
} }
if (ShouldOpenCellularSetupPsimFlowOnClick(network_id)) {
// Special case: Clicking on "click to activate" on a psim network item
// should open cellular setup dialogs' psim flow if the device has
// |kUpdatedCellularActivationUi| feature enabled and is a non-activated
// cellular network
ShowSettingsCellularSetupPsimFlow();
return;
}
std::string page = chromeos::settings::mojom::kNetworkSectionPath; std::string page = chromeos::settings::mojom::kNetworkSectionPath;
const chromeos::NetworkState* network_state = GetNetworkState(network_id); const chromeos::NetworkState* network_state = GetNetworkState(network_id);
if (!network_id.empty() && network_state) { if (!network_id.empty() && network_state) {
......
...@@ -75,6 +75,7 @@ class SystemTrayClient : public ash::SystemTrayClient, ...@@ -75,6 +75,7 @@ class SystemTrayClient : public ash::SystemTrayClient,
void ShowEnterpriseInfo() override; void ShowEnterpriseInfo() override;
void ShowNetworkConfigure(const std::string& network_id) override; void ShowNetworkConfigure(const std::string& network_id) override;
void ShowNetworkCreate(const std::string& type) override; void ShowNetworkCreate(const std::string& type) override;
void ShowSettingsCellularSetupPsimFlow() override;
void ShowThirdPartyVpnCreate(const std::string& extension_id) override; void ShowThirdPartyVpnCreate(const std::string& extension_id) override;
void ShowArcVpnCreate(const std::string& app_id) override; void ShowArcVpnCreate(const std::string& app_id) override;
void ShowNetworkSettings(const std::string& network_id) override; void ShowNetworkSettings(const std::string& network_id) override;
...@@ -83,8 +84,6 @@ class SystemTrayClient : public ash::SystemTrayClient, ...@@ -83,8 +84,6 @@ class SystemTrayClient : public ash::SystemTrayClient,
void SetLocaleAndExit(const std::string& locale_iso_code) override; void SetLocaleAndExit(const std::string& locale_iso_code) override;
private: private:
// Opens cellular setup dialog in os settings
void ShowSettingsCellularSetupFlow();
// Helper function shared by ShowNetworkSettings() and ShowNetworkConfigure(). // Helper function shared by ShowNetworkSettings() and ShowNetworkConfigure().
void ShowNetworkSettingsHelper(const std::string& network_id, void ShowNetworkSettingsHelper(const std::string& network_id,
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
#include "chrome/browser/ui/webui/chromeos/cellular_setup/cellular_setup_dialog_launcher.h" #include "chrome/browser/ui/webui/chromeos/cellular_setup/cellular_setup_dialog_launcher.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "chrome/browser/ui/webui/chromeos/cellular_setup/cellular_setup_dialog.h" #include "chrome/browser/ui/ash/system_tray_client.h"
#include "chrome/browser/ui/webui/chromeos/cellular_setup/mobile_setup_dialog.h" #include "chrome/browser/ui/webui/chromeos/cellular_setup/mobile_setup_dialog.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
...@@ -16,7 +16,7 @@ namespace cellular_setup { ...@@ -16,7 +16,7 @@ namespace cellular_setup {
void OpenCellularSetupDialog(const std::string& cellular_network_guid) { void OpenCellularSetupDialog(const std::string& cellular_network_guid) {
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
chromeos::features::kUpdatedCellularActivationUi)) { chromeos::features::kUpdatedCellularActivationUi)) {
CellularSetupDialog::ShowDialog(cellular_network_guid); SystemTrayClient::Get()->ShowSettingsCellularSetupPsimFlow();
} else { } else {
MobileSetupDialog::ShowByNetworkId(cellular_network_guid); MobileSetupDialog::ShowByNetworkId(cellular_network_guid);
} }
......
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