Commit c25b1986 authored by Mia Bergeron's avatar Mia Bergeron Committed by Commit Bot

[Lock Screen Media Controls] Tweak UI based on new mocks

This CL rearranges the different components of the CrOS lock screen
media controls based on the newest mocks. This involves resizing most
of the child views and their spacings. The artwork was also resized
and re-positioned. Additionally, the close button was moved from the
main view to the header row child view.

Artist and title data about the current session will eventually be
placed to the right of the artwork, but right now this space is empty.

See the bug for before and after pictures.

Bug: 991647
Change-Id: I7b97f31982ccf2912bd2564d5241bfd849d21d92
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1746554Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Commit-Queue: Mia Bergeron <miaber@google.com>
Cr-Commit-Position: refs/heads/master@{#686253}
parent c0ab70c6
......@@ -12,7 +12,6 @@
#include "ash/strings/grit/ash_strings.h"
#include "components/media_message_center/media_controls_progress_view.h"
#include "components/media_message_center/media_notification_util.h"
#include "components/vector_icons/vector_icons.h"
#include "services/media_session/public/cpp/util.h"
#include "services/media_session/public/mojom/constants.mojom.h"
#include "services/media_session/public/mojom/media_session.mojom.h"
......@@ -27,7 +26,6 @@
#include "ui/views/controls/button/image_button_factory.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/layout/box_layout.h"
#include "ui/views/layout/grid_layout.h"
namespace ash {
......@@ -35,35 +33,29 @@ using media_session::mojom::MediaSessionAction;
namespace {
constexpr SkColor kMediaControlsBackground = SkColorSetA(SK_ColorDKGRAY, 100);
constexpr SkColor kMediaControlsBackground = SkColorSetA(SK_ColorDKGRAY, 150);
constexpr SkColor kMediaButtonColor = SK_ColorWHITE;
// Maximum number of actions that should be displayed on |button_row_|.
constexpr size_t kMaxActions = 5;
// Dimensions.
constexpr int kMediaControlsTotalWidthDp = 320;
constexpr int kMediaControlsTotalHeightDp = 400;
constexpr gfx::Insets kMediaControlsInsets = gfx::Insets(15, 15, 15, 15);
constexpr int kMediaControlsCornerRadius = 8;
constexpr int kMinimumIconSize = 16;
constexpr int kDesiredIconSize = 20;
constexpr int kIconSize = 20;
constexpr gfx::Insets kArtworkInsets = gfx::Insets(0, 25, 20, 25);
constexpr int kMinimumArtworkSize = 200;
constexpr int kDesiredArtworkSize = 300;
constexpr int kArtworkViewWidth = 270;
constexpr int kArtworkViewHeight = 200;
constexpr gfx::Size kMediaButtonSize = gfx::Size(45, 45);
constexpr int kMediaButtonRowSeparator = 10;
constexpr gfx::Insets kButtonRowInsets = gfx::Insets(10, 25, 25, 25);
constexpr int kPlayPauseIconSize = 32;
constexpr int kChangeTrackIconSize = 16;
constexpr int kSeekingIconsSize = 28;
constexpr int kMinimumArtworkSize = 50;
constexpr int kDesiredArtworkSize = 80;
constexpr gfx::Size kArtworkSize = gfx::Size(80, 80);
constexpr gfx::Size kMediaButtonSize = gfx::Size(38, 38);
constexpr int kMediaButtonRowSeparator = 24;
constexpr gfx::Insets kButtonRowInsets = gfx::Insets(10, 0, 0, 0);
constexpr int kPlayPauseIconSize = 28;
constexpr int kChangeTrackIconSize = 14;
constexpr int kSeekingIconsSize = 26;
constexpr gfx::Size kMediaControlsButtonRowSize =
gfx::Size(270, kMediaButtonSize.height());
constexpr int kCloseButtonOffset = 290;
constexpr gfx::Size kCloseButtonSize = gfx::Size(24, 24);
constexpr int kCloseButtonIconSize = 20;
gfx::Size(300, kMediaButtonSize.height());
constexpr int kDragVelocityThreshold = -6;
constexpr int kHeightDismissalThreshold = 20;
......@@ -151,46 +143,23 @@ LockScreenMediaControlsView::LockScreenMediaControlsView(
contents_view_ = AddChildView(std::make_unique<views::View>());
contents_view_->SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical));
views::BoxLayout::Orientation::kVertical, kMediaControlsInsets));
contents_view_->SetBackground(views::CreateRoundedRectBackground(
kMediaControlsBackground, kMediaControlsCornerRadius));
contents_view_->SetPaintToLayer(); // Needed for opacity animation.
contents_view_->layer()->SetFillsBoundsOpaquely(false);
// |close_button_row| contains the close button to dismiss the controls.
auto close_button_row = std::make_unique<NonAccessibleView>();
views::GridLayout* close_button_layout =
close_button_row->SetLayoutManager(std::make_unique<views::GridLayout>());
views::ColumnSet* columns = close_button_layout->AddColumnSet(0);
columns->AddPaddingColumn(0, kCloseButtonOffset);
columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER, 0,
views::GridLayout::USE_PREF, 0, 0);
close_button_layout->StartRowWithPadding(
0, 0, 0, 5 /* padding between close button and top of view */);
auto close_button = CreateVectorImageButton(this);
SetImageFromVectorIcon(close_button.get(), vector_icons::kCloseRoundedIcon,
kCloseButtonIconSize, gfx::kGoogleGrey700);
close_button->SetPreferredSize(kCloseButtonSize);
close_button->SetFocusBehavior(View::FocusBehavior::ALWAYS);
base::string16 close_button_label(
l10n_util::GetStringUTF16(IDS_ASH_LOCK_SCREEN_MEDIA_CONTROLS_CLOSE));
close_button->SetAccessibleName(close_button_label);
close_button_ = close_button_layout->AddView(std::move(close_button));
close_button_->SetVisible(false);
contents_view_->AddChildView(std::move(close_button_row));
// |header_row_| contains the app icon and source title of the current media
// session.
header_row_ =
contents_view_->AddChildView(std::make_unique<MediaControlsHeaderView>());
// session. It also contains the close button.
header_row_ = contents_view_->AddChildView(
std::make_unique<MediaControlsHeaderView>(base::BindOnce(
&LockScreenMediaControlsView::Dismiss, base::Unretained(this))));
auto session_artwork = std::make_unique<views::ImageView>();
session_artwork->SetPreferredSize(
gfx::Size(kArtworkViewWidth, kArtworkViewHeight));
session_artwork->SetBorder(views::CreateEmptyBorder(kArtworkInsets));
session_artwork->SetPreferredSize(kArtworkSize);
session_artwork->SetHorizontalAlignment(
views::ImageView::Alignment::kLeading);
session_artwork_ = contents_view_->AddChildView(std::move(session_artwork));
progress_ = contents_view_->AddChildView(
......@@ -296,7 +265,7 @@ const char* LockScreenMediaControlsView::GetClassName() const {
}
gfx::Size LockScreenMediaControlsView::CalculatePreferredSize() const {
return gfx::Size(kMediaControlsTotalWidthDp, kMediaControlsTotalHeightDp);
return contents_view_->GetPreferredSize();
}
void LockScreenMediaControlsView::Layout() {
......@@ -319,14 +288,14 @@ void LockScreenMediaControlsView::OnMouseEntered(const ui::MouseEvent& event) {
if (is_in_drag_ || contents_view_->layer()->GetAnimator()->is_animating())
return;
close_button_->SetVisible(true);
header_row_->SetCloseButtonVisibility(true);
}
void LockScreenMediaControlsView::OnMouseExited(const ui::MouseEvent& event) {
if (is_in_drag_ || contents_view_->layer()->GetAnimator()->is_animating())
return;
close_button_->SetVisible(false);
header_row_->SetCloseButtonVisibility(false);
}
views::View* LockScreenMediaControlsView::GetMiddleSpacingView() {
......@@ -473,11 +442,6 @@ void LockScreenMediaControlsView::OnImplicitAnimationsCompleted() {
void LockScreenMediaControlsView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == close_button_) {
Dismiss();
return;
}
if (!base::Contains(enabled_actions_,
media_message_center::GetActionFromButtonTag(*sender)) ||
!media_session_id_.has_value()) {
......@@ -593,8 +557,7 @@ void LockScreenMediaControlsView::SetArtwork(
return;
}
session_artwork_->SetImageSize(ScaleSizeToFitView(
img->size(), gfx::Size(kArtworkViewWidth, kArtworkViewHeight)));
session_artwork_->SetImageSize(ScaleSizeToFitView(img->size(), kArtworkSize));
session_artwork_->SetImage(*img);
}
......
......@@ -22,7 +22,6 @@ class Connector;
namespace views {
class ImageView;
class ToggleImageButton;
class ImageButton;
} // namespace views
namespace media_message_center {
......@@ -203,7 +202,6 @@ class ASH_EXPORT LockScreenMediaControlsView
views::ImageView* session_artwork_ = nullptr;
NonAccessibleView* button_row_ = nullptr;
views::ToggleImageButton* play_pause_button_ = nullptr;
views::ImageButton* close_button_ = nullptr;
media_message_center::MediaControlsProgressView* progress_ = nullptr;
// Callbacks.
......
......@@ -30,8 +30,8 @@ using media_session::test::TestMediaController;
namespace {
const int kAppIconSize = 20;
constexpr int kArtworkViewWidth = 270;
constexpr int kArtworkViewHeight = 200;
constexpr int kArtworkViewWidth = 80;
constexpr int kArtworkViewHeight = 80;
const base::string16 kTestAppName = base::ASCIIToUTF16("Test app");
......@@ -202,14 +202,14 @@ class LockScreenMediaControlsViewTest : public LoginTestBase {
return media_controls_view_->session_artwork_;
}
views::ImageButton* close_button() const {
return media_controls_view_->close_button_;
}
media_message_center::MediaControlsProgressView* progress_view() const {
return media_controls_view_->progress_;
}
views::ImageButton* close_button() const {
return header_row()->close_button_for_testing();
}
const views::ImageView* icon_view() const {
return header_row()->app_icon_for_testing();
}
......@@ -590,7 +590,7 @@ TEST_F(LockScreenMediaControlsViewTest, UpdateArtwork) {
// Create artwork that must be scaled down to fit the view.
SkBitmap artwork;
artwork.allocN32Pixels(540, 300);
artwork.allocN32Pixels(200, 100);
media_controls_view_->MediaControllerImageChanged(
media_session::mojom::MediaSessionImageType::kArtwork, artwork);
......@@ -599,10 +599,10 @@ TEST_F(LockScreenMediaControlsViewTest, UpdateArtwork) {
// Verify that the provided artwork is correctly scaled down.
EXPECT_EQ(kArtworkViewWidth, artwork_bounds.width());
EXPECT_EQ(150, artwork_bounds.height());
EXPECT_EQ(40, artwork_bounds.height());
// Create artwork that must be scaled up to fit the view.
artwork.allocN32Pixels(200, 190);
artwork.allocN32Pixels(40, 70);
media_controls_view_->MediaControllerImageChanged(
media_session::mojom::MediaSessionImageType::kArtwork, artwork);
......@@ -610,11 +610,11 @@ TEST_F(LockScreenMediaControlsViewTest, UpdateArtwork) {
artwork_bounds = artwork_view()->GetImageBounds();
// Verify that the provided artwork is correctly scaled up.
EXPECT_EQ(210, artwork_bounds.width());
EXPECT_EQ(45, artwork_bounds.width());
EXPECT_EQ(kArtworkViewHeight, artwork_bounds.height());
// Create artwork that already fits the view size.
artwork.allocN32Pixels(250, kArtworkViewHeight);
artwork.allocN32Pixels(70, kArtworkViewHeight);
media_controls_view_->MediaControllerImageChanged(
media_session::mojom::MediaSessionImageType::kArtwork, artwork);
......@@ -622,7 +622,7 @@ TEST_F(LockScreenMediaControlsViewTest, UpdateArtwork) {
artwork_bounds = artwork_view()->GetImageBounds();
// Verify that the provided artwork size doesn't change.
EXPECT_EQ(250, artwork_bounds.width());
EXPECT_EQ(70, artwork_bounds.width());
EXPECT_EQ(kArtworkViewHeight, artwork_bounds.height());
}
......
......@@ -4,31 +4,44 @@
#include "ash/login/ui/media_controls_header_view.h"
#include "ash/login/ui/non_accessible_view.h"
#include "ash/strings/grit/ash_strings.h"
#include "components/vector_icons/vector_icons.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/font_list.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/background.h"
#include "ui/views/border.h"
#include "ui/views/controls/button/image_button.h"
#include "ui/views/controls/button/image_button_factory.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/controls/label.h"
#include "ui/views/layout/box_layout.h"
#include "ui/views/layout/flex_layout.h"
#include "ui/views/layout/flex_layout_types.h"
#include "ui/views/view_class_properties.h"
namespace ash {
namespace {
constexpr gfx::Insets kMediaControlsHeaderInsets = gfx::Insets(0, 25, 25, 25);
constexpr int kIconSize = 20;
constexpr int kHeaderTextFontSize = 14;
constexpr int kMediaControlsHeaderChildSpacing = 10;
constexpr gfx::Insets kHeaderViewInsets = gfx::Insets(0, 0, 15, 0);
constexpr int kIconSize = 18;
constexpr int kHeaderTextFontSize = 12;
constexpr gfx::Insets kIconPadding = gfx::Insets(1, 1, 1, 1);
constexpr int kIconCornerRadius = 2;
constexpr gfx::Insets kAppNamePadding = gfx::Insets(0, 10, 0, 0);
constexpr int kIconCornerRadius = 1;
constexpr gfx::Size kCloseButtonSize = gfx::Size(20, 20);
constexpr int kCloseButtonIconSize = 18;
constexpr gfx::Size kSpacerPreferredSize = gfx::Size(10, 5);
} // namespace
MediaControlsHeaderView::MediaControlsHeaderView() {
SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kHorizontal, kMediaControlsHeaderInsets,
kMediaControlsHeaderChildSpacing));
MediaControlsHeaderView::MediaControlsHeaderView(
base::OnceClosure close_button_cb)
: close_button_cb_(std::move(close_button_cb)) {
auto* layout = SetLayoutManager(std::make_unique<views::FlexLayout>());
layout->SetInteriorMargin(kHeaderViewInsets);
auto app_icon_view = std::make_unique<views::ImageView>();
app_icon_view->SetImageSize(gfx::Size(kIconSize, kIconSize));
......@@ -51,7 +64,28 @@ MediaControlsHeaderView::MediaControlsHeaderView() {
app_name_view->SetHorizontalAlignment(gfx::ALIGN_LEFT);
app_name_view->SetEnabledColor(SK_ColorWHITE);
app_name_view->SetAutoColorReadabilityEnabled(false);
app_name_view->SetBorder(views::CreateEmptyBorder(kAppNamePadding));
app_name_view_ = AddChildView(std::move(app_name_view));
// Space between app name and close button.
auto spacer = std::make_unique<NonAccessibleView>();
spacer->SetPreferredSize(kSpacerPreferredSize);
spacer->SetProperty(views::kFlexBehaviorKey,
views::FlexSpecification::ForSizeRule(
views::MinimumFlexSizeRule::kScaleToMinimum,
views::MaximumFlexSizeRule::kUnbounded));
AddChildView(std::move(spacer));
auto close_button = CreateVectorImageButton(this);
SetImageFromVectorIcon(close_button.get(), vector_icons::kCloseRoundedIcon,
kCloseButtonIconSize, gfx::kGoogleGrey700);
close_button->SetPreferredSize(kCloseButtonSize);
close_button->SetFocusBehavior(View::FocusBehavior::ALWAYS);
base::string16 close_button_label(
l10n_util::GetStringUTF16(IDS_ASH_LOCK_SCREEN_MEDIA_CONTROLS_CLOSE));
close_button->SetAccessibleName(close_button_label);
close_button->SetVisible(false);
close_button_ = AddChildView(std::move(close_button));
}
MediaControlsHeaderView::~MediaControlsHeaderView() = default;
......@@ -64,10 +98,22 @@ void MediaControlsHeaderView::SetAppName(const base::string16& name) {
app_name_view_->SetText(name);
}
void MediaControlsHeaderView::SetCloseButtonVisibility(bool visible) {
if (visible != close_button_->GetVisible()) {
close_button_->SetVisible(visible);
Layout();
}
}
void MediaControlsHeaderView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
node_data->SetName(app_name_view_->GetText());
}
void MediaControlsHeaderView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
std::move(close_button_cb_).Run();
}
const base::string16& MediaControlsHeaderView::app_name_for_testing() const {
return app_name_view_->GetText();
}
......@@ -76,4 +122,8 @@ const views::ImageView* MediaControlsHeaderView::app_icon_for_testing() const {
return app_icon_view_;
}
} // namespace ash
\ No newline at end of file
views::ImageButton* MediaControlsHeaderView::close_button_for_testing() const {
return close_button_;
}
} // namespace ash
......@@ -6,36 +6,48 @@
#define ASH_LOGIN_UI_MEDIA_CONTROLS_HEADER_VIEW_H_
#include "ash/ash_export.h"
#include "ui/views/controls/button/button.h"
#include "ui/views/view.h"
namespace views {
class ImageView;
class Label;
class ImageButton;
} // namespace views
namespace ash {
class ASH_EXPORT MediaControlsHeaderView : public views::View {
class ASH_EXPORT MediaControlsHeaderView : public views::View,
public views::ButtonListener {
public:
MediaControlsHeaderView();
explicit MediaControlsHeaderView(base::OnceClosure close_button_cb);
~MediaControlsHeaderView() override;
void SetAppIcon(const gfx::ImageSkia& img);
void SetAppName(const base::string16& name);
void SetCloseButtonVisibility(bool visible);
// views::View:
void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
const base::string16& app_name_for_testing() const;
const views::ImageView* app_icon_for_testing() const;
views::ImageButton* close_button_for_testing() const;
private:
views::ImageView* app_icon_view_;
views::Label* app_name_view_;
views::ImageButton* close_button_ = nullptr;
base::OnceClosure close_button_cb_;
DISALLOW_COPY_AND_ASSIGN(MediaControlsHeaderView);
};
} // namespace ash
#endif // ASH_LOGIN_UI_MEDIA_CONTROLS_HEADER_VIEW_H_
\ No newline at end of file
#endif // ASH_LOGIN_UI_MEDIA_CONTROLS_HEADER_VIEW_H_
......@@ -19,10 +19,13 @@ namespace media_message_center {
namespace {
constexpr int kProgressTimeFontSize = 12;
constexpr int kProgressBarAndTimeSpacing = 3;
constexpr int kProgressTimeFontSize = 11;
constexpr int kProgressBarHeight = 4;
constexpr int kMinClickHeight = 14;
constexpr int kMaxClickHeight = 24;
constexpr gfx::Size kTimeSpacingSize = gfx::Size(150, 10);
constexpr gfx::Insets kProgressViewInsets = gfx::Insets(15, 25, 0, 25);
constexpr gfx::Insets kProgressBarInsets = gfx::Insets(5, 0, 5, 0);
constexpr gfx::Insets kProgressViewInsets = gfx::Insets(15, 0, 0, 0);
} // namespace
......@@ -30,10 +33,11 @@ MediaControlsProgressView::MediaControlsProgressView(
base::RepeatingCallback<void(double)> seek_callback)
: seek_callback_(std::move(seek_callback)) {
SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, kProgressViewInsets));
views::BoxLayout::Orientation::kVertical, kProgressViewInsets,
kProgressBarAndTimeSpacing));
progress_bar_ = AddChildView(std::make_unique<views::ProgressBar>(5, false));
progress_bar_->SetBorder(views::CreateEmptyBorder(kProgressBarInsets));
progress_bar_ = AddChildView(
std::make_unique<views::ProgressBar>(kProgressBarHeight, false));
// Font list for text views.
gfx::Font default_font;
......@@ -127,28 +131,22 @@ void MediaControlsProgressView::UpdateProgress(
}
bool MediaControlsProgressView::OnMousePressed(const ui::MouseEvent& event) {
gfx::Point location_in_bar(event.location());
ConvertPointToTarget(this, this->progress_bar_, &location_in_bar);
if (!event.IsOnlyLeftMouseButton() ||
!progress_bar_->GetLocalBounds().Contains(location_in_bar)) {
if (!event.IsOnlyLeftMouseButton() || event.y() < kMinClickHeight ||
event.y() > kMaxClickHeight) {
return false;
}
HandleSeeking(location_in_bar);
HandleSeeking(event.location());
return true;
}
void MediaControlsProgressView::OnGestureEvent(ui::GestureEvent* event) {
gfx::Point location_in_bar(event->location());
ConvertPointToTarget(this, this->progress_bar_, &location_in_bar);
if (event->type() != ui::ET_GESTURE_TAP ||
!progress_bar_->GetLocalBounds().Contains(location_in_bar)) {
if (event->type() != ui::ET_GESTURE_TAP || event->y() < kMinClickHeight ||
event->y() > kMaxClickHeight) {
return;
}
HandleSeeking(location_in_bar);
HandleSeeking(event->location());
event->SetHandled();
}
......@@ -178,8 +176,10 @@ void MediaControlsProgressView::SetDuration(const base::string16& duration) {
duration_->SetText(duration);
}
void MediaControlsProgressView::HandleSeeking(
const gfx::Point& location_in_bar) {
void MediaControlsProgressView::HandleSeeking(const gfx::Point& location) {
gfx::Point location_in_bar(location);
ConvertPointToTarget(this, progress_bar_, &location_in_bar);
double seek_to_progress =
static_cast<double>(location_in_bar.x()) / progress_bar_->width();
seek_callback_.Run(seek_to_progress);
......
......@@ -38,7 +38,7 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaControlsProgressView
void SetProgressTime(const base::string16& time);
void SetDuration(const base::string16& duration);
void HandleSeeking(const gfx::Point& location_in_bar);
void HandleSeeking(const gfx::Point& location);
views::ProgressBar* progress_bar_;
views::Label* progress_time_;
......
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