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

Issue 651417 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 638807



Sign in to add a comment

Update media controls pop up menu style

Project Member Reported by mlamouri@chromium.org, Sep 29 2016

Issue description

It should be a bit lower, has round corner and a box shadow.
 

Comment 1 by rolfe@chromium.org, Sep 30 2016

Pulling all the relevant specs from the Clank MD folder...
Overflow menu
https://drive.google.com/file/d/0B6x6iYCtKinEaWVLZkxsU2ZfNHc/view
Tap switcher overflow menu
https://drive.google.com/file/d/0B6x6iYCtKinEaFhsS25CX1h4R2M/view

V1 spec with CC menu details:
https://drive.google.com/file/d/0B-Vmdj5n-YddX2RpeHlqbzZRbzg/view
rachelis@ made a V2 one in that same folder that doesn't have spec notes on the CC menu but I think it might have been an accidental layer-turned-off-thing. I'll leave it to her to determine which of the two files to use and delete the other, then update the bug here if this link needs updating.

Hoping you can use the overflow menu nine-patch with the shadow (and I believe, the corner radius.) If you need specifics here I can check with bettes@, our new visual designer contact, on Monday.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 1 2016

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

commit 62fa917d17ee2358ab4c9c747318eafa897400e7
Author: mlamouri <mlamouri@chromium.org>
Date: Sat Oct 01 15:33:05 2016

Update style of media controls popup.

This is:
- changing the position of the popup.
- adding round corner.
- adding a shadow.

It applies to both the overflow and CC menu.

BUG= 651417 

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

[modify] https://crrev.com/62fa917d17ee2358ab4c9c747318eafa897400e7/third_party/WebKit/Source/core/css/mediaControlsNew.css

Status: Fixed (was: Started)
Cc: rolfe@chromium.org
Closing this but there might be some polish required. The CL that has landed should be closer to the spec but the details might not be respected. Though at that point it should be about changing the value of a colour or the intensity of the shadow, probably no major changes that can't be sherry-picked.

Comment 5 by rolfe@chromium.org, Oct 3 2016

Just for posterity, specs from bettes@ if you need them:
- corner radius = 2dp
- shadow is "--shadow-elevation-8dp" on this page
https://github.com/PolymerElements/paper-styles/blob/master/shadow.html (or visual version here: https://elements.polymer-project.org/elements/paper-styles?view=demo:demo/index.html&active=paper-styles)
Thanks Mounir!

I've attached a screenshot of how this currently looks.

Polish details I see:
- 2dp corners (this is bigger)
- shadow adjustments (see above comment)
- we should add the material touch animation on tap

It sounds like you're testing out these adjustments now.
overflow-menu-media-controls.png
331 KB View Download
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5 2016

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

commit 05be21172a2fde5ddf35f36d1932d8f3f94f6695
Author: mlamouri <mlamouri@chromium.org>
Date: Wed Oct 05 16:49:35 2016

Polish overflow menu style.

Changes the border-radius and box-shadow style.

BUG= 651417 
R=foolip@chromium.org

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

[modify] https://crrev.com/05be21172a2fde5ddf35f36d1932d8f3f94f6695/third_party/WebKit/Source/core/css/mediaControlsNew.css

"- we should add the material touch animation on tap"

We currently don't use the material touch animation for the controls. We should probably file this as a future improvement.

Comment 9 by rolfe@chromium.org, Oct 5 2016

Hey mounir@ - rachelis@ and I were talking a bit about the shadow. It's not a deal-breaker but it seems a bit tighter than the current omnibox shadow. rachelis@ tells me you had to take the polymer script and translate to CSS. What did you end up on? I could fiddle with it to get it where it feels better. Or if you have ideas feel free to fiddle too! I'd try doubling the size of what you have now, which is the totally ambiguous designer feedback that the mass populace of users won't really care about, but a detail we feel obligated to sweat to ensure our job security.

Attaching a comparison image for reference.
MenuComparison.png
513 KB View Download
I'm going to apply your suggestion and send a screenshot. A few comments/clarifications though:
- the media controls are implemented in Blink and use CSS for the styling (contrary to the menu you show that is probably implemented in Java)
- this feature is desktop/mobile and at the moment we use the same style (maybe we shouldn't?)
- the shadow seems consistent with the omnibox icon shadow on desktop
- I'm afraid that having something bigger will make the overflow menu over power the video -- but we will see :)
I made the border radius smaller and the box-shadow bigger.

It changed from:
    border-radius: 2px;
    box-shadow: 0 8px 10px 1px rgba(0, 0, 0, 0.14),
                0 3px 14px 2px rgba(0, 0, 0, 0.12),
                0 5px 5px -3px rgba(0, 0, 0, 0.4);

to:
    border-radius: 1px;
    box-shadow: 0 8px 20px 1px rgba(0, 0, 0, 0.14),
                0 3px 28px 2px rgba(0, 0, 0, 0.12),
                0 5px 10px -3px rgba(0, 0, 0, 0.4);
Screenshot_2016-10-06-15-57-07.png
174 KB View Download
Looks good to me - thanks! rachelis@ - will let you have final say though.
Looks great! Thanks!
Rachel, do you want me to land this change?
Yes, please do.
It has landed on trunk. It should be merged in Chrome Beta.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 27 2016

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

commit 05be21172a2fde5ddf35f36d1932d8f3f94f6695
Author: mlamouri <mlamouri@chromium.org>
Date: Wed Oct 05 16:49:35 2016

Polish overflow menu style.

Changes the border-radius and box-shadow style.

BUG= 651417 
R=foolip@chromium.org

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

[modify] https://crrev.com/05be21172a2fde5ddf35f36d1932d8f3f94f6695/third_party/WebKit/Source/core/css/mediaControlsNew.css

Comment 18 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment