Commit d7ba8589 authored by Andre Le's avatar Andre Le Committed by Commit Bot

[CrOS PhoneHub] Add phone name to PhoneHub notification.

- Added phone name to the notification and fix the text layout according
to spec.
- Added IDs to access app name and summary text in NotificationViewMD
for customization.

Screenshots:
https://screenshot.googleplex.com/3sbTp3UdCCbmEHZ
https://screenshot.googleplex.com/62qWt4XimsUwMKB
https://screenshot.googleplex.com/5ie8JCirGmYpeS2
https://screenshot.googleplex.com/5KvapXRjX9zBKXP

Bug: 1106937,1126208
Change-Id: I401566ad05e248a724f3665f0c1e4c9d1a7891c5
Fixed: 1147264
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532041Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarTim Song <tengs@chromium.org>
Commit-Queue: Andre Le <leandre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826990}
parent 69c25dfd
......@@ -1159,9 +1159,6 @@ This file contains the strings for ash.
<message name="IDS_ASH_PHONE_HUB_BLUETOOTH_DISABLED_DIALOG_LEARN_MORE_BUTTON" desc="Learn more button on the bluetooth disabled dialog to show more related information.">
Learn more
</message>
<message name="IDS_ASH_PHONE_HUB_NOTIFICATION_FROM_PHONE_TITLE" desc="The title at the header of PhoneHub notification, indicating that this notification was coming from the connected phone.">
From Phone
</message>
<message name="IDS_ASH_PHONE_HUB_NOTIFICATION_INLINE_REPLY_BUTTON" desc="Label for the inline reply button inside a PhoneHub notification.">
Reply
</message>
......
9cc1f57092e64a415e36c5a377cdba7216e3a31b
\ No newline at end of file
......@@ -20,8 +20,10 @@
#include "base/timer/timer.h"
#include "chromeos/components/phonehub/notification.h"
#include "chromeos/components/phonehub/phone_hub_manager.h"
#include "chromeos/components/phonehub/phone_model.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/text_elider.h"
#include "ui/message_center/message_center.h"
#include "ui/message_center/public/cpp/notification.h"
#include "ui/message_center/public/cpp/notification_delegate.h"
......@@ -39,6 +41,8 @@ const char kPhoneHubInstantTetherNotificationId[] =
"chrome://phonehub-instant-tether";
const char kNotificationCustomViewType[] = "phonehub";
const int kReplyButtonIndex = 0;
const int kNotificationHeaderTextWidth = 180;
const int kNotificationAppNameMaxWidth = 140;
// The amount of time the reply button is disabled after sending an inline
// reply. This is used to make sure that all the replies are received by the
......@@ -50,14 +54,27 @@ constexpr base::TimeDelta kInlineReplyDisableTime =
class PhoneHubNotificationView : public message_center::NotificationViewMD {
public:
explicit PhoneHubNotificationView(
const message_center::Notification& notification)
const message_center::Notification& notification,
const base::string16& phone_name)
: message_center::NotificationViewMD(notification) {
// Add customized header.
message_center::NotificationHeaderView* header_row =
static_cast<message_center::NotificationHeaderView*>(
GetViewByID(message_center::NotificationViewMD::kHeaderRow));
header_row->SetSummaryText(l10n_util::GetStringUTF16(
IDS_ASH_PHONE_HUB_NOTIFICATION_FROM_PHONE_TITLE));
views::View* app_name_view =
GetViewByID(message_center::NotificationViewMD::kAppNameView);
views::Label* summary_text_view = static_cast<views::Label*>(
GetViewByID(message_center::NotificationViewMD::kSummaryTextView));
// The app name should be displayed in full, leaving the rest of the space
// for device name. App name will only be truncated when it reached it
// maximum width.
int app_name_width = std::min(app_name_view->GetPreferredSize().width(),
kNotificationAppNameMaxWidth);
int device_name_width = kNotificationHeaderTextWidth - app_name_width;
header_row->SetSummaryText(
gfx::ElideText(phone_name, summary_text_view->font_list(),
device_name_width, gfx::ELIDE_TAIL));
action_buttons_row_ =
GetViewByID(message_center::NotificationViewMD::kActionButtonsRow);
......@@ -189,7 +206,8 @@ PhoneHubNotificationController::PhoneHubNotificationController() {
message_center::MessageViewFactory::SetCustomNotificationViewFactory(
kNotificationCustomViewType,
base::BindRepeating(
&PhoneHubNotificationController::CreateCustomNotificationView));
&PhoneHubNotificationController::CreateCustomNotificationView,
weak_ptr_factory_.GetWeakPtr()));
}
PhoneHubNotificationController::~PhoneHubNotificationController() {
......@@ -205,6 +223,7 @@ void PhoneHubNotificationController::SetManager(
phone_hub_manager->GetNotificationManager();
chromeos::phonehub::TetherController* tether_controller =
phone_hub_manager->GetTetherController();
phone_model_ = phone_hub_manager->GetPhoneModel();
if (manager_ == notification_manager &&
tether_controller_ == tether_controller) {
......@@ -224,6 +243,12 @@ void PhoneHubNotificationController::SetManager(
tether_controller_->AddObserver(this);
}
const base::string16 PhoneHubNotificationController::GetPhoneName() const {
if (!phone_model_)
return base::string16();
return phone_model_->phone_name().value_or(base::string16());
}
void PhoneHubNotificationController::OnNotificationsAdded(
const base::flat_set<int64_t>& notification_ids) {
for (int64_t id : notification_ids) {
......@@ -402,9 +427,15 @@ PhoneHubNotificationController::CreateNotification(
// static
std::unique_ptr<message_center::MessageView>
PhoneHubNotificationController::CreateCustomNotificationView(
base::WeakPtr<PhoneHubNotificationController> notification_controller,
const message_center::Notification& notification) {
DCHECK_EQ(kNotificationCustomViewType, notification.custom_view_type());
return std::make_unique<PhoneHubNotificationView>(notification);
base::string16 phone_name = base::string16();
if (notification_controller)
phone_name = notification_controller->GetPhoneName();
return std::make_unique<PhoneHubNotificationView>(notification, phone_name);
}
} // namespace ash
......@@ -15,6 +15,7 @@ namespace chromeos {
namespace phonehub {
class Notification;
class PhoneHubManager;
class PhoneModel;
} // namespace phonehub
} // namespace chromeos
......@@ -42,9 +43,13 @@ class ASH_EXPORT PhoneHubNotificationController
// notifications.
void SetManager(chromeos::phonehub::PhoneHubManager* phone_hub_manager);
const base::string16 GetPhoneName() const;
private:
FRIEND_TEST_ALL_PREFIXES(PhoneHubNotificationControllerTest,
ReplyBrieflyDisabled);
FRIEND_TEST_ALL_PREFIXES(PhoneHubNotificationControllerTest,
NotificationHasPhoneName);
class NotificationDelegate;
......@@ -82,12 +87,16 @@ class ASH_EXPORT PhoneHubNotificationController
static std::unique_ptr<message_center::MessageView>
CreateCustomNotificationView(
base::WeakPtr<PhoneHubNotificationController> notification_controller,
const message_center::Notification& notification);
chromeos::phonehub::NotificationManager* manager_ = nullptr;
chromeos::phonehub::TetherController* tether_controller_ = nullptr;
chromeos::phonehub::PhoneModel* phone_model_ = nullptr;
std::unordered_map<int64_t, std::unique_ptr<NotificationDelegate>>
notification_map_;
base::WeakPtrFactory<PhoneHubNotificationController> weak_ptr_factory_{this};
};
} // namespace ash
......
......@@ -13,6 +13,7 @@
#include "base/test/scoped_feature_list.h"
#include "chromeos/components/phonehub/fake_notification_manager.h"
#include "chromeos/components/phonehub/fake_phone_hub_manager.h"
#include "chromeos/components/phonehub/mutable_phone_model.h"
#include "chromeos/components/phonehub/notification.h"
#include "chromeos/constants/chromeos_features.h"
#include "third_party/skia/include/core/SkBitmap.h"
......@@ -246,6 +247,27 @@ TEST_F(PhoneHubNotificationControllerTest, NotificationHasCustomViewType) {
EXPECT_EQ(kNotificationCustomViewType, notification->custom_view_type());
}
TEST_F(PhoneHubNotificationControllerTest, NotificationHasPhoneName) {
notification_manager_->SetNotificationsInternal(fake_notifications_);
auto* notification =
message_center_->FindVisibleNotificationById(kCrOSNotificationId0);
const base::string16 expected_phone_name = base::UTF8ToUTF16("Phone name");
phone_hub_manager_.mutable_phone_model()->SetPhoneName(expected_phone_name);
auto notification_view =
PhoneHubNotificationController::CreateCustomNotificationView(
controller_->weak_ptr_factory_.GetWeakPtr(), *notification);
auto* notification_view_md =
static_cast<message_center::NotificationViewMD*>(notification_view.get());
views::Label* summary_text_label =
static_cast<views::Label*>(notification_view_md->GetViewByID(
message_center::NotificationViewMD::kSummaryTextView));
// Notification should contain phone name in the summary text.
EXPECT_EQ(expected_phone_name, summary_text_label->GetText());
}
TEST_F(PhoneHubNotificationControllerTest, ReplyBrieflyDisabled) {
notification_manager_->SetNotificationsInternal(fake_notifications_);
auto* notification =
......@@ -253,7 +275,7 @@ TEST_F(PhoneHubNotificationControllerTest, ReplyBrieflyDisabled) {
auto notification_view =
PhoneHubNotificationController::CreateCustomNotificationView(
*notification);
controller_->weak_ptr_factory_.GetWeakPtr(), *notification);
auto* notification_view_md =
static_cast<message_center::NotificationViewMD*>(notification_view.get());
views::View* action_buttons_row = notification_view_md->GetViewByID(
......
......@@ -18,6 +18,7 @@
#include "ui/gfx/paint_vector_icon.h"
#include "ui/message_center/public/cpp/message_center_constants.h"
#include "ui/message_center/vector_icons.h"
#include "ui/message_center/views/notification_view_md.h"
#include "ui/message_center/views/relative_time_formatter.h"
#include "ui/strings/grit/ui_strings.h"
#include "ui/views/border.h"
......@@ -200,6 +201,7 @@ NotificationHeaderView::NotificationHeaderView(PressedCallback callback)
// Explicitly disable multiline to support proper text elision for URLs.
app_name_view_->SetMultiLine(false);
app_name_view_->SetProperty(views::kFlexBehaviorKey, kAppNameFlex);
app_name_view_->SetID(NotificationViewMD::kAppNameView);
AddChildView(app_name_view_);
// Detail views which will be hidden in settings mode.
......@@ -219,6 +221,7 @@ NotificationHeaderView::NotificationHeaderView(PressedCallback callback)
// Summary text view
summary_text_view_ = create_label();
summary_text_view_->SetVisible(false);
summary_text_view_->SetID(NotificationViewMD::kSummaryTextView);
detail_views_->AddChildView(summary_text_view_);
// Timestamp divider
......
......@@ -167,6 +167,8 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD
enum ViewId {
// We start from 1 because 0 is the default view ID.
kHeaderRow = 1,
kAppNameView,
kSummaryTextView,
kActionButtonsRow,
kInlineReply,
};
......
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