Commit c3f59ad4 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Make Chrome conform with other Windows apps in tablet mode.

Chrome now has a visible caption bar in tablet mode above the toolbar/
tabstrip that behaves like most other Windows apps (explorer, console,
etc.)

When maximized in non-split-screen mode:
- Caption bar is thin.
- Restore caption button is present but disabled (greyed out, won't
  highlight). Note that this is an improvement on Windows apps which
  display a non-disabled button that doesn't do anything.
- User can pull down on the caption bar to move the window and go into
  split-screen mode.

When in split-screen mode:
- Caption bar and buttons are default size.
- Maximize caption button is present but disabled (see note above).
- User can pull down on the caption bar to move the window in and out
  of split-screen mode, etc.

This change also makes it much easier for users to generate touch
gestures that clearly show intent to either move the window or open the
tabstrip shade, by dragging on the caption bar vs. the toolbar.

The general accuracy of drag/swipe gestures on the toolbar seems to be
very much improved by this change.

Known issue: when in split-screen Windows sometimes (but not always)
cuts off the top few pixels of a window. This happens to native/first-
party Windows apps the same as Chrome, so is not a Chrome bug, and
because it is intermittent we can't compensate for it except to use a
taller caption bar in this mode (which is also consistent with other
Windows first-party applications).

Note: the ability of the caption button container to be torn out and
moved to the tabstrip container, which was previously required in order
for us to hide the caption buttons and show them when the tabstrip tray
was opened, will be removed in a follow-up CL.

