New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 666589 link

Starred by 7 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocked on:
issue 307091
issue 697057



Sign in to add a comment

Files app: Use native overlay scrollbar.

Project Member Reported by fukino@chromium.org, Nov 18 2016

Issue description

Overlay scrollbar was enabled on Chrome OS.
https://codereview.chromium.org/2480903002

Current Files app draws its scroll bar by itself, but now we can replace the own one with the native scroll bar.
It should be simpler and be more consistent with other parts.
 

Comment 1 by fukino@chromium.org, Nov 18 2016

Cc: yamaguchi@chromium.org oka@chromium.org fukino@chromium.org
Components: Platform>Apps>FileManager
Labels: -Pri-3 OS-Chrome Pri-2
Status: Available (was: Aval)

Comment 2 by fukino@chromium.org, Nov 18 2016

Summary: Files app: Use native overlay scrollbar. (was: Files app: :Use native scroll bar.)
Labels: M-57
Owner: yamaguchi@chromium.org
Status: Assigned (was: Available)
How about yamaguchi@?
Blockedon: 669810 669672 669668
The overlay scrollbar release has several blocking bugs.
Especially, these will to affect the UI of Files app as well, if we use native overlay scrollbar at the moment.
I added those to blocked-on list.

669668 Overlay scrollbar shouldn't fade out while mouse is over it
669672 Disable touch on overlay scrollbars
669810 Overlay scrollbar doesn't appear by hover
Cc: mitsuji@chromium.org sgabr...@chromium.org weifangsun@chromium.org
I found that the scrollbar in Files app differs from the overlay scrollbar in blink by these points:
- In current files app, the scrollbar re-appears when hovering on each div (e.g. the directory tree list pane). With the upcoming change, native scrollbar will never appear without scrolling, resizing, or refreshing the view.
- The thickness of the scrollbar in Files App never change (although it can be hidden), overlay scrollbar is designed to expand when hovered. It shrinks when inactive, before fades out.)

Should we reflect it to the scrollbars in Files app along with this change?
I think it'd make more sense to have it consistent with other places in Chrome.
Yes the files app one should be consistent if possible.
Status: Started (was: Assigned)
IIUC, the goal of this bug is to simplify the code base by eliminating the homemade scrollbar implementation dedicated for Files app.

True "native overlay scrollbar" is only used in 2 places (System menu and notification center) as noted in the change,
https://codereview.chromium.org/2496643002/
but Blink also has overlay scrollbar now, like the one we see in chrome://settings.
I think the goal can be achieved by just using Blink's scrollbar, and I think it'd be much simpler. Therefore I will take this approach first.

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 7 2016

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

commit fa7b28294ab9ee7e07569054d4e65626e2535f4e
Author: yamaguchi <yamaguchi@chromium.org>
Date: Wed Dec 07 05:10:18 2016

Replace homebrew scrollbar in Files app with that of Blink.

When overlay scrollbar is enabled in Chrome, there is animation to shrink and fade out the scrollbar thumb when inactive. However, scroll bars that aren't composited will not be animated
(see  Issue 671644 ). It would make the scroll bar always thick and simply disappear without fading out after inactive.
As a workaround, this change makes the 2 boxes have solid background and contain:paint so as to make sure the scrollers are composited.

