0

Add "+ wifi network" entry in the qs network subpage

Screenshot:
https://screenshot.googleplex.com/BySeYWH9yDA7rQx

Bug: b/253086997
Change-Id: I29949e21a8b3df1a75852277922c534f460c9013
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4156639
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Commit-Queue: Jiaming Cheng <jiamingc@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Olivier Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1092047}
This commit is contained in:
Jiaming Cheng
2023-01-12 21:45:29 +00:00
committed by Chromium LUCI CQ
parent 28f48d7ecf
commit 253063103d
14 changed files with 136 additions and 18 deletions

@ -2311,6 +2311,9 @@ Connect your device to power.
<message name="IDS_ASH_QUICK_SETTINGS_UNKNOWN_NETWORKS" desc="The label used in the network detailed page for the unknown networks group">
Unknown networks
</message>
<message name="IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK" desc="The label used in the network detailed page for the Join Wi-Fi network entry">
Join Wi-Fi network
</message>
<message name="IDS_ASH_STATUS_TRAY_NETWORK_A11Y_LABEL_OPEN" desc="Accessibility label used for a network in quick settings network list that opens network settings page.">
Open settings for <ph name="NETWORK_NAME">$1<ex>Ethernet</ex></ph>
</message>

@ -0,0 +1 @@
123d65c68c27f1b901ad27f37431ad472d39bfca

