Skip to content

Commit

Permalink
VC UI: Remove disabled icon for toggle effects button
Browse files Browse the repository at this point in the history
We don't want to use the strikethrough icons anymore, so we will now
just use one icons for all states.

Fixed: b:288914497
Change-Id: I13ccc33fecda3388718d068455c8f649bf3cfe0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4969594
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Andre Le <leandre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216726}
  • Loading branch information
Andre Le authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent f91b05f commit e090c22
Show file tree
Hide file tree
Showing 15 changed files with 21 additions and 303 deletions.
4 changes: 0 additions & 4 deletions ash/resources/vector_icons/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -575,17 +575,13 @@ aggregate_vector_icons("ash_vector_icons") {
"video_conference_background_blur_maximum.icon",
"video_conference_background_blur_off.icon",
"video_conference_camera_capturing.icon",
"video_conference_camera_framing_off.icon",
"video_conference_camera_framing_on.icon",
"video_conference_camera_muted.icon",
"video_conference_linux_app_warning.icon",
"video_conference_live_caption_off.icon",
"video_conference_live_caption_on.icon",
"video_conference_microphone_capturing.icon",
"video_conference_microphone_muted.icon",
"video_conference_noise_cancellation_off.icon",
"video_conference_noise_cancellation_on.icon",
"video_conference_portrait_relight_off.icon",
"video_conference_portrait_relight_on.icon",
"video_conference_screen_share.icon",
"video_conference_up_chevron.icon",
Expand Down

This file was deleted.

69 changes: 0 additions & 69 deletions ash/resources/vector_icons/video_conference_live_caption_off.icon

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 0 additions & 2 deletions ash/system/audio/audio_effects_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ void AudioEffectsController::AddNoiseCancellationEffect() {
weak_factory_.GetWeakPtr(),
/*effect_id=*/noise_cancellation_id,
/*value=*/0));
effect_state->set_disabled_icon(&kVideoConferenceNoiseCancellationOffIcon);
effect->AddState(std::move(effect_state));

effect->set_dependency_flags(VcHostedEffect::ResourceDependency::kMicrophone);
Expand Down Expand Up @@ -230,7 +229,6 @@ void AudioEffectsController::AddLiveCaptionEffect() {
weak_factory_.GetWeakPtr(),
/*effect_id=*/live_caption_id,
/*value=*/0));
effect_state->set_disabled_icon(&kVideoConferenceLiveCaptionOffIcon);

effect->AddState(std::move(effect_state));
AddEffect(std::move(effect));
Expand Down
2 changes: 0 additions & 2 deletions ash/system/camera/camera_effects_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,6 @@ void CameraEffectsController::OnAutozoomControlEnabledChanged(bool enabled) {
base::Unretained(this),
/*effect_id=*/VcEffectId::kCameraFraming,
/*value=*/absl::nullopt));
effect_state->set_disabled_icon(&kVideoConferenceCameraFramingOffIcon);
effect->AddState(std::move(effect_state));

effect->set_dependency_flags(VcHostedEffect::ResourceDependency::kCamera);
Expand Down Expand Up @@ -524,7 +523,6 @@ void CameraEffectsController::InitializeEffectControls() {
base::Unretained(this),
/*effect_id=*/VcEffectId::kPortraitRelighting,
/*value=*/absl::nullopt));
effect_state->set_disabled_icon(&kVideoConferencePortraitRelightOffIcon);
effect->AddState(std::move(effect_state));

effect->set_dependency_flags(VcHostedEffect::ResourceDependency::kCamera);
Expand Down
12 changes: 6 additions & 6 deletions ash/system/video_conference/bubble/bubble_view_pixeltest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ TEST_F(BubbleViewPixelTest, Basic) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_basic",
/*revision_number=*/4, bubble_view()));
/*revision_number=*/5, bubble_view()));
}

// Pixel test that tests toggled on/off and focused/not focused for the toggle
Expand All @@ -215,7 +215,7 @@ TEST_F(BubbleViewPixelTest, ToggleButton) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_no_focus_not_toggled",
/*revision_number=*/6, toggle_effect_button_container));
/*revision_number=*/7, toggle_effect_button_container));