BUG= 666589 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2554433002
Cr-Commit-Position: refs/heads/master@{#436868}

[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/css/file_manager.css
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/ui/file_grid.js
[modify] https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[delete] https://crrev.com/472c0e40aeb583e2638eb45c79f7ae931d10bceb/ui/file_manager/file_manager/foreground/js/ui/scrollbar.js

Blockedon: -669810 -669672 -669668
Status: Fixed (was: Started)
Issues  669668 ,  669672 ,  669810  are no longer blockers to fix this issue. Fixes to those bugs will also be reflected to the one in Files app consequently.
Status: Verified (was: Fixed)
Verified on ChromeOS 9089.0.0, 57.0.2952.0. Files app now shows native overlay scrollbar.
Status: Assigned (was: Verified)
https://bugs.chromium.org/p/chromium/issues/detail?id=307091#c124

Overlay scrollbar have been disabled for M57.
We need to revert the change in order to the get the same appearance in the Files app as M56.
Blockedon: 307091
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 26 2017

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

commit 526e1938a4fef1e70e97295715b581d79a51574c
Author: yamaguchi <yamaguchi@chromium.org>
Date: Thu Jan 26 14:39:17 2017

Revert of Replace homebrew scrollbar in Files app with that of Blink. (patchset #3 id:40001 of https://codereview.chromium.org/2554433002/ )

Reason for revert:
Overlay scrollbar in Blink will be disabled on M57.
https://bugs.chromium.org/p/chromium/issues/detail?id=307091#c124

Original issue's description:
> Replace homebrew scrollbar in Files app with that of Blink.
>
> When overlay scrollbar is enabled in Chrome, there is animation to shrink and fade out the scrollbar thumb when inactive. However, scroll bars that aren't composited will not be animated
> (see  Issue 671644 ). It would make the scroll bar always thick and simply disappear without fading out after inactive.
> As a workaround, this change makes the 2 boxes have solid background and contain:paint so as to make sure the scrollers are composited.
>
> BUG= 666589 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Committed: https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e
> Cr-Commit-Position: refs/heads/master@{#436868}

TBR=fukino@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 666589 , 684695 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2654873006
Cr-Commit-Position: refs/heads/master@{#446323}

[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/css/file_manager.css
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/ui/file_grid.js
[modify] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[add] https://crrev.com/526e1938a4fef1e70e97295715b581d79a51574c/ui/file_manager/file_manager/foreground/js/ui/scrollbar.js

Labels: -M-57 M-58
Changed labels from M-57 to M-58 due to the dependency (Issue 307091).
Project Member

Comment 16 by bugdroid1@chromium.org, Feb 6 2017

Labels: merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5

commit e50697019dd8e4092b07607e7cf9f3f4fbc8aef5
Author: yamaguchi <yamaguchi@chromium.org>
Date: Mon Feb 06 02:36:45 2017

Revert of Replace homebrew scrollbar in Files app with that of Blink. (patchset #3 id:40001 of https://codereview.chromium.org/2554433002/ )

Reason for revert:
Overlay scrollbar in Blink will be disabled on M57.
https://bugs.chromium.org/p/chromium/issues/detail?id=307091#c124

Original issue's description:
> Replace homebrew scrollbar in Files app with that of Blink.
>
> When overlay scrollbar is enabled in Chrome, there is animation to shrink and fade out the scrollbar thumb when inactive. However, scroll bars that aren't composited will not be animated
> (see  Issue 671644 ). It would make the scroll bar always thick and simply disappear without fading out after inactive.
> As a workaround, this change makes the 2 boxes have solid background and contain:paint so as to make sure the scrollers are composited.
>
> BUG= 666589 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Committed: https://crrev.com/fa7b28294ab9ee7e07569054d4e65626e2535f4e
> Cr-Commit-Position: refs/heads/master@{#436868}

TBR=fukino@chromium.org
BUG= 666589 , 684695 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2654873006
Cr-Commit-Position: refs/heads/master@{#446323}
(cherry picked from commit 526e1938a4fef1e70e97295715b581d79a51574c)

NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2657143002
Cr-Commit-Position: refs/branch-heads/2987@{#315}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/css/file_manager.css
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/ui/file_grid.js
[modify] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[add] https://crrev.com/e50697019dd8e4092b07607e7cf9f3f4fbc8aef5/ui/file_manager/file_manager/foreground/js/ui/scrollbar.js

Labels: -M-58 M-59
Punting to M59, because the dependency issue 307091 has been punted to M59.
Blockedon: 697057
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 29 2017

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

commit c4faf5fee6fab54abd2e49a5e2170434c0440914
Author: yamaguchi <yamaguchi@chromium.org>
Date: Wed Mar 29 05:41:44 2017

Reland of "Replace homebrew scrollbar in Files app with that of Blink."

This changelist is based on the revert patch of commit e50697019dd8e4092b07607e7cf9f3f4fbc8aef5.

BUG= 666589 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2782063002
Cr-Commit-Position: refs/heads/master@{#460290}

[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/css/file_manager.css
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/ui/compiled_resources2.gyp
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/ui/file_grid.js
[modify] https://crrev.com/c4faf5fee6fab54abd2e49a5e2170434c0440914/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[delete] https://crrev.com/5ec1039bc225dfb4221c0c5f387a37cd87fc33a1/ui/file_manager/file_manager/foreground/js/ui/scrollbar.js

Status: Fixed (was: Assigned)
I think  Issue 697057  is less important than this issue, because that only happens with changing about:flags setting from the default value explicitly.
Status: Verified (was: Fixed)
9460.5.0 / 59.0.3071.15
Project Member

Comment 23 by bugdroid1@chromium.org, Jun 1 2017

Labels: merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47e91004c618d26d32ce0a7d1803720affa72489

commit 47e91004c618d26d32ce0a7d1803720affa72489
Author: yamaguchi <yamaguchi@chromium.org>
Date: Thu Jun 01 00:53:16 2017

M59: Revert of "Replace homebrew scrollbar in Files app with that of Blink." (patchset #3 id:40001 of https://codereview.chromium.org/2782063002/ )

Reason for revert:
We don't enable native overlay scrollbar by default on M59.
The Files app should continue using the older scrollbar that was
built for the app.

Original issue's description:
> Reland of "Replace homebrew scrollbar in Files app with that of Blink."
>
> This changelist is based on the revert patch of commit e50697019dd8e4092b07607e7cf9f3f4fbc8aef5.
>
>
> BUG= 666589 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Review-Url: https://codereview.chromium.org/2782063002
> Cr-Commit-Position: refs/heads/master@{#460290}
> Committed: https://chromium.googlesource.com/chromium/src/+/c4faf5fee6fab54abd2e49a5e2170434c0440914

# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 684695 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2914573003
Cr-Commit-Position: refs/branch-heads/3071@{#733}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/css/file_manager.css
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/compiled_resources.gyp
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/main_scripts.js
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/compiled_resources2.gyp
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/file_grid.js
[modify] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/file_table.js
[add] https://crrev.com/47e91004c618d26d32ce0a7d1803720affa72489/ui/file_manager/file_manager/foreground/js/ui/scrollbar.js

Sign in to add a comment