New issue
Advanced search Search tips

Issue 861975 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Use a circular flood-fill inkdrop for bubble close button

Project Member Reported by pbos@chromium.org, Jul 9

Issue description

Secondary UI bubbles should have close-button style that matches ToolbarButton and other inkdrop surfaces (e.g. use a floodfill).

Related issue on Mac seen here where the square inkdrop changes to a circle, which is instant on Mac. It should instead instantly fill a circle w/ appropriate size: https://bugs.chromium.org/p/chromium/issues/detail?id=851984#c5
 
Cc: bettes@chromium.org
bettes@: I assume this inkdrop can / should be circular and match the newer Refresh-style ones?
close-x-inkdrop.png
12.0 KB View Download
Labels: Hotlist-Polish
Labels: Group-Toolbar
pbos: That's correct. Thank you!
Labels: M-70 Target-70
Cc: pbos@chromium.org
Owner: cyan@chromium.org
Cc: cyan@chromium.org
Owner: pbos@chromium.org
Labels: Proj-DesktopUI
Labels: Pri-1
Labels: -M-70 -Target-70 M-71 Target-71
Labels: Hotlist-MdRefreshDesignPolish
Labels: -Proj-MdRefresh
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e02b4364e6420e9b9cba7698be80c516a33c9c6b

commit e02b4364e6420e9b9cba7698be80c516a33c9c6b
Author: Peter Boström <pbos@chromium.org>
Date: Tue Sep 25 00:51:52 2018

Use highlight path for InkDropMask and FocusRing

Introduces a kHighlightPathKey property to views and uses it as a
default for FocusRing and CreateInkDropMask in InkDropHostView. If no
highlight path is added this fall backs to prior behavior, which allows
introducing this change in parts.

This highlight path is added to the bookmarks-bar buttons and the dialog
close button. CreateInkDropMask and installing the focus ring is removed
from the prior.

Bug:  chromium:861975 , chromium:888204
Change-Id: I7eb682efe82caea7e8088d7277733698967d0234
Reviewed-on: https://chromium-review.googlesource.com/1239715
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593783}
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/chrome/browser/ui/views/bookmarks/bookmark_bar_view.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/animation/ink_drop_host_view.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/animation/ink_drop_mask.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/animation/ink_drop_mask.h
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/bubble/bubble_frame_view.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/controls/focus_ring.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/controls/focus_ring.h
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/view_properties.cc
[modify] https://crrev.com/e02b4364e6420e9b9cba7698be80c516a33c9c6b/ui/views/view_properties.h

Status: Fixed (was: Assigned)
Cc: phanindra.mandapaka@chromium.org
Labels: TE-Verified-M71 TE-Verified-71.0.3561.0
Able to reproduce the issue on chrome version 69.0.3487.0 (build without fix) as per the comment #0.
Verified the fix on Mac 10.13.6, Windows 10 and Ubuntu 14.04 using Chrome version # 71.0.3561.0 
Attaching screen-cast for reference.
Observed that  "lose-button style that matches ToolbarButton and other inkdrop surfaces" 
The fix is working as expected, adding Verified labels

Thanks...!
861975.mp4
2.9 MB View Download
Project Member

Comment 16 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93ca9993020d49cea34bde56e503726bccada8d2

commit 93ca9993020d49cea34bde56e503726bccada8d2
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Sep 26 12:37:00 2018

Add declaration for a newly defined highlight path property

A missing declaration broke some jumbo builds because in those
builds an implicit instantiation was seen before an explicit
specialization. This is illegal in C++. A forward declaration
solves the problem.

TBR=pbos@chromium.org,tapted@chromium.org

Bug:  861975 ,888204
Change-Id: Iecf736bcf9748d3c68265c21f9351c56d9a23dd8
Reviewed-on: https://chromium-review.googlesource.com/1245785
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#594289}
[modify] https://crrev.com/93ca9993020d49cea34bde56e503726bccada8d2/ui/views/view_properties.h

Sign in to add a comment