Commit 76700eb8 authored by Olesia Marukhno's avatar Olesia Marukhno Committed by Chromium LUCI CQ

Fix chip UI interaction with omnibox

Temporary hide chip when user starts typing in the omnibox and re-show
it after user exits the edit mode.

Bug: 1153197
Change-Id: Ibb5cce6e384b6fe6e0a52a4fda838ca26cab208f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2574277
Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
Reviewed-by: default avatarBret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836139}
parent e30de22f
...@@ -1155,6 +1155,7 @@ void LocationBarView::OnChanged() { ...@@ -1155,6 +1155,7 @@ void LocationBarView::OnChanged() {
SchedulePaint(); SchedulePaint();
UpdateSendTabToSelfIcon(); UpdateSendTabToSelfIcon();
UpdateQRCodeGeneratorIcon(); UpdateQRCodeGeneratorIcon();
UpdatePermissionChipVisibility();
} }
void LocationBarView::OnPopupVisibilityChanged() { void LocationBarView::OnPopupVisibilityChanged() {
...@@ -1286,3 +1287,16 @@ ui::ImageModel LocationBarView::GetLocationIcon( ...@@ -1286,3 +1287,16 @@ ui::ImageModel LocationBarView::GetLocationIcon(
std::move(on_icon_fetched)) std::move(on_icon_fetched))
: ui::ImageModel(); : ui::ImageModel();
} }
void LocationBarView::UpdatePermissionChipVisibility() {
if (!permission_chip()->HasActiveRequest()) {
DCHECK(!permission_chip()->GetVisible());
return;
}
if (IsEditingOrEmpty()) {
permission_chip()->Hide();
} else {
permission_chip()->Reshow();
}
}
...@@ -347,6 +347,10 @@ class LocationBarView : public LocationBar, ...@@ -347,6 +347,10 @@ class LocationBarView : public LocationBar,
// Returns true if visibility changed. // Returns true if visibility changed.
bool UpdateSendTabToSelfIcon(); bool UpdateSendTabToSelfIcon();
// Updates the visibility of the permission chip when omnibox is in the edit
// mode.
void UpdatePermissionChipVisibility();
// The Browser this LocationBarView is in. Note that at least // The Browser this LocationBarView is in. Note that at least
// chromeos::SimpleWebViewDialog uses a LocationBarView outside any browser // chromeos::SimpleWebViewDialog uses a LocationBarView outside any browser
// window, so this may be NULL. // window, so this may be NULL.
......
...@@ -102,9 +102,7 @@ PermissionChip::PermissionChip(Browser* browser) ...@@ -102,9 +102,7 @@ PermissionChip::PermissionChip(Browser* browser)
std::make_unique<views::Button::DefaultButtonControllerDelegate>( std::make_unique<views::Button::DefaultButtonControllerDelegate>(
chip_button_))); chip_button_)));
constexpr auto kAnimationDuration = base::TimeDelta::FromMilliseconds(350);
animation_ = std::make_unique<gfx::SlideAnimation>(this); animation_ = std::make_unique<gfx::SlideAnimation>(this);
animation_->SetSlideDuration(kAnimationDuration);
} }
PermissionChip::~PermissionChip() { PermissionChip::~PermissionChip() {
...@@ -113,7 +111,8 @@ PermissionChip::~PermissionChip() { ...@@ -113,7 +111,8 @@ PermissionChip::~PermissionChip() {
CHECK(!IsInObserverList()); CHECK(!IsInObserverList());
} }
void PermissionChip::Show(permissions::PermissionPrompt::Delegate* delegate) { void PermissionChip::DisplayRequest(
permissions::PermissionPrompt::Delegate* delegate) {
DCHECK(delegate); DCHECK(delegate);
delegate_ = delegate; delegate_ = delegate;
...@@ -136,16 +135,14 @@ void PermissionChip::Show(permissions::PermissionPrompt::Delegate* delegate) { ...@@ -136,16 +135,14 @@ void PermissionChip::Show(permissions::PermissionPrompt::Delegate* delegate) {
SetVisible(true); SetVisible(true);
// TODO(olesiamarukhno): Add tests for animation logic. // TODO(olesiamarukhno): Add tests for animation logic.
animation_->Reset(); animation_->Reset();
is_collapsed_ = true;
if (!delegate_->WasCurrentRequestAlreadyDisplayed()) { if (!delegate_->WasCurrentRequestAlreadyDisplayed()) {
animation_->Show(); AnimateExpand();
is_collapsed_ = false;
} }
requested_time_ = base::TimeTicks::Now(); requested_time_ = base::TimeTicks::Now();
PreferredSizeChanged(); PreferredSizeChanged();
} }
void PermissionChip::Hide() { void PermissionChip::FinalizeRequest() {
SetVisible(false); SetVisible(false);
timer_.AbandonAndStop(); timer_.AbandonAndStop();
delegate_ = nullptr; delegate_ = nullptr;
...@@ -155,6 +152,38 @@ void PermissionChip::Hide() { ...@@ -155,6 +152,38 @@ void PermissionChip::Hide() {
PreferredSizeChanged(); PreferredSizeChanged();
} }
void PermissionChip::Reshow() {
if (GetVisible())
return;
SetVisible(true);
// TODO(olesiamarukhno): Add tests for animation logic.
animation_->Reset();
if (!delegate_->WasCurrentRequestAlreadyDisplayed())
AnimateExpand();
PreferredSizeChanged();
}
void PermissionChip::Hide() {
SetVisible(false);
}
void PermissionChip::AnimateCollapse() {
constexpr auto kAnimationDuration = base::TimeDelta::FromMilliseconds(250);
animation_->SetSlideDuration(kAnimationDuration);
animation_->Hide();
}
void PermissionChip::AnimateExpand() {
constexpr auto kAnimationDuration = base::TimeDelta::FromMilliseconds(350);
animation_->SetSlideDuration(kAnimationDuration);
animation_->Show();
}
bool PermissionChip::HasActiveRequest() {
return delegate_;
}
gfx::Size PermissionChip::CalculatePreferredSize() const { gfx::Size PermissionChip::CalculatePreferredSize() const {
const int fixed_width = GetIconSize() + chip_button_->GetInsets().width(); const int fixed_width = GetIconSize() + chip_button_->GetInsets().width();
const int collapsable_width = const int collapsable_width =
...@@ -177,6 +206,7 @@ void PermissionChip::OnThemeChanged() { ...@@ -177,6 +206,7 @@ void PermissionChip::OnThemeChanged() {
void PermissionChip::AnimationEnded(const gfx::Animation* animation) { void PermissionChip::AnimationEnded(const gfx::Animation* animation) {
DCHECK_EQ(animation, animation_.get()); DCHECK_EQ(animation, animation_.get());
is_collapsed_ = animation->GetCurrentValue() != 1.0;
if (animation->GetCurrentValue() == 1.0) if (animation->GetCurrentValue() == 1.0)
StartCollapseTimer(); StartCollapseTimer();
} }
...@@ -190,8 +220,7 @@ void PermissionChip::OnWidgetDestroying(views::Widget* widget) { ...@@ -190,8 +220,7 @@ void PermissionChip::OnWidgetDestroying(views::Widget* widget) {
DCHECK_EQ(widget, prompt_bubble_->GetWidget()); DCHECK_EQ(widget, prompt_bubble_->GetWidget());
widget->RemoveObserver(this); widget->RemoveObserver(this);
prompt_bubble_ = nullptr; prompt_bubble_ = nullptr;
animation_->Hide(); AnimateCollapse();
is_collapsed_ = true;
} }
void PermissionChip::OpenBubble() { void PermissionChip::OpenBubble() {
...@@ -224,8 +253,7 @@ void PermissionChip::Collapse() { ...@@ -224,8 +253,7 @@ void PermissionChip::Collapse() {
if (IsMouseHovered() || prompt_bubble_) { if (IsMouseHovered() || prompt_bubble_) {
StartCollapseTimer(); StartCollapseTimer();
} else { } else {
animation_->Hide(); AnimateCollapse();
is_collapsed_ = true;
} }
} }
......
...@@ -39,9 +39,12 @@ class PermissionChip : public views::View, ...@@ -39,9 +39,12 @@ class PermissionChip : public views::View,
~PermissionChip() override; ~PermissionChip() override;
void Show(permissions::PermissionPrompt::Delegate* delegate); void DisplayRequest(permissions::PermissionPrompt::Delegate* delegate);
void Hide(); void FinalizeRequest();
void OpenBubble(); void OpenBubble();
void Hide();
void Reshow();
bool HasActiveRequest();
views::Button* button() { return chip_button_; } views::Button* button() { return chip_button_; }
bool is_collapsed() { return is_collapsed_; } bool is_collapsed() { return is_collapsed_; }
...@@ -74,6 +77,9 @@ class PermissionChip : public views::View, ...@@ -74,6 +77,9 @@ class PermissionChip : public views::View,
base::string16 GetPermissionMessage(); base::string16 GetPermissionMessage();
const gfx::VectorIcon& GetPermissionIconId(); const gfx::VectorIcon& GetPermissionIconId();
void AnimateCollapse();
void AnimateExpand();
Browser* browser_ = nullptr; Browser* browser_ = nullptr;
permissions::PermissionPrompt::Delegate* delegate_ = nullptr; permissions::PermissionPrompt::Delegate* delegate_ = nullptr;
PermissionPromptBubbleView* prompt_bubble_ = nullptr; PermissionPromptBubbleView* prompt_bubble_ = nullptr;
......
...@@ -71,7 +71,7 @@ PermissionPromptImpl::~PermissionPromptImpl() { ...@@ -71,7 +71,7 @@ PermissionPromptImpl::~PermissionPromptImpl() {
case PermissionPromptStyle::kChip: case PermissionPromptStyle::kChip:
DCHECK(!prompt_bubble_); DCHECK(!prompt_bubble_);
DCHECK(permission_chip_); DCHECK(permission_chip_);
permission_chip_->Hide(); permission_chip_->FinalizeRequest();
permission_chip_ = nullptr; permission_chip_ = nullptr;
break; break;
case PermissionPromptStyle::kQuiet: case PermissionPromptStyle::kQuiet:
...@@ -108,7 +108,7 @@ void PermissionPromptImpl::UpdateAnchorPosition() { ...@@ -108,7 +108,7 @@ void PermissionPromptImpl::UpdateAnchorPosition() {
// If there is fresh pending request shown as chip UI and location bar // If there is fresh pending request shown as chip UI and location bar
// isn't visible anymore, show bubble UI instead. // isn't visible anymore, show bubble UI instead.
if (!permission_chip_->is_collapsed() && !is_location_bar_drawn) { if (!permission_chip_->is_collapsed() && !is_location_bar_drawn) {
permission_chip_->Hide(); permission_chip_->FinalizeRequest();
permission_chip_ = nullptr; permission_chip_ = nullptr;
ShowBubble(); ShowBubble();
} }
...@@ -128,7 +128,7 @@ void PermissionPromptImpl::ShowChipUI() { ...@@ -128,7 +128,7 @@ void PermissionPromptImpl::ShowChipUI() {
DCHECK(lbv); DCHECK(lbv);
permission_chip_ = lbv->permission_chip(); permission_chip_ = lbv->permission_chip();
permission_chip_->Show(delegate_); permission_chip_->DisplayRequest(delegate_);
prompt_style_ = PermissionPromptStyle::kChip; prompt_style_ = PermissionPromptStyle::kChip;
} }
......
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