Bug: 1116651
Change-Id: I09ca9381f2add1198a680d9ef6f45cedd7916bf4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358249Reviewed-by: default avatarCollin Baker <collinbaker@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798874}
parent c200fb92
...@@ -2933,35 +2933,8 @@ void BrowserView::MaybeInitializeWebUITabStrip() { ...@@ -2933,35 +2933,8 @@ void BrowserView::MaybeInitializeWebUITabStrip() {
loading_bar_ = top_container_->AddChildView( loading_bar_ = top_container_->AddChildView(
std::make_unique<TopContainerLoadingBar>(browser_.get())); std::make_unique<TopContainerLoadingBar>(browser_.get()));
loading_bar_->SetWebContents(GetActiveWebContents()); loading_bar_->SetWebContents(GetActiveWebContents());
// If possible, borrow the caption buttons from the frame to put on the
// tab strip (so that they are visible only when the tab strip is open).
// If the platform does not support this, GetCaptionButtonContainer() will
// return null.
auto* const frame_view = frame()->GetFrameView();
auto* const caption_button_container =
frame_view->GetCaptionButtonContainer();
if (caption_button_container) {
caption_button_container->set_paint_frame_background(true);
webui_tab_strip_->AddChildViewAt(
frame_view->RemoveChildViewT(caption_button_container), 0);
}
} }
} else if (webui_tab_strip_) { } else if (webui_tab_strip_) {
// If we borrowed the caption buttons from the frame to put on the tab strip
// (see above) then we need to return them to the frame *before* we dispose
// the tab strip else the buttons will be destroyed.
// If GetCaptionButtonContainer() returns null, we were not able to borrow
// the caption buttons in the first place.
auto* const frame_view = frame()->GetFrameView();
auto* const caption_button_container =
frame_view->GetCaptionButtonContainer();
if (caption_button_container) {
caption_button_container->set_paint_frame_background(false);
frame_view->AddChildView(
webui_tab_strip_->RemoveChildViewT(caption_button_container));
}
top_container_->RemoveChildView(webui_tab_strip_); top_container_->RemoveChildView(webui_tab_strip_);
delete webui_tab_strip_; delete webui_tab_strip_;
webui_tab_strip_ = nullptr; webui_tab_strip_ = nullptr;
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/strings/grit/ui_strings.h" #include "ui/strings/grit/ui_strings.h"
#include "ui/views/layout/flex_layout.h" #include "ui/views/layout/flex_layout.h"
#include "ui/views/view_class_properties.h"
namespace { namespace {
...@@ -51,9 +52,16 @@ GlassBrowserCaptionButtonContainer::GlassBrowserCaptionButtonContainer( ...@@ -51,9 +52,16 @@ GlassBrowserCaptionButtonContainer::GlassBrowserCaptionButtonContainer(
// Layout is horizontal, with buttons placed at the trailing end of the view. // Layout is horizontal, with buttons placed at the trailing end of the view.
// This allows the container to expand to become a faux titlebar/drag handle. // This allows the container to expand to become a faux titlebar/drag handle.
auto* const layout = SetLayoutManager(std::make_unique<views::FlexLayout>()); auto* const layout = SetLayoutManager(std::make_unique<views::FlexLayout>());
layout->SetOrientation(views::LayoutOrientation::kHorizontal); layout->SetOrientation(views::LayoutOrientation::kHorizontal)
layout->SetMainAxisAlignment(views::LayoutAlignment::kEnd); .SetMainAxisAlignment(views::LayoutAlignment::kEnd)
layout->SetCrossAxisAlignment(views::LayoutAlignment::kStart); .SetCrossAxisAlignment(views::LayoutAlignment::kStart)
.SetDefault(
views::kFlexBehaviorKey,
views::FlexSpecification(views::LayoutOrientation::kHorizontal,
views::MinimumFlexSizeRule::kPreferred,
views::MaximumFlexSizeRule::kPreferred,
/* adjust_width_for_height */ false,
views::MinimumFlexSizeRule::kScaleToZero));
} }
GlassBrowserCaptionButtonContainer::~GlassBrowserCaptionButtonContainer() {} GlassBrowserCaptionButtonContainer::~GlassBrowserCaptionButtonContainer() {}
...@@ -97,25 +105,24 @@ void GlassBrowserCaptionButtonContainer::AddedToWidget() { ...@@ -97,25 +105,24 @@ void GlassBrowserCaptionButtonContainer::AddedToWidget() {
views::Widget* const widget = GetWidget(); views::Widget* const widget = GetWidget();
if (!widget_observer_.IsObserving(widget)) if (!widget_observer_.IsObserving(widget))
widget_observer_.Add(widget); widget_observer_.Add(widget);
UpdateButtonVisibility(); UpdateButtons();
} }
void GlassBrowserCaptionButtonContainer::OnWidgetBoundsChanged( void GlassBrowserCaptionButtonContainer::OnWidgetBoundsChanged(
views::Widget* widget, views::Widget* widget,
const gfx::Rect& new_bounds) { const gfx::Rect& new_bounds) {
UpdateButtonVisibility(); UpdateButtons();
} }
void GlassBrowserCaptionButtonContainer::UpdateButtonVisibility() { void GlassBrowserCaptionButtonContainer::UpdateButtons() {
if (frame_view_->IsWebUITabStrip()) { const bool is_maximized = frame_view_->IsMaximized();
// In tablet mode, all windows are effectively full-screen and cannot be restore_button_->SetVisible(is_maximized);
// restored, so don't show either button. maximize_button_->SetVisible(!is_maximized);
restore_button_->SetVisible(false);
maximize_button_->SetVisible(false); // In touch mode, windows cannot be taken out of fullscreen or tiled mode, so
} else { // the maximize/restore button should be disabled.
const bool is_maximized = frame_view_->IsMaximized(); const bool is_touch = ui::TouchUiController::Get()->touch_ui();
restore_button_->SetVisible(is_maximized); restore_button_->SetEnabled(!is_touch);
maximize_button_->SetVisible(!is_maximized); maximize_button_->SetEnabled(!is_touch);
}
InvalidateLayout(); InvalidateLayout();
} }
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/scoped_observer.h" #include "base/scoped_observer.h"
#include "chrome/browser/ui/views/frame/caption_button_container.h" #include "chrome/browser/ui/views/frame/caption_button_container.h"
#include "ui/base/pointer/touch_ui_controller.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
#include "ui/views/view.h" #include "ui/views/view.h"
#include "ui/views/widget/widget.h" #include "ui/views/widget/widget.h"
...@@ -41,9 +42,10 @@ class GlassBrowserCaptionButtonContainer : public CaptionButtonContainer, ...@@ -41,9 +42,10 @@ class GlassBrowserCaptionButtonContainer : public CaptionButtonContainer,
void ResetWindowControls(); void ResetWindowControls();
void ButtonPressed(views::Button* sender); void ButtonPressed(views::Button* sender);
// Sets caption button visibility based on window state. Only one of maximize // Sets caption button visibility and enabled state based on window state.
// or restore button should ever be visible at the same time. // Only one of maximize or restore button should ever be visible at the same
void UpdateButtonVisibility(); // time, and both are disabled in tablet UI mode.
void UpdateButtons();
GlassBrowserFrameView* const frame_view_; GlassBrowserFrameView* const frame_view_;
Windows10CaptionButton* const minimize_button_; Windows10CaptionButton* const minimize_button_;
...@@ -52,6 +54,11 @@ class GlassBrowserCaptionButtonContainer : public CaptionButtonContainer, ...@@ -52,6 +54,11 @@ class GlassBrowserCaptionButtonContainer : public CaptionButtonContainer,
Windows10CaptionButton* const close_button_; Windows10CaptionButton* const close_button_;
ScopedObserver<views::Widget, views::WidgetObserver> widget_observer_{this}; ScopedObserver<views::Widget, views::WidgetObserver> widget_observer_{this};
std::unique_ptr<ui::TouchUiController::Subscription> subscription_ =
ui::TouchUiController::Get()->RegisterCallback(base::BindRepeating(
&GlassBrowserCaptionButtonContainer::UpdateButtons,
base::Unretained(this)));
}; };
#endif // CHROME_BROWSER_UI_VIEWS_FRAME_GLASS_BROWSER_CAPTION_BUTTON_CONTAINER_H_ #endif // CHROME_BROWSER_UI_VIEWS_FRAME_GLASS_BROWSER_CAPTION_BUTTON_CONTAINER_H_
...@@ -407,7 +407,7 @@ void GlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) { ...@@ -407,7 +407,7 @@ void GlassBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
void GlassBrowserFrameView::Layout() { void GlassBrowserFrameView::Layout() {
TRACE_EVENT0("views.frame", "GlassBrowserFrameView::Layout"); TRACE_EVENT0("views.frame", "GlassBrowserFrameView::Layout");
if (ShouldCustomDrawSystemTitlebar()) if (ShouldCustomDrawSystemTitlebar() || IsWebUITabStrip())
LayoutCaptionButtons(); LayoutCaptionButtons();
if (ShouldCustomDrawSystemTitlebar()) if (ShouldCustomDrawSystemTitlebar())
...@@ -426,8 +426,6 @@ int GlassBrowserFrameView::FrameBorderThickness() const { ...@@ -426,8 +426,6 @@ int GlassBrowserFrameView::FrameBorderThickness() const {
} }
int GlassBrowserFrameView::FrameTopBorderThickness(bool restored) const { int GlassBrowserFrameView::FrameTopBorderThickness(bool restored) const {
constexpr int kRestoredWebUITopBorder = 1;
const bool is_fullscreen = const bool is_fullscreen =
(frame()->IsFullscreen() || IsMaximized()) && !restored; (frame()->IsFullscreen() || IsMaximized()) && !restored;
if (!is_fullscreen) { if (!is_fullscreen) {
...@@ -437,8 +435,15 @@ int GlassBrowserFrameView::FrameTopBorderThickness(bool restored) const { ...@@ -437,8 +435,15 @@ int GlassBrowserFrameView::FrameTopBorderThickness(bool restored) const {
// value. // value.
if (browser_view()->IsTabStripVisible()) if (browser_view()->IsTabStripVisible())
return drag_handle_padding_; return drag_handle_padding_;
// There is no top border in tablet mode when the window is "restored"
// because it is still tiled into either the left or right pane of the
// display takes up the entire vertical extent of the screen. Note that a
// rendering bug in Windows may still cause the very top of the window to be
// cut off intermittently, but that's an OS issue that affects all
// applications, not specifically Chrome.
if (IsWebUITabStrip()) if (IsWebUITabStrip())
return kRestoredWebUITopBorder; return 0;
} }
// Mouse and touch locations are floored but GetSystemMetricsInDIP is rounded, // Mouse and touch locations are floored but GetSystemMetricsInDIP is rounded,
...@@ -472,10 +477,20 @@ int GlassBrowserFrameView::TopAreaHeight(bool restored) const { ...@@ -472,10 +477,20 @@ int GlassBrowserFrameView::TopAreaHeight(bool restored) const {
if (frame()->IsFullscreen() && !restored) if (frame()->IsFullscreen() && !restored)
return 0; return 0;
// Return only the top border thickness (no extra drag handle) if maximized or const bool maximized = IsMaximized() && !restored;
// in WebUI tab strip (tablet) mode.
int top = FrameTopBorderThickness(restored); int top = FrameTopBorderThickness(restored);
if ((IsMaximized() && !restored) || IsWebUITabStrip()) if (IsWebUITabStrip()) {
// Caption bar is default Windows size in maximized mode but full size when
// windows are tiled in tablet mode (baesd on behavior of first-party
// Windows applications).
top += maximized ? TitlebarMaximizedVisualHeight()
: caption_button_container_->GetPreferredSize().height();
return top;
}
// In maximized mode, we do not add any additional thickness to the grab
// handle above the tabs; just return the frame thickness.
if (maximized)
return top; return top;
// Besides the frame border, there's empty space atop the window in restored // Besides the frame border, there's empty space atop the window in restored
...@@ -506,10 +521,14 @@ int GlassBrowserFrameView::TitlebarMaximizedVisualHeight() const { ...@@ -506,10 +521,14 @@ int GlassBrowserFrameView::TitlebarMaximizedVisualHeight() const {
int GlassBrowserFrameView::TitlebarHeight(bool restored) const { int GlassBrowserFrameView::TitlebarHeight(bool restored) const {
if (frame()->IsFullscreen() && !restored) if (frame()->IsFullscreen() && !restored)
return 0; return 0;
// The titlebar's actual height is the same in restored and maximized, but // The titlebar's actual height is the same in restored and maximized, but
// some of it is above the screen in maximized mode. See the comment in // some of it is above the screen in maximized mode. See the comment in
// FrameTopBorderThicknessPx(). // FrameTopBorderThicknessPx(). For WebUI,
return TitlebarMaximizedVisualHeight() + FrameTopBorderThickness(false); return (IsWebUITabStrip()
? caption_button_container_->GetPreferredSize().height()
: TitlebarMaximizedVisualHeight()) +
FrameTopBorderThickness(false);
} }
int GlassBrowserFrameView::WindowTopY() const { int GlassBrowserFrameView::WindowTopY() const {
...@@ -517,7 +536,9 @@ int GlassBrowserFrameView::WindowTopY() const { ...@@ -517,7 +536,9 @@ int GlassBrowserFrameView::WindowTopY() const {
// FrameTopBorderThickness()) and floor(system dsf) pixels when restored. // FrameTopBorderThickness()) and floor(system dsf) pixels when restored.
// Unfortunately we can't represent either of those at hidpi without using // Unfortunately we can't represent either of those at hidpi without using
// non-integral dips, so we return the closest reasonable values instead. // non-integral dips, so we return the closest reasonable values instead.
return IsMaximized() ? FrameTopBorderThickness(false) : 1; if (IsMaximized())
return FrameTopBorderThickness(false);
return IsWebUITabStrip() ? FrameTopBorderThickness(true) : 1;
} }
int GlassBrowserFrameView::MinimizeButtonX() const { int GlassBrowserFrameView::MinimizeButtonX() const {
...@@ -690,9 +711,14 @@ void GlassBrowserFrameView::LayoutCaptionButtons() { ...@@ -690,9 +711,14 @@ void GlassBrowserFrameView::LayoutCaptionButtons() {
const gfx::Size preferred_size = const gfx::Size preferred_size =
caption_button_container_->GetPreferredSize(); caption_button_container_->GetPreferredSize();
int height = preferred_size.height();
// We use the standard caption bar height when maximized in tablet mode, which
// is smaller than our preferred button size.
if (IsWebUITabStrip() && IsMaximized())
height = std::min(height, TitlebarMaximizedVisualHeight());
caption_button_container_->SetBounds(width() - preferred_size.width(), caption_button_container_->SetBounds(width() - preferred_size.width(),
WindowTopY(), preferred_size.width(), WindowTopY(), preferred_size.width(),
preferred_size.height()); height);
} }
void GlassBrowserFrameView::LayoutClientView() { void GlassBrowserFrameView::LayoutClientView() {
......
...@@ -180,8 +180,9 @@ void DrawRect(gfx::Canvas* canvas, ...@@ -180,8 +180,9 @@ void DrawRect(gfx::Canvas* canvas,
void Windows10CaptionButton::PaintSymbol(gfx::Canvas* canvas) { void Windows10CaptionButton::PaintSymbol(gfx::Canvas* canvas) {
SkColor symbol_color = GetBaseColor(); SkColor symbol_color = GetBaseColor();
if (!frame_view_->ShouldPaintAsActive() && GetState() != STATE_HOVERED && if (!GetEnabled() ||
GetState() != STATE_PRESSED) { (!frame_view_->ShouldPaintAsActive() && GetState() != STATE_HOVERED &&
GetState() != STATE_PRESSED)) {
symbol_color = SkColorSetA( symbol_color = SkColorSetA(
symbol_color, GlassBrowserFrameView::kInactiveTitlebarFeatureAlpha); symbol_color, GlassBrowserFrameView::kInactiveTitlebarFeatureAlpha);
} else if (button_type_ == VIEW_ID_CLOSE_BUTTON && } else if (button_type_ == VIEW_ID_CLOSE_BUTTON &&
......
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