// Toggle the first button, the UI should change.
LeftClickOn(first_toggle_effect_button);
Expand All @@ -234,7 +234,7 @@ TEST_F(BubbleViewPixelTest, ToggleButton) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_with_focus_not_toggled",
/*revision_number=*/6, toggle_effect_button_container));
/*revision_number=*/7, toggle_effect_button_container));

// Re-toggle the button.
event_generator->PressAndReleaseKey(ui::KeyboardCode::VKEY_RETURN);
Expand Down Expand Up @@ -339,7 +339,7 @@ TEST_F(BubbleViewPixelTest, OneToggleEffects) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_one_toggle_effect",
/*revision_number=*/4, GetToggleEffectsView()));
/*revision_number=*/5, GetToggleEffectsView()));
}

TEST_F(BubbleViewPixelTest, TwoToggleEffects) {
Expand All @@ -354,7 +354,7 @@ TEST_F(BubbleViewPixelTest, TwoToggleEffects) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_two_toggle_effects",
/*revision_number=*/4, GetToggleEffectsView()));
/*revision_number=*/5, GetToggleEffectsView()));
}

TEST_F(BubbleViewPixelTest, ThreeToggleEffects) {
Expand All @@ -374,7 +374,7 @@ TEST_F(BubbleViewPixelTest, ThreeToggleEffects) {

EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
"video_conference_bubble_view_three_toggle_effects",
/*revision_number=*/4, GetToggleEffectsView()));
/*revision_number=*/5, GetToggleEffectsView()));
}

} // namespace ash::video_conference
1 change: 0 additions & 1 deletion ash/system/video_conference/bubble/bubble_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class SquareCinnamonCereal : public VcEffectsDelegate {
base::Unretained(this),
/*effect_id=*/VcEffectId::kTestEffect,
/*value=*/absl::nullopt));
state->set_disabled_icon(&kVideoConferenceBackgroundBlurOffIcon);
effect->AddState(std::move(state));

AddEffect(std::move(effect));
Expand Down
15 changes: 5 additions & 10 deletions ash/system/video_conference/bubble/toggle_effects_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ END_METADATA

ToggleEffectsButton::ToggleEffectsButton(
views::Button::PressedCallback callback,
const gfx::VectorIcon* enabled_vector_icon,
const gfx::VectorIcon* disabled_vector_icon,
const gfx::VectorIcon* vector_icon,
bool toggle_state,
const std::u16string& label_text,
const int accessible_name_id,
Expand All @@ -173,8 +172,7 @@ ToggleEffectsButton::ToggleEffectsButton(
: callback_(callback),
toggled_(toggle_state),
effect_id_(effect_id),
enabled_vector_icon_(enabled_vector_icon),
disabled_vector_icon_(disabled_vector_icon),
vector_icon_(vector_icon),
accessible_name_id_(accessible_name_id) {
SetCallback(base::BindRepeating(&ToggleEffectsButton::OnButtonClicked,
weak_ptr_factory_.GetWeakPtr()));
Expand Down Expand Up @@ -269,8 +267,7 @@ void ToggleEffectsButton::UpdateColorsAndBackground() {
toggled_ ? cros_tokens::kCrosSysSystemOnPrimaryContainer
: cros_tokens::kCrosSysOnSurface;
icon_->SetImage(ui::ImageModel::FromVectorIcon(
toggled_ ? *enabled_vector_icon_ : *disabled_vector_icon_,
foreground_color_id, kIconSize));
*vector_icon_, foreground_color_id, kIconSize));
label_->SetEnabledColorId(foreground_color_id);
}

Expand Down Expand Up @@ -320,11 +317,9 @@ ToggleEffectsView::ToggleEffectsView(
// `current_state` can only be a `bool` for a toggle effect.
bool toggle_state = current_state.value() != 0;
const VcEffectState* state = tile->GetState(/*index=*/0);
CHECK(state->disabled_icon())
<< "Toggle effects must define a disabled icon.";
row_view->AddChildView(std::make_unique<ToggleEffectsButton>(
state->button_callback(), state->icon(), state->disabled_icon(),
toggle_state, state->label_text(), state->accessible_name_id(),
state->button_callback(), state->icon(), toggle_state,
state->label_text(), state->accessible_name_id(),
tile->container_id(), tile->id(), /*num_button_per_row=*/row.size()));
}

Expand Down

0 comments on commit e090c22

Please sign in to comment.