Commit 48af1c3a authored by Connie Wan's avatar Connie Wan Committed by Commit Bot

Simplify painting of Tab Group dragging highlight

Remove the ShouldPaint logic in TabGroupView because it's already handled in TabStrip::PaintChildren(): https://cs.chromium.org/chromium/src/chrome/browser/ui/views/tabs/tab_strip.cc?l=2088&rcl=2170efeafe8ee9cafa678d8d1bae67f8182798a8

Also simplify TabStrip::StoppedDraggingView(), which was handling group headers in a hacky way. The group header handling is now more explicit and separate from the deleted-tab logic.

Bug: 1034169
Change-Id: I7e87387e6cb89863cb6a0d4b5b706c3ef14d780e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1988678
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: default avatarCharlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728689}
parent f91732da
...@@ -23,9 +23,6 @@ TabGroupHighlight::TabGroupHighlight(TabGroupViews* tab_group_views, ...@@ -23,9 +23,6 @@ TabGroupHighlight::TabGroupHighlight(TabGroupViews* tab_group_views,
} }
void TabGroupHighlight::OnPaint(gfx::Canvas* canvas) { void TabGroupHighlight::OnPaint(gfx::Canvas* canvas) {
if (!tab_group_views_->ShouldPaintGroupBackground())
return;
SetBoundsRect(tab_group_views_->GetBounds()); SetBoundsRect(tab_group_views_->GetBounds());
SkPath path = GetPath(); SkPath path = GetPath();
......
...@@ -91,7 +91,3 @@ SkColor TabGroupViews::GetGroupBackgroundColor() const { ...@@ -91,7 +91,3 @@ SkColor TabGroupViews::GetGroupBackgroundColor() const {
TabStyle::kSelectedTabOpacity, TabStyle::kSelectedTabOpacity,
SK_AlphaTRANSPARENT, SK_AlphaOPAQUE)); SK_AlphaTRANSPARENT, SK_AlphaOPAQUE));
} }
bool TabGroupViews::ShouldPaintGroupBackground() const {
return header_->dragging();
}
...@@ -49,10 +49,6 @@ class TabGroupViews { ...@@ -49,10 +49,6 @@ class TabGroupViews {
// tab color. Needed to layer painting for the group background highlight. // tab color. Needed to layer painting for the group background highlight.
SkColor GetGroupBackgroundColor() const; SkColor GetGroupBackgroundColor() const;
// Returns whether the group highlight background should be shown. Currently
// it should only be shown if the entire group is dragging via its header.
bool ShouldPaintGroupBackground() const;
private: private:
TabStrip* const tab_strip_; TabStrip* const tab_strip_;
const tab_groups::TabGroupId group_; const tab_groups::TabGroupId group_;
......
...@@ -2686,14 +2686,16 @@ void TabStrip::OnGroupCloseAnimationCompleted( ...@@ -2686,14 +2686,16 @@ void TabStrip::OnGroupCloseAnimationCompleted(
} }
void TabStrip::StoppedDraggingView(TabSlotView* view, bool* is_first_view) { void TabStrip::StoppedDraggingView(TabSlotView* view, bool* is_first_view) {
if (view &&
view->GetTabSlotViewType() == TabSlotView::ViewType::kTabGroupHeader) {
// Ensure all tab group UI is repainted, especially the dragging highlight.
view->set_dragging(false);
SchedulePaint();
return;
}
int tab_data_index = GetModelIndexOf(view); int tab_data_index = GetModelIndexOf(view);
if (tab_data_index == -1) { if (tab_data_index == -1) {
// Ensure the drag status is updated even if the view is not a valid tab.
// This is primarily to make sure group headers are updated correctly.
// Otherwise, tab drag status is only updated in PrepareForAnimation().
if (view)
view->set_dragging(false);
// The tab was removed before the drag completed. Don't do anything. // The tab was removed before the drag completed. Don't do anything.
return; return;
} }
......
...@@ -1230,13 +1230,6 @@ TEST_P(TabStripTest, GroupHighlightBasics) { ...@@ -1230,13 +1230,6 @@ TEST_P(TabStripTest, GroupHighlightBasics) {
std::vector<TabGroupViews*> views = ListGroupViews(); std::vector<TabGroupViews*> views = ListGroupViews();
EXPECT_EQ(1u, views.size()); EXPECT_EQ(1u, views.size());
// Highlights should not be painted by default.
EXPECT_FALSE(views[0]->ShouldPaintGroupBackground());
// Highlights should be painted when the group header is dragging.
views[0]->header()->set_dragging(true);
EXPECT_TRUE(views[0]->ShouldPaintGroupBackground());
// The highlight bounds match the group view bounds. Grab this manually // The highlight bounds match the group view bounds. Grab this manually
// here, since there isn't a real paint cycle to trigger OnPaint(). // here, since there isn't a real paint cycle to trigger OnPaint().
gfx::Rect bounds = views[0]->GetBounds(); gfx::Rect bounds = views[0]->GetBounds();
......
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