Skip to content

Commit

Permalink
[CrOS PhoneHub] Update onboarding dismiss prompt UI to spec.
Browse files Browse the repository at this point in the history
Removes the header status view on top for the onboarding dismiss prompt
per spec.

Screenshot: https://screenshot.googleplex.com/9a6EphBjLrfBRyP

BUG=1106937

Change-Id: Icc66d781f2df91e7fb74cac1cbbd573225def2bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2522117
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Tim Song <tengs@chromium.org>
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824986}
  • Loading branch information
Meilin Wang authored and Commit Bot committed Nov 6, 2020
1 parent 2802039 commit 6531973
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 32 deletions.
9 changes: 5 additions & 4 deletions ash/system/phonehub/onboarding_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ class OnboardingDismissPromptView : public PhoneHubInterstitialView {
// OnboardingView -------------------------------------------------------------
OnboardingView::OnboardingView(
chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker,
TrayBubbleView* bubble_view,
Delegate* delegate,
OnboardingFlow onboarding_flow)
: onboarding_ui_tracker_(onboarding_ui_tracker), bubble_view_(bubble_view) {
: onboarding_ui_tracker_(onboarding_ui_tracker), delegate_(delegate) {
SetID(PhoneHubViewID::kOnboardingView);

SetLayoutManager(std::make_unique<views::FillLayout>());
Expand Down Expand Up @@ -207,8 +207,9 @@ void OnboardingView::ShowDismissPrompt() {
main_view_ = AddChildView(
std::make_unique<OnboardingDismissPromptView>(onboarding_ui_tracker_));

// Updates bubble to handle size change with a different child view.
bubble_view_->UpdateBubble();
// We don't show status header view on top for the dismiss prompt.
DCHECK(delegate_);
delegate_->HideStatusHeaderView();
}

BEGIN_METADATA(OnboardingView, views::View)
Expand Down
9 changes: 7 additions & 2 deletions ash/system/phonehub/onboarding_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,16 @@ class ASH_EXPORT OnboardingView : public PhoneHubContentView {
public:
METADATA_HEADER(OnboardingView);

class Delegate {
public:
virtual void HideStatusHeaderView() = 0;
};

// The different onboarding flows that are supported.
enum OnboardingFlow { kExistingMultideviceUser = 0, kNewMultideviceUser };

OnboardingView(chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker,
TrayBubbleView* bubble_view,
Delegate* delegate,
OnboardingFlow onboarding_flow);
OnboardingView(const OnboardingView&) = delete;
OnboardingView& operator=(const OnboardingView&) = delete;
Expand All @@ -55,7 +60,7 @@ class ASH_EXPORT OnboardingView : public PhoneHubContentView {
PhoneHubInterstitialView* main_view_ = nullptr;

chromeos::phonehub::OnboardingUiTracker* onboarding_ui_tracker_ = nullptr;
TrayBubbleView* bubble_view_ = nullptr;
Delegate* delegate_ = nullptr;
};

} // namespace ash
Expand Down
26 changes: 15 additions & 11 deletions ash/system/phonehub/phone_hub_tray.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void PhoneHubTray::OnPhoneHubUiStateChanged() {

DCHECK(ui_controller_.get());
std::unique_ptr<PhoneHubContentView> content_view =
ui_controller_->CreateContentView(bubble_view);
ui_controller_->CreateContentView(this);
if (!content_view.get()) {
CloseBubble();
return;
Expand Down Expand Up @@ -168,20 +168,16 @@ void PhoneHubTray::ShowBubble(bool show_by_click) {
bubble_view->set_margins(GetSecondaryBubbleInsets());
bubble_view->SetBorder(views::CreateEmptyBorder(kBubblePadding));

// We will always have this phone status view on top of the bubble view
// to display any available phone status and the settings icon.
std::unique_ptr<views::View> phone_status =
ui_controller_->CreateStatusHeaderView(this);
if (phone_status) {
phone_status->SetPaintToLayer();
phone_status->layer()->SetFillsBoundsOpaquely(false);
bubble_view->AddChildView(std::move(phone_status));
}
// Creates header view on top for displaying phone status and settings icon.
auto phone_status = ui_controller_->CreateStatusHeaderView(this);
phone_status_view_ = phone_status.get();
DCHECK(phone_status_view_);
bubble_view->AddChildView(std::move(phone_status));

// Other contents, i.e. the connected view and the interstitial views,
// will be positioned underneath the phone status view and updated based
// on the current mode.
auto content_view = ui_controller_->CreateContentView(bubble_view);
auto content_view = ui_controller_->CreateContentView(this);
content_view_ = content_view.get();
DCHECK(content_view_);
bubble_view->AddChildView(std::move(content_view));
Expand Down Expand Up @@ -224,6 +220,14 @@ void PhoneHubTray::OpenConnectedDevicesSettings() {
Shell::Get()->system_tray_model()->client()->ShowConnectedDevicesSettings();
}

void PhoneHubTray::HideStatusHeaderView() {
if (!phone_status_view_)
return;

phone_status_view_->SetVisible(false);
bubble_->bubble_view()->UpdateBubble();
}

void PhoneHubTray::CloseBubble() {
if (!bubble_)
return;
Expand Down
8 changes: 8 additions & 0 deletions ash/system/phonehub/phone_hub_tray.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define ASH_SYSTEM_PHONEHUB_PHONE_HUB_TRAY_H_

#include "ash/ash_export.h"
#include "ash/system/phonehub/onboarding_view.h"
#include "ash/system/phonehub/phone_hub_content_view.h"
#include "ash/system/phonehub/phone_hub_ui_controller.h"
#include "ash/system/phonehub/phone_status_view.h"
Expand All @@ -30,6 +31,7 @@ class TrayBubbleWrapper;
// This class represents the Phone Hub tray button in the status area and
// controls the bubble that is shown when the tray button is clicked.
class ASH_EXPORT PhoneHubTray : public TrayBackgroundView,
public OnboardingView::Delegate,
public PhoneStatusView::Delegate,
public PhoneHubUiController::Observer {
public:
Expand Down Expand Up @@ -59,6 +61,9 @@ class ASH_EXPORT PhoneHubTray : public TrayBackgroundView,
bool CanOpenConnectedDeviceSettings() override;
void OpenConnectedDevicesSettings() override;

// OnboardingView::Delegate:
void HideStatusHeaderView() override;

views::View* content_view_for_testing() { return content_view_; }

PhoneHubUiController* ui_controller_for_testing() {
Expand Down Expand Up @@ -88,6 +93,9 @@ class ASH_EXPORT PhoneHubTray : public TrayBackgroundView,
// The bubble that appears after clicking the tray button.
std::unique_ptr<TrayBubbleWrapper> bubble_;

// The header status view on top of the bubble.
views::View* phone_status_view_ = nullptr;

// The main content view of the bubble, which changes depending on the state.
// Unowned.
PhoneHubContentView* content_view_ = nullptr;
Expand Down
6 changes: 3 additions & 3 deletions ash/system/phonehub/phone_hub_ui_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,17 @@ std::unique_ptr<views::View> PhoneHubUiController::CreateStatusHeaderView(
}

std::unique_ptr<PhoneHubContentView> PhoneHubUiController::CreateContentView(
TrayBubbleView* bubble_view) {
OnboardingView::Delegate* delegate) {
switch (ui_state_) {
case UiState::kHidden:
return nullptr;
case UiState::kOnboardingWithoutPhone:
return std::make_unique<OnboardingView>(
phone_hub_manager_->GetOnboardingUiTracker(), bubble_view,
phone_hub_manager_->GetOnboardingUiTracker(), delegate,
OnboardingView::kNewMultideviceUser);
case UiState::kOnboardingWithPhone:
return std::make_unique<OnboardingView>(
phone_hub_manager_->GetOnboardingUiTracker(), bubble_view,
phone_hub_manager_->GetOnboardingUiTracker(), delegate,
OnboardingView::kExistingMultideviceUser);
case UiState::kBluetoothDisabled:
return std::make_unique<BluetoothDisabledView>();
Expand Down
5 changes: 2 additions & 3 deletions ash/system/phonehub/phone_hub_ui_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define ASH_SYSTEM_PHONEHUB_PHONE_HUB_UI_CONTROLLER_H_

#include "ash/ash_export.h"
#include "ash/system/phonehub/onboarding_view.h"
#include "ash/system/phonehub/phone_hub_content_view.h"
#include "ash/system/phonehub/phone_status_view.h"
#include "base/observer_list.h"
Expand All @@ -25,8 +26,6 @@ class View;

namespace ash {

class TrayBubbleView;

// This controller translates the state received from PhoneHubManager into the
// corresponding main content view to be displayed in the tray bubble.
class ASH_EXPORT PhoneHubUiController
Expand Down Expand Up @@ -65,7 +64,7 @@ class ASH_EXPORT PhoneHubUiController
// Creates the corresponding content view for the current UI state.
// |bubble_view| will be the parent the created content view.
std::unique_ptr<PhoneHubContentView> CreateContentView(
TrayBubbleView* bubble_view);
OnboardingView::Delegate* delegate);

// Creates the header view displaying the phone status.
std::unique_ptr<views::View> CreateStatusHeaderView(
Expand Down
18 changes: 9 additions & 9 deletions ash/system/phonehub/phone_hub_ui_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ TEST_F(PhoneHubUiControllerTest, NotEligibleForFeature) {
GetFeatureStatusProvider()->SetStatus(FeatureStatus::kNotEligibleForFeature);
EXPECT_EQ(PhoneHubUiController::UiState::kHidden, controller_.ui_state());
EXPECT_TRUE(ui_state_changed_);
EXPECT_FALSE(controller_.CreateContentView(/*bubble_view=*/nullptr).get());
EXPECT_FALSE(controller_.CreateContentView(/*delegate=*/nullptr).get());
}

TEST_F(PhoneHubUiControllerTest, OnboardingNotEligible) {
GetFeatureStatusProvider()->SetStatus(FeatureStatus::kDisabled);
EXPECT_EQ(PhoneHubUiController::UiState::kHidden, controller_.ui_state());
EXPECT_FALSE(controller_.CreateContentView(/*bubble_view=*/nullptr).get());
EXPECT_FALSE(controller_.CreateContentView(/*delegate=*/nullptr).get());
}

TEST_F(PhoneHubUiControllerTest, ShowOnboardingUi_WithoutPhone) {
Expand All @@ -78,7 +78,7 @@ TEST_F(PhoneHubUiControllerTest, ShowOnboardingUi_WithoutPhone) {
EXPECT_EQ(PhoneHubUiController::UiState::kOnboardingWithoutPhone,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kOnboardingView, content_view->GetID());
}

Expand All @@ -93,7 +93,7 @@ TEST_F(PhoneHubUiControllerTest, ShowOnboardingUi_WithPhone) {
EXPECT_EQ(PhoneHubUiController::UiState::kOnboardingWithPhone,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kOnboardingView, content_view->GetID());
}

Expand All @@ -103,7 +103,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneConnectingForOnboarding) {
EXPECT_EQ(PhoneHubUiController::UiState::kInitialConnecting,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kInitialConnectingView, content_view->GetID());
}

Expand All @@ -113,7 +113,7 @@ TEST_F(PhoneHubUiControllerTest, BluetoothOff) {
EXPECT_EQ(PhoneHubUiController::UiState::kBluetoothDisabled,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kBluetoothDisabledView, content_view->GetID());
}

Expand All @@ -122,7 +122,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneDisconnected) {
EXPECT_EQ(PhoneHubUiController::UiState::kConnectionError,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kDisconnectedView, content_view->GetID());
}

Expand All @@ -131,7 +131,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneConnecting) {
EXPECT_EQ(PhoneHubUiController::UiState::kPhoneConnecting,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(PhoneHubViewID::kReconnectingView, content_view->GetID());
}

Expand All @@ -140,7 +140,7 @@ TEST_F(PhoneHubUiControllerTest, PhoneConnected) {
EXPECT_EQ(PhoneHubUiController::UiState::kPhoneConnected,
controller_.ui_state());

auto content_view = controller_.CreateContentView(/*bubble_view=*/nullptr);
auto content_view = controller_.CreateContentView(/*delegate=*/nullptr);
EXPECT_EQ(kPhoneConnectedView, content_view->GetID());
}

Expand Down
2 changes: 2 additions & 0 deletions ash/system/phonehub/phone_status_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ PhoneStatusView::PhoneStatusView(chromeos::phonehub::PhoneModel* phone_model,
battery_label_(new views::Label) {
DCHECK(delegate);

SetPaintToLayer();
layer()->SetFillsBoundsOpaquely(false);
SetID(PhoneHubViewID::kPhoneStatusView);

SetBorder(views::CreateEmptyBorder(kBorderInsets));
Expand Down

0 comments on commit 6531973

Please sign in to comment.