From a422305acc64fb352120abaad5522b4f75affe1e Mon Sep 17 00:00:00 2001 From: Roger Tinkoff Date: Sat, 17 Dec 2022 19:20:09 +0000 Subject: [PATCH] ash: Scrollable VC bubble effects section BubbleView now makes the effects controls sections children of a ScrollView. This is done in (overridden) ViewAddedToWidget() because BubbleDialogDelegate::GetBubbleBounds() requires a parent widget, which isn't officially assigned until after the call to ShowBubble() in VideoConferenceTray::ToggleBubble(). The "return to app" button is added to a top-level FlexLayout in the BubbleView constructor, as it is not part of the scrollable area. The fake controller now owns a fake_video_conference::EffectRepository, which contains an instance of each of the fake effects. Now individual effects can be added/removed as needed without requiring changes to the fake controller. To include the effects in the fake_video_conference::EffectRepository, the feature "VcControlsUiFakeEffects" must be enabled. Bug: b/262767123 Change-Id: Ica668ef64f179a9835df05fa8a1e00620b18f4d7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4112846 Reviewed-by: Alex Newcomer Commit-Queue: Roger Tinkoff Cr-Commit-Position: refs/heads/main@{#1084733} --- ash/constants/ash_features.cc | 13 ++++- ash/constants/ash_features.h | 2 + .../video_conference/bubble/bubble_view.cc | 45 +++++++++++++-- .../video_conference/bubble/bubble_view.h | 7 +++ .../effects/fake_video_conference_effects.cc | 55 +++++++++++++++++++ .../effects/fake_video_conference_effects.h | 35 +++++++++++- .../fake_video_conference_tray_controller.cc | 10 ++++ .../fake_video_conference_tray_controller.h | 11 +++- 8 files changed, 168 insertions(+), 10 deletions(-) diff --git a/ash/constants/ash_features.cc b/ash/constants/ash_features.cc index ce1d9018ce0021..27feefe277d8ee 100644 --- a/ash/constants/ash_features.cc +++ b/ash/constants/ash_features.cc @@ -2079,6 +2079,12 @@ BASE_FEATURE(kUserActivityPrediction, // Enable or disable the ChromeOS video conferencing controls UI. BASE_FEATURE(kVcControlsUi, "VcControlsUi", base::FEATURE_DISABLED_BY_DEFAULT); +// Enable or disable the fake effects for ChromeOS video conferencing controls +// UI. Only meaningful in the emulator. +BASE_FEATURE(kVcControlsUiFakeEffects, + "VcControlsUiFakeEffects", + base::FEATURE_DISABLED_BY_DEFAULT); + // Enable or disable multitouch for virtual keyboard on ChromeOS. BASE_FEATURE(kVirtualKeyboardMultitouch, "VirtualKeyboardMultitouch", @@ -2236,8 +2242,9 @@ bool AreCaptureModeDemoToolsEnabled() { } bool AreContextualNudgesEnabled() { - if (!IsHideShelfControlsInTabletModeEnabled()) + if (!IsHideShelfControlsInTabletModeEnabled()) { return false; + } return base::FeatureList::IsEnabled(kContextualNudges); } @@ -3178,6 +3185,10 @@ bool IsVcControlsUiEnabled() { return base::FeatureList::IsEnabled(kVcControlsUi); } +bool IsVcControlsUiFakeEffectsEnabled() { + return base::FeatureList::IsEnabled(kVcControlsUiFakeEffects); +} + bool IsViewPpdEnabled() { return base::FeatureList::IsEnabled(kEnableViewPpd); } diff --git a/ash/constants/ash_features.h b/ash/constants/ash_features.h index 2b25c08bfa67c5..df18690efa06ba 100644 --- a/ash/constants/ash_features.h +++ b/ash/constants/ash_features.h @@ -593,6 +593,7 @@ BASE_DECLARE_FEATURE(kUseStorkSmdsServerAddress); COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kUseWallpaperStagingUrl); COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kUserActivityPrediction); COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kVcControlsUi); +COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kVcControlsUiFakeEffects); COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kVirtualKeyboardBorderedKey); COMPONENT_EXPORT(ASH_CONSTANTS) @@ -876,6 +877,7 @@ COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCBackgroundBlurEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCBackgroundReplaceEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVCPortraitRelightingEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVcControlsUiEnabled(); +COMPONENT_EXPORT(ASH_CONSTANTS) bool IsVcControlsUiFakeEffectsEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsViewPpdEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsWallpaperFastRefreshEnabled(); COMPONENT_EXPORT(ASH_CONSTANTS) bool IsWallpaperFullScreenPreviewEnabled(); diff --git a/ash/system/video_conference/bubble/bubble_view.cc b/ash/system/video_conference/bubble/bubble_view.cc index 1978bea9bc09a7..11056a702a1ecb 100644 --- a/ash/system/video_conference/bubble/bubble_view.cc +++ b/ash/system/video_conference/bubble/bubble_view.cc @@ -12,7 +12,10 @@ #include "ash/system/video_conference/effects/video_conference_tray_effects_manager.h" #include "ash/system/video_conference/video_conference_tray_controller.h" #include "ui/views/border.h" +#include "ui/views/controls/scroll_view.h" +#include "ui/views/layout/fill_layout.h" #include "ui/views/layout/flex_layout.h" +#include "ui/views/layout/flex_layout_view.h" namespace ash::video_conference { @@ -24,22 +27,54 @@ const int kBorderInsetDimension = 10; BubbleView::BubbleView(const InitParams& init_params, VideoConferenceTrayController* controller) - : TrayBubbleView(init_params) { + : TrayBubbleView(init_params), controller_(controller) { SetID(BubbleViewID::kMainBubbleView); + // Add a `FlexLayout` for the entire view. views::FlexLayout* layout = SetLayoutManager(std::make_unique()); layout->SetOrientation(views::LayoutOrientation::kVertical); layout->SetMainAxisAlignment(views::LayoutAlignment::kCenter); layout->SetCrossAxisAlignment(views::LayoutAlignment::kStretch); + // `ReturnToAppButton` resides in the top-level layout and isn't part of the + // scrollable area (that can't be added until the `BubbleView` officially has + // a parent waidget). AddChildView(std::make_unique()); +} + +void BubbleView::AddedToWidget() { + // Create the `views::ScrollView` to house the effects sections. This has to + // be done here because `BubbleDialogDelegate::GetBubbleBounds` requires a + // parent widget, which isn't officially assigned until after the call to + // `ShowBubble` in `VideoConferenceTray::ToggleBubble`. + auto* scroll_view = AddChildView(std::make_unique()); + scroll_view->SetAllowKeyboardScrolling(false); + scroll_view->SetBackgroundColor(absl::nullopt); - if (controller->effects_manager().HasToggleEffects()) - AddChildView(std::make_unique(controller)); + // TODO(b/262930924): Use the correct max_height. + scroll_view->ClipHeightTo(/*min_height=*/0, /*max_height=*/300); + scroll_view->SetDrawOverflowIndicator(false); + scroll_view->SetVerticalScrollBarMode( + views::ScrollView::ScrollBarMode::kHiddenButEnabled); - if (controller->effects_manager().HasSetValueEffects()) { - AddChildView(std::make_unique(controller)); + // `FlexLayout` makes the most sense for the contents of the + // `views::ScrollView`, since the effects sections are stacked vertically and + // need to expand to fill the bubble. + views::FlexLayoutView* layout = + scroll_view->SetContents(std::make_unique()); + layout->SetOrientation(views::LayoutOrientation::kVertical); + layout->SetMainAxisAlignment(views::LayoutAlignment::kCenter); + layout->SetCrossAxisAlignment(views::LayoutAlignment::kStretch); + + // Make the effects sections children of the `views::FlexLayoutView`, so that + // they scroll (if more effects are present than can fit in the available + // height). + if (controller_->effects_manager().HasToggleEffects()) { + layout->AddChildView(std::make_unique(controller_)); + } + if (controller_->effects_manager().HasSetValueEffects()) { + layout->AddChildView(std::make_unique(controller_)); } SetBorder(views::CreateEmptyBorder( diff --git a/ash/system/video_conference/bubble/bubble_view.h b/ash/system/video_conference/bubble/bubble_view.h index 924c751f23b374..0521c6fd8ef163 100644 --- a/ash/system/video_conference/bubble/bubble_view.h +++ b/ash/system/video_conference/bubble/bubble_view.h @@ -22,6 +22,13 @@ class BubbleView : public TrayBubbleView { BubbleView(const BubbleView&) = delete; BubbleView& operator=(const BubbleView&) = delete; ~BubbleView() override = default; + + // views::View: + void AddedToWidget() override; + + private: + // Unowned by `BubbleView`. + VideoConferenceTrayController* controller_; }; } // namespace video_conference diff --git a/ash/system/video_conference/effects/fake_video_conference_effects.cc b/ash/system/video_conference/effects/fake_video_conference_effects.cc index 683399f8d30b42..e6874c75d4c7be 100644 --- a/ash/system/video_conference/effects/fake_video_conference_effects.cc +++ b/ash/system/video_conference/effects/fake_video_conference_effects.cc @@ -6,9 +6,11 @@ #include +#include "ash/constants/ash_features.h" #include "ash/resources/vector_icons/vector_icons.h" #include "ash/strings/grit/ash_strings.h" #include "ash/system/video_conference/effects/video_conference_tray_effects_manager_types.h" +#include "ash/system/video_conference/fake_video_conference_tray_controller.h" #include "base/functional/bind.h" #include "ui/views/controls/button/button.h" @@ -210,4 +212,57 @@ int SuperCutnessEffect::GetNumActivationsForTesting(int value) { return num_activations_for_testing_[value]; } +// This registers/unregisters all effects owned by `EffectRepository`. +// Comment-out the `RegisterDelegate`/`UnregisterDelegate` calls for effects +// that are not needed e.g. to test `ash::video_conference::BubbleView` with +// only one or two registered effects. +EffectRepository::EffectRepository( + ash::FakeVideoConferenceTrayController* controller) + : controller_(controller), + cat_ears_(std::make_unique()), + dog_fur_(std::make_unique()), + spaceship_(std::make_unique()), + office_bunny_(std::make_unique()), + calm_forest_(std::make_unique()), + stylish_kitchen_(std::make_unique()), + greenhouse_(std::make_unique()), + shaggy_fur_(std::make_unique()), + super_cuteness_(std::make_unique()) { + DCHECK(controller_); + if (features::IsVcControlsUiFakeEffectsEnabled()) { + controller_->effects_manager().RegisterDelegate(cat_ears_.get()); + controller_->effects_manager().RegisterDelegate(dog_fur_.get()); + controller_->effects_manager().RegisterDelegate(spaceship_.get()); + controller_->effects_manager().RegisterDelegate(office_bunny_.get()); + controller_->effects_manager().RegisterDelegate(calm_forest_.get()); + controller_->effects_manager().RegisterDelegate(stylish_kitchen_.get()); + controller_->effects_manager().RegisterDelegate(greenhouse_.get()); + controller_->effects_manager().RegisterDelegate(shaggy_fur_.get()); + controller_->effects_manager().RegisterDelegate(super_cuteness_.get()); + } +} + +EffectRepository::~EffectRepository() { + if (features::IsVcControlsUiFakeEffectsEnabled()) { + controller_->effects_manager().UnregisterDelegate(cat_ears_.get()); + cat_ears_.reset(); + controller_->effects_manager().UnregisterDelegate(dog_fur_.get()); + dog_fur_.reset(); + controller_->effects_manager().UnregisterDelegate(spaceship_.get()); + spaceship_.reset(); + controller_->effects_manager().UnregisterDelegate(office_bunny_.get()); + office_bunny_.reset(); + controller_->effects_manager().UnregisterDelegate(calm_forest_.get()); + calm_forest_.reset(); + controller_->effects_manager().UnregisterDelegate(stylish_kitchen_.get()); + stylish_kitchen_.reset(); + controller_->effects_manager().UnregisterDelegate(greenhouse_.get()); + greenhouse_.reset(); + controller_->effects_manager().UnregisterDelegate(shaggy_fur_.get()); + shaggy_fur_.reset(); + controller_->effects_manager().UnregisterDelegate(super_cuteness_.get()); + super_cuteness_.reset(); + } +} + } // namespace ash::fake_video_conference diff --git a/ash/system/video_conference/effects/fake_video_conference_effects.h b/ash/system/video_conference/effects/fake_video_conference_effects.h index 75672b051e819f..f0a61bc468572e 100644 --- a/ash/system/video_conference/effects/fake_video_conference_effects.h +++ b/ash/system/video_conference/effects/fake_video_conference_effects.h @@ -15,7 +15,11 @@ namespace gfx { struct VectorIcon; } // namespace gfx -namespace ash::fake_video_conference { +namespace ash { + +class FakeVideoConferenceTrayController; + +namespace fake_video_conference { // A convenience base class, for creating a delegate that hosts the simplest // type of effect there is i.e. a toggle with only one state. @@ -181,6 +185,33 @@ class ASH_EXPORT SuperCutnessEffect : public VcEffectsDelegate { std::vector num_activations_for_testing_; }; -} // namespace ash::fake_video_conference +// A simple residence for any fake effects used for testing. For all of these +// fake effects to be registered, the feature `VcControlsUiFakeEffects` must be +// enabled. +class EffectRepository { + public: + explicit EffectRepository(FakeVideoConferenceTrayController* controller); + + EffectRepository(const EffectRepository&) = delete; + EffectRepository& operator=(const EffectRepository&) = delete; + + ~EffectRepository(); + + private: + FakeVideoConferenceTrayController* controller_; + std::unique_ptr cat_ears_; + std::unique_ptr dog_fur_; + std::unique_ptr spaceship_; + std::unique_ptr office_bunny_; + std::unique_ptr calm_forest_; + std::unique_ptr stylish_kitchen_; + std::unique_ptr greenhouse_; + std::unique_ptr shaggy_fur_; + std::unique_ptr super_cuteness_; +}; + +} // namespace fake_video_conference + +} // namespace ash #endif // ASH_SYSTEM_VIDEO_CONFERENCE_EFFECTS_FAKE_VIDEO_CONFERENCE_EFFECTS_H_ diff --git a/ash/system/video_conference/fake_video_conference_tray_controller.cc b/ash/system/video_conference/fake_video_conference_tray_controller.cc index 4f84dd6612da39..6453538644e432 100644 --- a/ash/system/video_conference/fake_video_conference_tray_controller.cc +++ b/ash/system/video_conference/fake_video_conference_tray_controller.cc @@ -4,11 +4,21 @@ #include "ash/system/video_conference/fake_video_conference_tray_controller.h" +#include "ash/system/video_conference/effects/fake_video_conference_effects.h" #include "chromeos/ash/components/audio/cras_audio_handler.h" #include "media/capture/video/chromeos/mojom/cros_camera_service.mojom-shared.h" namespace ash { +FakeVideoConferenceTrayController::FakeVideoConferenceTrayController() + : effect_repository_( + std::make_unique( + /*controller=*/this)) {} + +FakeVideoConferenceTrayController::~FakeVideoConferenceTrayController() { + effect_repository_.reset(); +} + void FakeVideoConferenceTrayController::SetCameraMuted(bool muted) { camera_muted_ = muted; OnCameraSWPrivacySwitchStateChanged( diff --git a/ash/system/video_conference/fake_video_conference_tray_controller.h b/ash/system/video_conference/fake_video_conference_tray_controller.h index 860be7d3350d31..0d3654542abfbb 100644 --- a/ash/system/video_conference/fake_video_conference_tray_controller.h +++ b/ash/system/video_conference/fake_video_conference_tray_controller.h @@ -10,19 +10,23 @@ namespace ash { +namespace fake_video_conference { +class EffectRepository; +} + // A fake version of VideoConferenceTrayController that will be use in tests or // mocking in the emulator. class ASH_EXPORT FakeVideoConferenceTrayController : public VideoConferenceTrayController { public: - FakeVideoConferenceTrayController() = default; + FakeVideoConferenceTrayController(); FakeVideoConferenceTrayController(const FakeVideoConferenceTrayController&) = delete; FakeVideoConferenceTrayController& operator=( const FakeVideoConferenceTrayController&) = delete; - ~FakeVideoConferenceTrayController() override = default; + ~FakeVideoConferenceTrayController() override; // VideoConferenceTrayController: void SetCameraMuted(bool muted) override; @@ -35,6 +39,9 @@ class ASH_EXPORT FakeVideoConferenceTrayController // Indicates whether camera/microphone is muted. bool camera_muted_ = false; bool microphone_muted_ = false; + + // General-purpose repository for fake effects. + std::unique_ptr effect_repository_; }; } // namespace ash