Skip to content

Commit

Permalink
Remove Tasks bubble on offline bubble
Browse files Browse the repository at this point in the history
When Glanceables tries to show the Task bubble, if the client fails to
fetch tasks, such as an error with the connection, the bubble will still
be added to the UI tree, but it will be an empty bubble with 0px height.

To avoid future issues, only add the Task bubble if the client had a
successful query.

Bug: b:292247870
Change-Id: I94dcdedb28aa8c48a3761e944465f6762b4c6ea3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4803448
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Ana Salazar <anasalazar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1188629}
  • Loading branch information
Ana Salazar authored and Chromium LUCI CQ committed Aug 26, 2023
1 parent 9bfb43f commit 3b11b86
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 39 deletions.
2 changes: 2 additions & 0 deletions ash/glanceables/tasks/fake_glanceables_tasks_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ class ASH_EXPORT FakeGlanceablesTasksClient : public GlanceablesTasksClient {

void set_paused(bool paused) { paused_ = paused; }

ui::ListModel<GlanceablesTaskList>* task_lists() { return task_lists_.get(); }

private:
void PopulateTasks(base::Time tasks_due_time);
void PopulateTaskLists(base::Time tasks_due_time);
Expand Down
33 changes: 26 additions & 7 deletions ash/system/unified/glanceable_tray_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include "ash/constants/ash_features.h"
#include "ash/glanceables/classroom/glanceables_classroom_client.h"
#include "ash/glanceables/glanceables_v2_controller.h"
#include "ash/glanceables/tasks/glanceables_tasks_client.h"
#include "ash/glanceables/tasks/glanceables_tasks_types.h"
#include "ash/public/cpp/session/user_info.h"
#include "ash/shelf/shelf.h"
#include "ash/shell.h"
Expand All @@ -23,6 +25,7 @@
#include "base/check.h"
#include "base/functional/bind.h"
#include "components/session_manager/session_manager_types.h"
#include "ui/base/models/list_model.h"
#include "ui/compositor/layer.h"
#include "ui/gfx/geometry/rounded_corners_f.h"
#include "ui/views/controls/scroll_view.h"
Expand Down Expand Up @@ -147,21 +150,24 @@ void GlanceableTrayBubbleView::InitializeContents() {
session_manager::SessionState::ACTIVE &&
session_controller->GetUserSession(0)->user_info.has_gaia_account;

if (should_show_non_calendar_glanceables && !tasks_bubble_view_) {
tasks_bubble_view_ = child_glanceable_container->AddChildView(
std::make_unique<TasksBubbleView>(detailed_view_delegate_.get()));
scroll_view_->SetContents(std::move(child_glanceable_container));

auto* const tasks_client =
Shell::Get()->glanceables_v2_controller()->GetTasksClient();
if (should_show_non_calendar_glanceables && tasks_client) {
CHECK(!tasks_bubble_view_);
tasks_client->GetTaskLists(
base::BindOnce(&GlanceableTrayBubbleView::AddTaskBubbleViewIfNeeded,
weak_ptr_factory_.GetWeakPtr()));
}

if (!calendar_view_) {
calendar_view_ =
child_glanceable_container->AddChildView(std::make_unique<CalendarView>(
scroll_view_->contents()->AddChildView(std::make_unique<CalendarView>(
detailed_view_delegate_.get(), /*for_glanceables_container=*/true));
// TODO(b:277268122): Update with glanceable spec.
calendar_view_->SetPreferredSize(gfx::Size(kRevampedTrayMenuWidth, 400));
}

scroll_view_->SetContents(std::move(child_glanceable_container));

int max_height = CalculateMaxTrayBubbleHeight(shelf_->GetWindow());
SetMaxHeight(max_height);
ChangeAnchorAlignment(shelf_->alignment());
Expand Down Expand Up @@ -230,6 +236,19 @@ void GlanceableTrayBubbleView::AddClassroomBubbleViewIfNeeded(
std::make_unique<T>(detailed_view_delegate_.get()), calendar_view_index);
}

void GlanceableTrayBubbleView::AddTaskBubbleViewIfNeeded(
ui::ListModel<GlanceablesTaskList>* task_lists) {
if (task_lists->item_count() == 0) {
return;
}
// Add tasks bubble before everything.
auto* const scroll_contents = scroll_view_->contents();
tasks_bubble_view_ = scroll_contents->AddChildViewAt(
std::make_unique<TasksBubbleView>(detailed_view_delegate_.get(),
task_lists),
0);
}

void GlanceableTrayBubbleView::OnGlanceablesContainerPreferredSizeChanged() {
UpdateBubble();
}
Expand Down
8 changes: 8 additions & 0 deletions ash/system/unified/glanceable_tray_bubble_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@
#include "ash/system/tray/tray_bubble_view.h"
#include "base/memory/weak_ptr.h"

namespace ui {
template <class ItemType>
class ListModel;
}

namespace ash {
class CalendarView;
class ClassroomBubbleStudentView;
class ClassroomBubbleTeacherView;
class DetailedViewDelegate;
struct GlanceablesTaskList;
class TasksBubbleView;
class Shelf;

Expand Down Expand Up @@ -53,6 +59,8 @@ class GlanceableTrayBubbleView : public TrayBubbleView,
template <typename T>
void AddClassroomBubbleViewIfNeeded(raw_ptr<T, ExperimentalAsh>* view,
bool is_role_active);
void AddTaskBubbleViewIfNeeded(
ui::ListModel<GlanceablesTaskList>* task_lists);

void OnGlanceablesContainerPreferredSizeChanged();
void OnGlanceablesContainerHeightChanged(int height_delta);
Expand Down
40 changes: 14 additions & 26 deletions ash/system/unified/tasks_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,33 +52,9 @@ constexpr char kTasksManagementPage[] =

namespace ash {

TasksBubbleView::TasksBubbleView(DetailedViewDelegate* delegate)
TasksBubbleView::TasksBubbleView(DetailedViewDelegate* delegate,
ui::ListModel<GlanceablesTaskList>* task_list)
: GlanceableTrayChildBubble(delegate, /*for_glanceables_container=*/true) {
if (Shell::Get()->glanceables_v2_controller()->GetTasksClient()) {
Shell::Get()->glanceables_v2_controller()->GetTasksClient()->GetTaskLists(
base::BindOnce(&TasksBubbleView::InitViews,
weak_ptr_factory_.GetWeakPtr()));
}
}

TasksBubbleView::~TasksBubbleView() = default;

void TasksBubbleView::OnViewFocused(views::View* view) {
CHECK_EQ(view, task_list_combo_box_view_);

AnnounceListStateOnComboBoxAccessibility();
}

void TasksBubbleView::CancelUpdates() {
weak_ptr_factory_.InvalidateWeakPtrs();
}

void TasksBubbleView::InitViews(ui::ListModel<GlanceablesTaskList>* task_list) {
// TODO(b:277268122): Implement empty tasks glanceable state.
if (task_list->item_count() == 0) {
return;
}

auto* layout_manager =
SetLayoutManager(std::make_unique<views::FlexLayout>());
layout_manager
Expand Down Expand Up @@ -169,6 +145,18 @@ void TasksBubbleView::InitViews(ui::ListModel<GlanceablesTaskList>* task_list) {
SelectedTasksListChanged();
}

TasksBubbleView::~TasksBubbleView() = default;

void TasksBubbleView::OnViewFocused(views::View* view) {
CHECK_EQ(view, task_list_combo_box_view_);

AnnounceListStateOnComboBoxAccessibility();
}

void TasksBubbleView::CancelUpdates() {
weak_ptr_factory_.InvalidateWeakPtrs();
}

void TasksBubbleView::ActionButtonPressed() {
NewWindowDelegate::GetPrimary()->OpenUrl(
GURL(kTasksManagementPage),
Expand Down
6 changes: 2 additions & 4 deletions ash/system/unified/tasks_bubble_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ class ASH_EXPORT TasksBubbleView : public GlanceableTrayChildBubble,
public:
METADATA_HEADER(TasksBubbleView);

explicit TasksBubbleView(DetailedViewDelegate* delegate);
TasksBubbleView(DetailedViewDelegate* delegate,
ui::ListModel<GlanceablesTaskList>* task_list);
TasksBubbleView(const TasksBubbleView&) = delete;
TasksBubbleView& operator=(const TasksBubbleView&) = delete;
~TasksBubbleView() override;
Expand All @@ -86,9 +87,6 @@ class ASH_EXPORT TasksBubbleView : public GlanceableTrayChildBubble,
void CancelUpdates();

private:
// Setup child views.
void InitViews(ui::ListModel<GlanceablesTaskList>* task_list);

// Handles press behavior for the "See all" button in `list_footer_view_` and
// `add_new_task_button_`.
void ActionButtonPressed();
Expand Down
6 changes: 4 additions & 2 deletions ash/system/unified/tasks_bubble_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "ash/test/ash_test_base.h"
#include "base/memory/raw_ptr.h"
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/types/cxx23_to_underlying.h"
#include "components/account_id/account_id.h"
Expand Down Expand Up @@ -84,8 +85,9 @@ class TasksBubbleViewTest : public AshTestBase {
widget_ = CreateFramelessTestWidget();
widget_->SetFullscreen(true);

view_ = widget_->SetContentsView(
std::make_unique<TasksBubbleView>(&detailed_view_delegate_));
view_ = widget_->SetContentsView(std::make_unique<TasksBubbleView>(
&detailed_view_delegate_,
fake_glanceables_tasks_client_->task_lists()));
}

void TearDown() override {
Expand Down

0 comments on commit 3b11b86

Please sign in to comment.