From eafb28e78e635509413d15a72a588d11b19117e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Fri, 7 May 2021 23:37:25 +0000 Subject: [PATCH] Remove ViewWithInkDrop template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves shared configuration between CaptureModeButton and CaptureModeToggleButton into CaptureModeButton::ConfigureButton. Some shared configuration is unfortunately left out due to using protected members. Bug: None Change-Id: Ifd1c548d6d42d3c54bc85f125864461994e6df83 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2880834 Reviewed-by: James Cook Commit-Queue: Peter Boström Cr-Commit-Position: refs/heads/master@{#880625} --- ash/BUILD.gn | 1 - ash/capture_mode/capture_mode_button.cc | 53 ++++++++++++---- ash/capture_mode/capture_mode_button.h | 13 +++- ash/capture_mode/capture_mode_source_view.cc | 1 + .../capture_mode_toggle_button.cc | 25 ++------ ash/capture_mode/capture_mode_toggle_button.h | 3 +- ash/capture_mode/view_with_ink_drop.h | 60 ------------------- 7 files changed, 58 insertions(+), 98 deletions(-) delete mode 100644 ash/capture_mode/view_with_ink_drop.h diff --git a/ash/BUILD.gn b/ash/BUILD.gn index d7c50e963b1c8..ad12c07bf8eb6 100644 --- a/ash/BUILD.gn +++ b/ash/BUILD.gn @@ -316,7 +316,6 @@ component("ash") { "capture_mode/video_file_handler.h", "capture_mode/video_recording_watcher.cc", "capture_mode/video_recording_watcher.h", - "capture_mode/view_with_ink_drop.h", "child_accounts/parent_access_controller_impl.cc", "child_accounts/parent_access_controller_impl.h", "clipboard/clipboard_history.cc", diff --git a/ash/capture_mode/capture_mode_button.cc b/ash/capture_mode/capture_mode_button.cc index 7e82451859554..ab72d582817f0 100644 --- a/ash/capture_mode/capture_mode_button.cc +++ b/ash/capture_mode/capture_mode_button.cc @@ -4,39 +4,66 @@ #include "ash/capture_mode/capture_mode_button.h" +#include "ash/capture_mode/capture_mode_constants.h" #include "ash/resources/vector_icons/vector_icons.h" #include "ash/style/ash_color_provider.h" +#include "base/bind.h" #include "ui/base/metadata/metadata_impl_macros.h" #include "ui/gfx/paint_vector_icon.h" #include "ui/views/accessibility/view_accessibility.h" +#include "ui/views/animation/ink_drop.h" +#include "ui/views/animation/ink_drop_highlight.h" +#include "ui/views/animation/ink_drop_host_view.h" #include "ui/views/controls/highlight_path_generator.h" namespace ash { CaptureModeButton::CaptureModeButton(views::Button::PressedCallback callback, const gfx::VectorIcon& icon) - : ViewWithInkDrop(callback) { - SetPreferredSize(capture_mode::kButtonSize); - SetBorder(views::CreateEmptyBorder(capture_mode::kButtonPadding)); - auto* color_provider = AshColorProvider::Get(); - const SkColor normal_color = color_provider->GetContentLayerColor( + : views::ImageButton(callback) { + ConfigureButton(this, focus_ring()); + const SkColor normal_color = AshColorProvider::Get()->GetContentLayerColor( AshColorProvider::ContentLayerType::kButtonIconColor); SetImage(views::Button::STATE_NORMAL, gfx::CreateVectorIcon(icon, normal_color)); - SetImageHorizontalAlignment(ALIGN_CENTER); - SetImageVerticalAlignment(ALIGN_MIDDLE); - GetViewAccessibility().OverrideIsLeaf(true); // TODO(afakhry): Fix this. GetViewAccessibility().OverrideName(GetClassName()); +} + +// static +void CaptureModeButton::ConfigureButton(views::ImageButton* button, + views::FocusRing* focus_ring) { + button->ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON); + button->SetHasInkDropActionOnClick(true); + button->ink_drop()->SetVisibleOpacity(capture_mode::kInkDropVisibleOpacity); + views::InkDrop::UseInkDropForFloodFillRipple(button->ink_drop(), + /*highlight_on_hover=*/false, + /*highlight_on_focus=*/false); + button->ink_drop()->SetCreateHighlightCallback(base::BindRepeating( + [](views::Button* host) { + auto highlight = std::make_unique( + gfx::SizeF(host->size()), host->ink_drop()->GetBaseColor()); + highlight->set_visible_opacity( + capture_mode::kInkDropHighlightVisibleOpacity); + return highlight; + }, + button)); + button->ink_drop()->SetBaseColor(capture_mode::kInkDropBaseColor); + + button->SetImageHorizontalAlignment(ALIGN_CENTER); + button->SetImageVerticalAlignment(ALIGN_MIDDLE); + button->SetPreferredSize(capture_mode::kButtonSize); + button->SetBorder(views::CreateEmptyBorder(capture_mode::kButtonPadding)); + button->GetViewAccessibility().OverrideIsLeaf(true); - SetInstallFocusRingOnFocus(true); - focus_ring()->SetColor(color_provider->GetControlsLayerColor( + button->SetInstallFocusRingOnFocus(true); + focus_ring->SetColor(AshColorProvider::Get()->GetControlsLayerColor( AshColorProvider::ControlsLayerType::kFocusRingColor)); - focus_ring()->SetPathGenerator( + focus_ring->SetPathGenerator( std::make_unique( capture_mode::kButtonPadding)); - views::InstallCircleHighlightPathGenerator(this, + views::InstallCircleHighlightPathGenerator(button, capture_mode::kButtonPadding); } @@ -44,7 +71,7 @@ views::View* CaptureModeButton::GetView() { return this; } -BEGIN_METADATA(CaptureModeButton, ViewWithInkDrop) +BEGIN_METADATA(CaptureModeButton, views::ImageButton) END_METADATA } // namespace ash diff --git a/ash/capture_mode/capture_mode_button.h b/ash/capture_mode/capture_mode_button.h index d11266a740eba..fa5ad5cf45bd1 100644 --- a/ash/capture_mode/capture_mode_button.h +++ b/ash/capture_mode/capture_mode_button.h @@ -7,20 +7,22 @@ #include "ash/ash_export.h" #include "ash/capture_mode/capture_mode_session_focus_cycler.h" -#include "ash/capture_mode/view_with_ink_drop.h" #include "ui/base/metadata/metadata_header_macros.h" -#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/image_button.h" namespace gfx { struct VectorIcon; } // namespace gfx +namespace views { +class FocusRing; +} // namespace views + namespace ash { // A view that shows a button which is part of the CaptureBarView. class ASH_EXPORT CaptureModeButton - : public ViewWithInkDrop, + : public views::ImageButton, public CaptureModeSessionFocusCycler::HighlightableView { public: METADATA_HEADER(CaptureModeButton); @@ -31,6 +33,11 @@ class ASH_EXPORT CaptureModeButton CaptureModeButton& operator=(const CaptureModeButton&) = delete; ~CaptureModeButton() override = default; + // Common configuration for CaptureModeButton and CaptureModeToggleButton, + // such as InkDrop, preferred size, border, etc. + static void ConfigureButton(views::ImageButton* button, + views::FocusRing* focus_ring); + // CaptureModeSessionFocusCycler::HighlightableView: views::View* GetView() override; }; diff --git a/ash/capture_mode/capture_mode_source_view.cc b/ash/capture_mode/capture_mode_source_view.cc index 19cecc3769edd..97feb3decef9b 100644 --- a/ash/capture_mode/capture_mode_source_view.cc +++ b/ash/capture_mode/capture_mode_source_view.cc @@ -6,6 +6,7 @@ #include +#include "ash/capture_mode/capture_mode_constants.h" #include "ash/capture_mode/capture_mode_controller.h" #include "ash/capture_mode/capture_mode_metrics.h" #include "ash/capture_mode/capture_mode_toggle_button.h" diff --git a/ash/capture_mode/capture_mode_toggle_button.cc b/ash/capture_mode/capture_mode_toggle_button.cc index 07b8b54f9c731..863ebe737f778 100644 --- a/ash/capture_mode/capture_mode_toggle_button.cc +++ b/ash/capture_mode/capture_mode_toggle_button.cc @@ -4,6 +4,8 @@ #include "ash/capture_mode/capture_mode_toggle_button.h" +#include "ash/capture_mode/capture_mode_button.h" +#include "ash/capture_mode/capture_mode_constants.h" #include "ash/style/ash_color_provider.h" #include "base/strings/utf_string_conversions.h" #include "ui/base/metadata/metadata_impl_macros.h" @@ -17,25 +19,11 @@ namespace ash { CaptureModeToggleButton::CaptureModeToggleButton( views::Button::PressedCallback callback, const gfx::VectorIcon& icon) - : ViewWithInkDrop(callback) { - SetPreferredSize(capture_mode::kButtonSize); - SetBorder(views::CreateEmptyBorder(capture_mode::kButtonPadding)); - SetImageHorizontalAlignment(ALIGN_CENTER); - SetImageVerticalAlignment(ALIGN_MIDDLE); - GetViewAccessibility().OverrideIsLeaf(true); - - SetInstallFocusRingOnFocus(true); - const auto* color_provider = AshColorProvider::Get(); - focus_ring()->SetColor(color_provider->GetControlsLayerColor( - AshColorProvider::ControlsLayerType::kFocusRingColor)); - focus_ring()->SetPathGenerator( - std::make_unique( - capture_mode::kButtonPadding)); - views::InstallCircleHighlightPathGenerator(this, - capture_mode::kButtonPadding); + : views::ToggleImageButton(callback) { + CaptureModeButton::ConfigureButton(this, focus_ring()); SetIcon(icon); - toggled_background_color_ = color_provider->GetControlsLayerColor( + toggled_background_color_ = AshColorProvider::Get()->GetControlsLayerColor( AshColorProvider::ControlsLayerType::kControlBackgroundColorActive); } @@ -83,8 +71,7 @@ void CaptureModeToggleButton::SetIcon(const gfx::VectorIcon& icon) { SetToggledImage(views::Button::STATE_NORMAL, &toggled_icon); } -BEGIN_METADATA(CaptureModeToggleButton, - ViewWithInkDrop) +BEGIN_METADATA(CaptureModeToggleButton, views::ToggleImageButton) END_METADATA } // namespace ash diff --git a/ash/capture_mode/capture_mode_toggle_button.h b/ash/capture_mode/capture_mode_toggle_button.h index 02c57d2f04eb5..7c42cde9a4a3a 100644 --- a/ash/capture_mode/capture_mode_toggle_button.h +++ b/ash/capture_mode/capture_mode_toggle_button.h @@ -7,7 +7,6 @@ #include "ash/ash_export.h" #include "ash/capture_mode/capture_mode_session_focus_cycler.h" -#include "ash/capture_mode/view_with_ink_drop.h" #include "ui/base/metadata/metadata_header_macros.h" #include "ui/views/controls/button/button.h" #include "ui/views/controls/button/image_button.h" @@ -22,7 +21,7 @@ namespace ash { // toggle between image and video capture, and between fullscreen, window, and // region capture sources. class ASH_EXPORT CaptureModeToggleButton - : public ViewWithInkDrop, + : public views::ToggleImageButton, public CaptureModeSessionFocusCycler::HighlightableView { public: METADATA_HEADER(CaptureModeToggleButton); diff --git a/ash/capture_mode/view_with_ink_drop.h b/ash/capture_mode/view_with_ink_drop.h deleted file mode 100644 index a463a2c2a67e9..0000000000000 --- a/ash/capture_mode/view_with_ink_drop.h +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2020 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef ASH_CAPTURE_MODE_VIEW_WITH_INK_DROP_H_ -#define ASH_CAPTURE_MODE_VIEW_WITH_INK_DROP_H_ - -#include - -#include "ash/capture_mode/capture_mode_constants.h" -#include "base/bind.h" -#include "ui/views/animation/ink_drop_host_view.h" -#include "ui/views/animation/ink_drop_impl.h" - -namespace ash { - -// A template class that relieves us from having to rewrite the ink drop boiler- -// plate code for all the Capture Mode views that will need it. This is used by -// CaptureModeToggleButton, CaptureModeCloseButton, ... etc. -// |T| must be a subtype of |views::InkDropHostView|. -// TODO(pbos): Move this constructor into a shared configuration method and call -// it for all uses of ViewWithInkDrop. -template -class ViewWithInkDrop : public T { - public: - static_assert(std::is_base_of::value, - "T must be a subtype of views::InkDropHostView"); - - // A constructor that forwards |args| to |T|'s constructor, so |args| are the - // exact same as required by |T|'s constructor. It sets up the ink drop on the - // view. - template - explicit ViewWithInkDrop(Args... args) : T(std::forward(args)...) { - T::ink_drop()->SetMode(views::InkDropHost::InkDropMode::ON); - T::SetHasInkDropActionOnClick(true); - T::ink_drop()->SetVisibleOpacity(capture_mode::kInkDropVisibleOpacity); - views::InkDrop::UseInkDropForFloodFillRipple(T::ink_drop(), - /*highlight_on_hover=*/false, - /*highlight_on_focus=*/false); - T::ink_drop()->SetCreateHighlightCallback(base::BindRepeating( - [](views::InkDropHostView* host) { - auto highlight = std::make_unique( - gfx::SizeF(host->size()), host->ink_drop()->GetBaseColor()); - highlight->set_visible_opacity( - capture_mode::kInkDropHighlightVisibleOpacity); - return highlight; - }, - this)); - // TODO(pbos): See if this is a no-op when replaced with - // ink_drop()->SetBaseColor(), i.e. that nothing sets it later. - T::ink_drop()->SetBaseColorCallback( - base::BindRepeating([]() { return capture_mode::kInkDropBaseColor; })); - } - - ~ViewWithInkDrop() override = default; -}; - -} // namespace ash - -#endif // ASH_CAPTURE_MODE_VIEW_WITH_INK_DROP_H_