Commit aff95c50 authored by Eric Willigers's avatar Eric Willigers Committed by Commit Bot

Desktop PWAs: Move minimal-ui buttons closer together

The spacing around the Back and Reload buttons now matches
mocks provided by UI team.

When a profile indicator icon is present, we move the
Back and Reload icons to the right of the profile indicator.

Logic controlling the Back and Reload buttons is moved into
a new nested class WebAppFrameToolbarView::NavigationButtonContainer.

Bug: 1016663, 1028789
Change-Id: Ie404aa64a191228161e4590e430be2dfff4a5381
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1909057
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719865}
parent 73ca8856
......@@ -387,8 +387,9 @@ void BrowserNonClientFrameViewAsh::Layout() {
if (profile_indicator_icon_)
LayoutProfileIndicator();
if (web_app_frame_toolbar()) {
web_app_frame_toolbar()->LayoutInContainer(
0, caption_button_container_->x(), 0, painted_height);
web_app_frame_toolbar()->LayoutInContainer(GetToolbarLeftInset(),
caption_button_container_->x(),
0, painted_height);
}
BrowserNonClientFrameView::Layout();
......@@ -642,7 +643,16 @@ bool BrowserNonClientFrameViewAsh::ShouldShowCaptionButtons() const {
return !IsInOverviewMode();
}
int BrowserNonClientFrameViewAsh::GetToolbarLeftInset() const {
// Include padding on left and right of icon.
return profile_indicator_icon_
? kProfileIndicatorPadding * 2 + profile_indicator_icon_->width()
: 0;
}
int BrowserNonClientFrameViewAsh::GetTabStripLeftInset() const {
// Include padding on left of icon.
// The tab strip has its own 'padding' to the right of the icon.
return profile_indicator_icon_
? kProfileIndicatorPadding + profile_indicator_icon_->width()
: 0;
......
......@@ -157,6 +157,10 @@ class BrowserNonClientFrameViewAsh
// example, in overview mode and tablet mode.
bool ShouldShowCaptionButtons() const;
// Distance between the edge of the NonClientFrameView and the web app frame
// toolbar.
int GetToolbarLeftInset() const;
// Distance between the edges of the NonClientFrameView and the tab strip.
int GetTabStripLeftInset() const;
int GetTabStripRightInset() const;
......
......@@ -34,7 +34,6 @@
namespace {
constexpr int kHostedAppMenuMargin = 7;
constexpr int kFramePaddingLeft = 75;
constexpr double kTitlePaddingWidthFraction = 0.1;
......@@ -74,8 +73,7 @@ BrowserNonClientFrameViewMac::BrowserNonClientFrameViewMac(
AddChildView(std::make_unique<WebAppFrameToolbarView>(
frame, browser_view,
GetCaptionColor(BrowserFrameActiveState::kActive),
GetCaptionColor(BrowserFrameActiveState::kInactive),
kHostedAppMenuMargin, kHostedAppMenuMargin)));
GetCaptionColor(BrowserFrameActiveState::kInactive))));
}
DCHECK(browser_view->ShouldShowWindowTitle());
......@@ -137,7 +135,7 @@ int BrowserNonClientFrameViewMac::GetTopInset(bool restored) const {
if (ShouldHideTopUIForFullscreen())
return 0;
return web_app_frame_toolbar()->GetPreferredSize().height() +
kHostedAppMenuMargin * 2;
kWebAppMenuMargin * 2;
}
if (!browser_view()->IsTabStripVisible())
......
......@@ -12,6 +12,7 @@
#include "build/build_config.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/app/vector_icons/vector_icons.h"
#include "chrome/browser/command_observer.h"
#include "chrome/browser/ui/browser_command_controller.h"
#include "chrome/browser/ui/browser_commands.h"
#include "chrome/browser/ui/browser_content_setting_bubble_model_delegate.h"
......@@ -38,6 +39,7 @@
#include "third_party/blink/public/common/features.h"
#include "ui/base/hit_test.h"
#include "ui/base/material_design/material_design_controller.h"
#include "ui/base/material_design/material_design_controller_observer.h"
#include "ui/base/window_open_disposition.h"
#include "ui/compositor/layer_animation_element.h"
#include "ui/compositor/layer_animation_sequence.h"
......@@ -72,6 +74,14 @@ bool g_animation_disabled_for_testing = false;
constexpr base::TimeDelta kContentSettingsFadeInDuration =
base::TimeDelta::FromMilliseconds(500);
constexpr int kPaddingBetweenNavigationButtons = 9;
#if defined(OS_CHROMEOS)
constexpr int kWebAppFrameLeftMargin = 4;
#else
constexpr int kWebAppFrameLeftMargin = 9;
#endif
class WebAppToolbarActionsBar : public ToolbarActionsBar {
public:
using ToolbarActionsBar::ToolbarActionsBar;
......@@ -97,11 +107,19 @@ class WebAppToolbarActionsBar : public ToolbarActionsBar {
DISALLOW_COPY_AND_ASSIGN(WebAppToolbarActionsBar);
};
int HorizontalPaddingBetweenItems() {
int HorizontalPaddingBetweenPageActionsAndAppMenuButtons() {
return views::LayoutProvider::Get()->GetDistanceMetric(
views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
}
int WebAppFrameRightMargin() {
#if defined(OS_MACOSX)
return kWebAppMenuMargin;
#else
return HorizontalPaddingBetweenPageActionsAndAppMenuButtons();
#endif
}
// An ink drop with round corners in shown when the user hovers over the button.
// Insets are kept small to avoid increasing web app frame toolbar height.
void SetInsetsForWebAppToolbarButton(ToolbarButton* toolbar_button,
......@@ -125,16 +143,125 @@ const gfx::VectorIcon& GetBackImage(bool touch_ui) {
} // namespace
// Holds controls in the far left of the toolbar.
class WebAppFrameToolbarView::NavigationButtonContainer
: public views::View,
public CommandObserver,
public views::ButtonListener,
public ui::MaterialDesignControllerObserver {
public:
explicit NavigationButtonContainer(BrowserView* browser_view)
: browser_view_(browser_view) {
views::BoxLayout& layout =
*SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kHorizontal,
gfx::Insets(0, kWebAppFrameLeftMargin),
kPaddingBetweenNavigationButtons));
// Right align to clip the leftmost items first when not enough space.
layout.set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd);
layout.set_cross_axis_alignment(
views::BoxLayout::CrossAxisAlignment::kCenter);
back_button_ =
AddChildView(CreateBackButton(this, browser_view_->browser()));
reload_button_ = AddChildView(CreateReloadButton(
browser_view_->browser(), ReloadButton::IconStyle::kMinimalUi));
const bool is_browser_focus_mode =
browser_view_->browser()->is_focus_mode();
SetInsetsForWebAppToolbarButton(back_button_, is_browser_focus_mode);
SetInsetsForWebAppToolbarButton(reload_button_, is_browser_focus_mode);
views::SetHitTestComponent(back_button_, static_cast<int>(HTCLIENT));
views::SetHitTestComponent(reload_button_, static_cast<int>(HTCLIENT));
chrome::AddCommandObserver(browser_view_->browser(), IDC_BACK, this);
chrome::AddCommandObserver(browser_view_->browser(), IDC_RELOAD, this);
md_observer_.Add(ui::MaterialDesignController::GetInstance());
}
~NavigationButtonContainer() override {
chrome::RemoveCommandObserver(browser_view_->browser(), IDC_BACK, this);
chrome::RemoveCommandObserver(browser_view_->browser(), IDC_RELOAD, this);
}
ToolbarButton* back_button() { return back_button_; }
ReloadButton* reload_button() { return reload_button_; }
void SetIconColor(SkColor icon_color) {
icon_color_ = icon_color;
GenerateMinimalUIButtonImages();
}
void GenerateMinimalUIButtonImages() {
const SkColor disabled_color =
SkColorSetA(icon_color_, gfx::kDisabledControlAlpha);
const bool touch_ui = ui::MaterialDesignController::touch_ui();
const gfx::VectorIcon& back_image = GetBackImage(touch_ui);
back_button_->SetImage(views::Button::STATE_NORMAL,
gfx::CreateVectorIcon(back_image, icon_color_));
back_button_->SetImage(views::Button::STATE_DISABLED,
gfx::CreateVectorIcon(back_image, disabled_color));
reload_button_->SetColors(icon_color_, disabled_color);
}
protected:
// CommandObserver:
void EnabledStateChangedForCommand(int id, bool enabled) override {
switch (id) {
case IDC_BACK:
back_button_->SetEnabled(enabled);
break;
case IDC_RELOAD:
reload_button_->SetEnabled(enabled);
break;
default:
NOTREACHED();
}
}
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override {
chrome::ExecuteCommandWithDisposition(
browser_view_->browser(), sender->tag(),
ui::DispositionFromEventFlags(event.flags()));
}
// ui::MaterialDesignControllerObserver:
void OnTouchUiChanged() override {
GenerateMinimalUIButtonImages();
SchedulePaint();
}
private:
// The containing browser view.
BrowserView* const browser_view_;
SkColor icon_color_ = gfx::kPlaceholderColor;
ScopedObserver<ui::MaterialDesignController,
ui::MaterialDesignControllerObserver>
md_observer_{this};
// These members are owned by the views hierarchy.
ToolbarButton* back_button_ = nullptr;
ReloadButton* reload_button_ = nullptr;
};
// Holds controls in the far left or far right of the toolbar.
// Forces a layout of the toolbar (and hence the window text) whenever a control
// changes visibility.
class WebAppFrameToolbarView::ToolbarButtonContainer : public views::View {
public:
explicit ToolbarButtonContainer(const gfx::Insets& inside_border_insets) {
ToolbarButtonContainer() {
views::BoxLayout& layout =
*SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kHorizontal, inside_border_insets,
HorizontalPaddingBetweenItems()));
views::BoxLayout::Orientation::kHorizontal,
gfx::Insets(0, WebAppFrameRightMargin()),
HorizontalPaddingBetweenPageActionsAndAppMenuButtons()));
// Right align to clip the leftmost items first when not enough space.
layout.set_main_axis_alignment(views::BoxLayout::MainAxisAlignment::kEnd);
layout.set_cross_axis_alignment(
......@@ -270,9 +397,7 @@ WebAppFrameToolbarView::ContentSettingsContainer::ContentSettingsContainer(
WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
BrowserView* browser_view,
SkColor active_color,
SkColor inactive_color,
base::Optional<int> left_margin,
base::Optional<int> right_margin)
SkColor inactive_color)
: browser_view_(browser_view),
active_color_(active_color),
inactive_color_(inactive_color) {
......@@ -291,33 +416,17 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
}
const auto* app_controller = browser_view_->browser()->app_controller();
const bool is_browser_focus_mode = browser_view_->browser()->is_focus_mode();
if (base::FeatureList::IsEnabled(features::kDesktopMinimalUI) &&
app_controller->HasMinimalUiButtons()) {
left_container_ = AddChildView(std::make_unique<ToolbarButtonContainer>(
gfx::Insets(0, left_margin.value_or(HorizontalPaddingBetweenItems()))));
left_container_ = AddChildView(
std::make_unique<NavigationButtonContainer>(browser_view_));
left_container_->SetProperty(
views::kFlexBehaviorKey,
views::FlexSpecification::ForSizeRule(
views::MinimumFlexSizeRule::kScaleToMinimumSnapToZero,
views::MaximumFlexSizeRule::kPreferred)
.WithOrder(2));
back_ = left_container_->AddChildView(
CreateBackButton(this, browser_view_->browser()));
reload_ = left_container_->AddChildView(CreateReloadButton(
browser_view_->browser(), ReloadButton::IconStyle::kMinimalUi));
SetInsetsForWebAppToolbarButton(back_, is_browser_focus_mode);
SetInsetsForWebAppToolbarButton(reload_, is_browser_focus_mode);
views::SetHitTestComponent(back_, static_cast<int>(HTCLIENT));
views::SetHitTestComponent(reload_, static_cast<int>(HTCLIENT));
chrome::AddCommandObserver(browser_view_->browser(), IDC_BACK, this);
chrome::AddCommandObserver(browser_view_->browser(), IDC_RELOAD, this);
md_observer_.Add(ui::MaterialDesignController::GetInstance());
}
center_container_ = AddChildView(std::make_unique<views::View>());
......@@ -327,8 +436,7 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
views::MaximumFlexSizeRule::kUnbounded)
.WithOrder(3));
right_container_ = AddChildView(std::make_unique<ToolbarButtonContainer>(
gfx::Insets(0, right_margin.value_or(HorizontalPaddingBetweenItems()))));
right_container_ = AddChildView(std::make_unique<ToolbarButtonContainer>());
right_container_->SetProperty(views::kFlexBehaviorKey,
views::FlexSpecification::ForSizeRule(
views::MinimumFlexSizeRule::kScaleToZero,
......@@ -359,7 +467,8 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
params.types_enabled.push_back(PageActionIconType::kSaveCard);
params.icon_size = GetLayoutConstant(WEB_APP_PAGE_ACTION_ICON_SIZE);
params.icon_color = GetCaptionColor();
params.between_icon_spacing = HorizontalPaddingBetweenItems();
params.between_icon_spacing =
HorizontalPaddingBetweenPageActionsAndAppMenuButtons();
params.browser = browser_view_->browser();
params.command_updater = browser_view_->browser()->command_controller();
params.page_action_icon_delegate = this;
......@@ -394,6 +503,7 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
web_app_menu_button_ = right_container_->AddChildView(
std::make_unique<WebAppMenuButton>(browser_view_));
#endif
const bool is_browser_focus_mode = browser_view_->browser()->is_focus_mode();
SetInsetsForWebAppToolbarButton(web_app_menu_button_, is_browser_focus_mode);
right_container_->SetChildControllingHeight(web_app_menu_button_);
......@@ -409,15 +519,9 @@ WebAppFrameToolbarView::WebAppFrameToolbarView(views::Widget* widget,
browser_view_->SetToolbarButtonProvider(this);
browser_view_->immersive_mode_controller()->AddObserver(this);
scoped_widget_observer_.Add(widget);
GenerateMinimalUIButtonImages();
}
WebAppFrameToolbarView::~WebAppFrameToolbarView() {
if (back_)
chrome::RemoveCommandObserver(browser_view_->browser(), IDC_BACK, this);
if (reload_)
chrome::RemoveCommandObserver(browser_view_->browser(), IDC_RELOAD, this);
ImmersiveModeController* immersive_controller =
browser_view_->immersive_mode_controller();
if (immersive_controller)
......@@ -437,7 +541,6 @@ void WebAppFrameToolbarView::UpdateCaptionColors() {
inactive_color_ =
frame_view->GetCaptionColor(BrowserFrameActiveState::kInactive);
UpdateChildrenColor();
GenerateMinimalUIButtonImages();
}
void WebAppFrameToolbarView::SetPaintAsActive(bool active) {
......@@ -445,7 +548,6 @@ void WebAppFrameToolbarView::SetPaintAsActive(bool active) {
return;
paint_as_active_ = active;
UpdateChildrenColor();
GenerateMinimalUIButtonImages();
}
std::pair<int, int> WebAppFrameToolbarView::LayoutInContainer(
......@@ -470,7 +572,7 @@ std::pair<int, int> WebAppFrameToolbarView::LayoutInContainer(
// Bounds for remaining inner space, in parent container coordinates.
gfx::Rect center_bounds = center_container_->bounds();
DCHECK(center_bounds.x() == 0 || back_ || reload_);
DCHECK(center_bounds.x() == 0 || left_container_);
center_bounds.Offset(bounds().OffsetFromOrigin());
return std::pair<int, int>(center_bounds.x(), center_bounds.right());
......@@ -502,26 +604,6 @@ WebAppFrameToolbarView::CreateToolbarActionsBar(
return std::make_unique<WebAppToolbarActionsBar>(delegate, browser, main_bar);
}
void WebAppFrameToolbarView::EnabledStateChangedForCommand(int id,
bool enabled) {
switch (id) {
case IDC_BACK:
back_->SetEnabled(enabled);
break;
case IDC_RELOAD:
reload_->SetEnabled(enabled);
break;
default:
NOTREACHED();
}
}
void WebAppFrameToolbarView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
chrome::ExecuteCommandWithDisposition(
browser_view_->browser(), sender->tag(),
ui::DispositionFromEventFlags(event.flags()));
}
SkColor WebAppFrameToolbarView::GetContentSettingInkDropColor() const {
return GetCaptionColor();
......@@ -627,11 +709,11 @@ AvatarToolbarButton* WebAppFrameToolbarView::GetAvatarToolbarButton() {
}
ToolbarButton* WebAppFrameToolbarView::GetBackButton() {
return back_;
return left_container_ ? left_container_->back_button() : nullptr;
}
ReloadButton* WebAppFrameToolbarView::GetReloadButton() {
return reload_;
return left_container_ ? left_container_->reload_button() : nullptr;
}
void WebAppFrameToolbarView::OnWidgetVisibilityChanged(views::Widget* widget,
......@@ -648,11 +730,6 @@ void WebAppFrameToolbarView::OnWidgetVisibilityChanged(views::Widget* widget,
}
}
void WebAppFrameToolbarView::OnTouchUiChanged() {
GenerateMinimalUIButtonImages();
SchedulePaint();
}
void WebAppFrameToolbarView::DisableAnimationForTesting() {
g_animation_disabled_for_testing = true;
}
......@@ -705,6 +782,8 @@ SkColor WebAppFrameToolbarView::GetCaptionColor() const {
void WebAppFrameToolbarView::UpdateChildrenColor() {
SkColor icon_color = GetCaptionColor();
if (left_container_)
left_container_->SetIconColor(icon_color);
if (web_app_origin_text_)
web_app_origin_text_->SetTextColor(icon_color);
if (content_settings_container_)
......@@ -714,21 +793,3 @@ void WebAppFrameToolbarView::UpdateChildrenColor() {
page_action_icon_container_view_->SetIconColor(icon_color);
web_app_menu_button_->SetColor(icon_color);
}
void WebAppFrameToolbarView::GenerateMinimalUIButtonImages() {
const SkColor normal_color = GetCaptionColor();
const SkColor disabled_color =
SkColorSetA(normal_color, gfx::kDisabledControlAlpha);
if (back_) {
const bool touch_ui = ui::MaterialDesignController::touch_ui();
const gfx::VectorIcon& back_image = GetBackImage(touch_ui);
back_->SetImage(views::Button::STATE_NORMAL,
gfx::CreateVectorIcon(back_image, normal_color));
back_->SetImage(views::Button::STATE_DISABLED,
gfx::CreateVectorIcon(back_image, disabled_color));
}
if (reload_)
reload_->SetColors(normal_color, disabled_color);
}
......@@ -12,14 +12,12 @@
#include "base/scoped_observer.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "chrome/browser/command_observer.h"
#include "build/build_config.h"
#include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
#include "chrome/browser/ui/views/frame/toolbar_button_provider.h"
#include "chrome/browser/ui/views/location_bar/content_setting_image_view.h"
#include "chrome/browser/ui/views/page_action/page_action_icon_view.h"
#include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
#include "ui/base/material_design/material_design_controller.h"
#include "ui/base/material_design/material_design_controller_observer.h"
#include "ui/gfx/color_palette.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/accessible_pane_view.h"
......@@ -39,17 +37,18 @@ class ToolbarButton;
class WebAppMenuButton;
class WebAppOriginText;
#if defined(OS_MACOSX)
constexpr int kWebAppMenuMargin = 7;
#endif
// A container for web app buttons in the title bar.
class WebAppFrameToolbarView : public views::AccessiblePaneView,
public BrowserActionsContainer::Delegate,
public CommandObserver,
public views::ButtonListener,
public ContentSettingImageView::Delegate,
public ImmersiveModeController::Observer,
public PageActionIconView::Delegate,
public ToolbarButtonProvider,
public views::WidgetObserver,
public ui::MaterialDesignControllerObserver {
public views::WidgetObserver {
public:
static const char kViewClassName[];
......@@ -71,9 +70,7 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
WebAppFrameToolbarView(views::Widget* widget,
BrowserView* browser_view,
SkColor active_color,
SkColor inactive_color,
base::Optional<int> left_margin = base::nullopt,
base::Optional<int> right_margin = base::nullopt);
SkColor inactive_color);
~WebAppFrameToolbarView() override;
void UpdateStatusIconsVisibility();
......@@ -104,12 +101,6 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
Browser* browser,
ToolbarActionsBar* main_bar) const override;
// CommandObserver:
void EnabledStateChangedForCommand(int id, bool enabled) override;
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
// ContentSettingImageView::Delegate:
SkColor GetContentSettingInkDropColor() const override;
content::WebContents* GetContentSettingWebContents() override;
......@@ -143,9 +134,6 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
// views::WidgetObserver:
void OnWidgetVisibilityChanged(views::Widget* widget, bool visible) override;
// ui::MaterialDesignControllerObserver:
void OnTouchUiChanged() override;
static void DisableAnimationForTesting();
views::View* GetRightContainerForTesting();
views::View* GetPageActionIconContainerForTesting();
......@@ -179,18 +167,12 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
SkColor GetCaptionColor() const;
void UpdateChildrenColor();
void GenerateMinimalUIButtonImages();
// Whether we're waiting for the widget to become visible.
bool pending_widget_visibility_ = true;
ScopedObserver<views::Widget, views::WidgetObserver> scoped_widget_observer_{
this};
ScopedObserver<ui::MaterialDesignController,
ui::MaterialDesignControllerObserver>
md_observer_{this};
// Timers for synchronising their respective parts of the titlebar animation.
base::OneShotTimer animation_start_delay_;
base::OneShotTimer icon_fade_in_delay_;
......@@ -203,14 +185,13 @@ class WebAppFrameToolbarView : public views::AccessiblePaneView,
SkColor active_color_;
SkColor inactive_color_;
class NavigationButtonContainer;
class ToolbarButtonContainer;
// All remaining members are owned by the views hierarchy.
// These three fields are only created when the display mode is minimal-ui.
ToolbarButtonContainer* left_container_ = nullptr;
ToolbarButton* back_ = nullptr;
ReloadButton* reload_ = nullptr;
// The navigation container is only created when display mode is minimal-ui.
NavigationButtonContainer* left_container_ = nullptr;
// Empty container used by the parent frame to layout additional elements.
views::View* center_container_ = nullptr;
......
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