@ -27,6 +27,9 @@ enum ViewID {
VIEW_ID_CAST_CAST_VIEW_LABEL,
VIEW_ID_CAST_MAIN_VIEW,
VIEW_ID_CAST_SELECT_VIEW,
// The entry to add wifi network in the quick settings network subpage.
VIEW_ID_JOIN_NETWORK_ENTRY,
VIEW_ID_MEDIA_TRAY_VIEW,
// System tray quick settings view buttons shown in the root QS view:

@ -59,6 +59,10 @@ FakeNetworkDetailedNetworkView::AddWifiSectionHeader() {
return network_list_->AddChildView(std::move(wifi_header_view));
}
HoverHighlightView* FakeNetworkDetailedNetworkView::AddJoinNetworkEntry() {
return nullptr;
}
NetworkListMobileHeaderView*
FakeNetworkDetailedNetworkView::AddMobileSectionHeader() {
std::unique_ptr<FakeNetworkListMobileHeaderView> mobile_header_view =

@ -48,6 +48,7 @@ class ASH_EXPORT FakeNetworkDetailedNetworkView
NetworkListNetworkItemView* AddNetworkListItem(
chromeos::network_config::mojom::NetworkType type) override;
NetworkListWifiHeaderView* AddWifiSectionHeader() override;
HoverHighlightView* AddJoinNetworkEntry() override;
NetworkListMobileHeaderView* AddMobileSectionHeader() override;
void UpdateScanningBarVisibility(bool visible) override;
void ReorderFirstListView(size_t index) override {}

@ -11,6 +11,7 @@
#include "ash/system/network/network_list_network_header_view.h"
#include "ash/system/network/network_list_network_item_view.h"
#include "ash/system/network/network_list_wifi_header_view_impl.h"
#include "ash/system/tray/hover_highlight_view.h"
#include "ui/views/view.h"
namespace ash {
@ -70,6 +71,11 @@ class ASH_EXPORT NetworkDetailedNetworkView {
virtual NetworkListNetworkItemView* AddNetworkListItem(
chromeos::network_config::mojom::NetworkType type) = 0;
// Creates, adds and returns a `HoverHighlightView`, which is the "Join WIFI
// network" entry. The client is expected to use the returned pointer for
// removing and rearranging this entry.
virtual HoverHighlightView* AddJoinNetworkEntry() = 0;
// Creates, adds and returns a Wifi sticky sub-header to the end of the
// network list. The client is expected to use the returned pointer for
// removing and rearranging the sub-header.

@ -5,8 +5,12 @@
#include "ash/system/network/network_detailed_network_view_impl.h"
#include "ash/constants/ash_features.h"
#include "ash/public/cpp/ash_view_ids.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/style/ash_color_id.h"
#include "ash/style/rounded_container.h"
#include "ash/system/network/network_detailed_view.h"
#include "ash/system/network/network_list_mobile_header_view_impl.h"
@ -15,9 +19,13 @@
#include "ash/system/network/network_utils.h"
#include "ash/system/network/tray_network_state_model.h"
#include "ash/system/tray/detailed_view_delegate.h"
#include "ash/system/tray/tray_popup_utils.h"
#include "base/notreached.h"
#include "chromeos/services/network_config/public/mojom/network_types.mojom-shared.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/view.h"
#include "ui/views/view_class_properties.h"
@ -74,6 +82,28 @@ NetworkListNetworkItemView* NetworkDetailedNetworkViewImpl::AddNetworkListItem(
std::make_unique<NetworkListNetworkItemView>(/*listener=*/this));
}
HoverHighlightView* NetworkDetailedNetworkViewImpl::AddJoinNetworkEntry() {
HoverHighlightView* entry =
GetNetworkList(NetworkType::kWiFi)
->AddChildView(
std::make_unique<HoverHighlightView>(/*listener=*/this));
entry->SetID(VIEW_ID_JOIN_NETWORK_ENTRY);
auto image_view = std::make_unique<views::ImageView>();
image_view->SetImage(ui::ImageModel::FromVectorIcon(
kSystemMenuPlusIcon, cros_tokens::kCrosSysPrimary));
entry->AddViewAndLabel(
std::move(image_view),
l10n_util::GetStringUTF16(IDS_ASH_QUICK_SETTINGS_JOIN_WIFI_NETWORK));
views::Label* label = entry->text_label();
label->SetEnabledColorId(cros_tokens::kCrosSysPrimary);
// TODO(b/253086997): Apply the correct font to the label.
TrayPopupUtils::SetLabelFontList(
label, TrayPopupUtils::FontStyle::kDetailedViewLabel);
return entry;
}
NetworkListWifiHeaderView*
NetworkDetailedNetworkViewImpl::AddWifiSectionHeader() {
if (!wifi_top_container_ && features::IsQsRevampEnabled()) {

@ -44,6 +44,7 @@ class ASH_EXPORT NetworkDetailedNetworkViewImpl
views::View* GetAsView() override;
NetworkListNetworkItemView* AddNetworkListItem(
chromeos::network_config::mojom::NetworkType type) override;
HoverHighlightView* AddJoinNetworkEntry() override;
NetworkListMobileHeaderView* AddMobileSectionHeader() override;
NetworkListWifiHeaderView* AddWifiSectionHeader() override;
void UpdateScanningBarVisibility(bool visible) override;

@ -7,6 +7,7 @@
#include <memory>
#include <utility>
#include "ash/public/cpp/ash_view_ids.h"
#include "ash/public/cpp/system_tray_client.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
@ -21,6 +22,7 @@
#include "base/memory/weak_ptr.h"
#include "base/metrics/user_metrics.h"
#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
#include "components/onc/onc_constants.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/controls/button/button.h"
@ -48,8 +50,17 @@ NetworkDetailedView::NetworkDetailedView(
NetworkDetailedView::~NetworkDetailedView() = default;
void NetworkDetailedView::HandleViewClicked(views::View* view) {
if (login_ == LoginStatus::LOCKED)
if (login_ == LoginStatus::LOCKED) {
return;
}
if (view->GetID() == VIEW_ID_JOIN_NETWORK_ENTRY) {
base::RecordAction(
base::UserMetricsAction("QS_Subpage_Network_JoinNetwork"));
Shell::Get()->system_tray_model()->client()->ShowNetworkCreate(
onc::network_type::kWiFi);
return;
}
delegate()->OnNetworkListItemSelected(
static_cast<NetworkListItemView*>(view)->network_properties());
}
@ -91,8 +102,9 @@ void NetworkDetailedView::OnInfoBubbleDestroyed() {
}
void NetworkDetailedView::OnInfoClicked() {
if (CloseInfoBubble())
if (CloseInfoBubble()) {
return;
}
info_bubble_ =
new NetworkInfoBubble(weak_ptr_factory_.GetWeakPtr(), tri_view());
@ -101,8 +113,9 @@ void NetworkDetailedView::OnInfoClicked() {
}
bool NetworkDetailedView::CloseInfoBubble() {
if (!info_bubble_)
if (!info_bubble_) {
return false;
}
info_bubble_->GetWidget()->Close();
return true;
@ -125,8 +138,9 @@ void NetworkDetailedView::OnSettingsClicked() {
SystemTrayClient* system_tray_client =
Shell::Get()->system_tray_model()->client();
if (system_tray_client)
if (system_tray_client) {
system_tray_client->ShowNetworkSettings(guid);
}
}
} // namespace ash

@ -336,7 +336,10 @@ void NetworkListViewControllerImpl::OnGetNetworkStateList(
} else {
RemoveAndResetViewIfExists(&unknown_header_);
}
network_item_index = CreateJoinWifiEntry(network_item_index);
if (!is_wifi_enabled_) {
RemoveAndResetViewIfExists(&join_wifi_entry_);
}
network_detailed_network_view()->ReorderNetworkListView(index++);
} else {
@ -613,6 +616,18 @@ size_t NetworkListViewControllerImpl::CreateWifiGroupHeader(
return index;
}
size_t NetworkListViewControllerImpl::CreateJoinWifiEntry(size_t index) {
if (join_wifi_entry_) {
network_detailed_network_view()
->GetNetworkList(NetworkType::kWiFi)
->ReorderChildView(join_wifi_entry_, index++);
return index;
}
join_wifi_entry_ = network_detailed_network_view()->AddJoinNetworkEntry();
return index++;
}
void NetworkListViewControllerImpl::UpdateMobileSection() {
if (!mobile_header_view_) {
return;

@ -30,10 +30,11 @@
namespace views {
class ImageView;
class Label;
}
} // namespace views
namespace ash {
class HoverHighlightView;
class NetworkDetailedNetworkView;
// Implementation of NetworkListViewController.
@ -121,6 +122,9 @@ class ASH_EXPORT NetworkListViewControllerImpl
// `unknown_header_`.
size_t CreateWifiGroupHeader(size_t index, const bool is_known);
// Creates and adds the join wifi entry at the bottom of the wifi networks.
size_t CreateJoinWifiEntry(size_t index);
// Updates Mobile data section, updates add eSIM button states and
// calls UpdateMobileToggleAndSetStatusMessage().
void UpdateMobileSection();
@ -215,6 +219,7 @@ class ASH_EXPORT NetworkListViewControllerImpl
// Owned by views hierarchy.
views::Label* known_header_ = nullptr;
views::Label* unknown_header_ = nullptr;
HoverHighlightView* join_wifi_entry_ = nullptr;
bool has_mobile_networks_;
bool has_wifi_networks_;

@ -9,6 +9,7 @@
#include <memory>
#include "ash/constants/ash_features.h"
#include "ash/public/cpp/ash_view_ids.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
@ -19,6 +20,7 @@
#include "ash/system/network/network_utils.h"
#include "ash/system/network/tray_network_state_model.h"
#include "ash/system/tray/detailed_view_delegate.h"
#include "ash/system/tray/hover_highlight_view.h"
#include "ash/system/tray/tray_info_label.h"
#include "ash/system/tray/tri_view.h"
#include "ash/test/ash_test_base.h"
@ -26,6 +28,7 @@
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chromeos/ash/services/bluetooth_config/fake_adapter_state_controller.h"
#include "chromeos/ash/services/bluetooth_config/public/mojom/cros_bluetooth_config.mojom.h"
@ -219,21 +222,23 @@ class NetworkListViewControllerTest : public AshTestBase,
detailed_view_delegate_ =
std::make_unique<DetailedViewDelegate>(/*tray_controller=*/nullptr);
network_detailed_network_view_ =
widget_ = CreateFramelessTestWidget();
widget_->SetFullscreen(true);
network_detailed_network_view_ = widget_->SetContentsView(
std::make_unique<NetworkDetailedNetworkViewImpl>(
detailed_view_delegate_.get(),
&fake_network_detailed_network_delagte_);
&fake_network_detailed_network_delagte_));
network_list_view_controller_impl_ =
std::make_unique<NetworkListViewControllerImpl>(
network_detailed_network_view_.get());
network_detailed_network_view_);
}
bool IsQsRevampEnabled() { return GetParam(); }
void TearDown() override {
network_list_view_controller_impl_.reset();
network_detailed_network_view_.reset();
widget_.reset();
AshTestBase::TearDown();
}
@ -251,6 +256,10 @@ class NetworkListViewControllerTest : public AshTestBase,
NetworkListMobileHeaderViewImpl::kAddESimButtonId);
}
HoverHighlightView* GetAddWifiEntry() {
return FindViewById<HoverHighlightView*>(VIEW_ID_JOIN_NETWORK_ENTRY);
}
NetworkListMobileHeaderView* GetMobileSubHeader() {
return network_list_view_controller_impl_->mobile_header_view_;
}
@ -462,7 +471,7 @@ class NetworkListViewControllerTest : public AshTestBase,
views::View* network_list(NetworkType type) {
return static_cast<NetworkDetailedNetworkView*>(
network_detailed_network_view_.get())
network_detailed_network_view_)
->GetNetworkList(type);
}
@ -473,8 +482,8 @@ class NetworkListViewControllerTest : public AshTestBase,
private:
template <class T>
T FindViewById(int id) {
return static_cast<T>(network_detailed_network_view_.get()->GetViewByID(
static_cast<int>(id)));
return static_cast<T>(
network_detailed_network_view_->GetViewByID(static_cast<int>(id)));
}
ScopedBluetoothConfigTestHelper* bluetooth_config_test_helper() {
@ -485,8 +494,11 @@ class NetworkListViewControllerTest : public AshTestBase,
std::unique_ptr<FakeCrosNetworkConfig> cros_network_;
FakeNetworkDetailedNetworkViewDelegate fake_network_detailed_network_delagte_;
std::unique_ptr<DetailedViewDelegate> detailed_view_delegate_;
std::unique_ptr<NetworkDetailedNetworkViewImpl>
network_detailed_network_view_;
std::unique_ptr<views::Widget> widget_;
// Owned by `widget_`.
NetworkDetailedNetworkViewImpl* network_detailed_network_view_ = nullptr;
std::unique_ptr<NetworkListViewControllerImpl>
network_list_view_controller_impl_;
};
@ -843,6 +855,7 @@ TEST_P(NetworkListViewControllerTest, HasCorrectWifiNetworkList) {
->GetText());
CheckNetworkListItem(NetworkType::kWiFi, /*index=*/1u,
/*guid=*/kWifiName);
EXPECT_TRUE(GetAddWifiEntry()->GetVisible());
} else {
// Wifi list item be at index 4 after Mobile header, Mobile network
// item, Wifi separator and header.
@ -873,6 +886,14 @@ TEST_P(NetworkListViewControllerTest, HasCorrectWifiNetworkList) {
CheckNetworkListItem(NetworkType::kWiFi, /*index=*/5u,
/*guid=*/kWifiName2);
}
if (IsQsRevampEnabled()) {
base::UserActionTester user_action_tester;
EXPECT_EQ(
0, user_action_tester.GetActionCount("QS_Subpage_Network_JoinNetwork"));
LeftClickOn(GetAddWifiEntry());
EXPECT_EQ(
1, user_action_tester.GetActionCount("QS_Subpage_Network_JoinNetwork"));
}
}
TEST_P(NetworkListViewControllerTest,

@ -32,7 +32,9 @@ using chromeos::network_config::mojom::NetworkType;
NetworkListWifiHeaderViewImpl::NetworkListWifiHeaderViewImpl(
NetworkListNetworkHeaderView::Delegate* delegate)
: NetworkListWifiHeaderView(delegate) {
AddExtraButtons();
if (!features::IsQsRevampEnabled()) {
AddExtraButtons();
}
}
NetworkListWifiHeaderViewImpl::~NetworkListWifiHeaderViewImpl() = default;
@ -53,7 +55,9 @@ void NetworkListWifiHeaderViewImpl::AddExtraButtons() {
void NetworkListWifiHeaderViewImpl::SetToggleState(bool enabled,
bool is_on,
bool animate_toggle) {
join_wifi_button_->SetEnabled(enabled && is_on);
if (!features::IsQsRevampEnabled()) {
join_wifi_button_->SetEnabled(enabled && is_on);
}
NetworkListNetworkHeaderView::SetToggleState(enabled, is_on, animate_toggle);
}
@ -71,8 +75,9 @@ void NetworkListWifiHeaderViewImpl::JoinWifiButtonPressed() {
void NetworkListWifiHeaderViewImpl::SetJoinWifiButtonState(bool enabled,
bool visible) {
if (!join_wifi_button_)
if (!join_wifi_button_) {
return;
}
join_wifi_button_->SetEnabled(enabled);
join_wifi_button_->SetVisible(visible);

@ -26735,6 +26735,15 @@ should be able to be added at any place in this file.
</description>
</action>
<action name="QS_Subpage_Network_JoinNetwork">
<owner>jiamingc@google.com</owner>
<owner>cros-status-area-eng@google.com</owner>
<description>
User clicked on the Join Wi-Fi network entry in the quick settings network
subpage on ChromeOS.
</description>
</action>
<action name="QuickActionSearchWidget.LensQuery">
<owner>ender@google.com</owner>
<owner>fgorski@chromium.org</owner>