New issue
Advanced search Search tips

Issue 635176 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 630179
issue 630357
issue 635166



Sign in to add a comment

Harmony - TabbedPane

Project Member Reported by est...@chromium.org, Aug 5 2016

Issue description

Need to implement an MD version of TabbedPane.
 
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)

Comment 3 by est...@chromium.org, Aug 26 2016

I would assume we want to animate the underline? (I think I've seen that on Android.)
Status: Started (was: Assigned)
WIP CL: https://codereview.chromium.org/2297193002/

tabbed-pane-old: no --secondary-ui-md, current "default" look
tabbed-pane-new: with --secondary-ui-md, Harmony look
Forgot to actually attach the two files :)
tabbed-pane-old.png
13.8 KB View Download
tabbed-pane-new.png
13.9 KB View Download
Cc: ellyjo...@chromium.org
 Issue 605590  has been merged into this issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 31 2016

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

commit 1968a3e844f82ecac7e14d97747db1a68a96dadf
Author: ellyjones <ellyjones@chromium.org>
Date: Wed Aug 31 20:09:41 2016

Harmony: implement TabbedPane styling

This CL:
1) Adds MdTabStrip and MdTab as subclasses of TabStrip and Tab;
2) Uses those classes to implement the Harmony visuals.

BUG= 635176 

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

[modify] https://crrev.com/1968a3e844f82ecac7e14d97747db1a68a96dadf/ui/views/controls/tabbed_pane/tabbed_pane.cc
[modify] https://crrev.com/1968a3e844f82ecac7e14d97747db1a68a96dadf/ui/views/controls/tabbed_pane/tabbed_pane.h

The remaining open question is whether to animate the underline when the state changes or not. I will send mail to the harmony group about it.
I've updated the spec to include keyboard focus (using the same pattern as we do for buttons) as well as ripples (using the same curves and visuals as Core UI). 

https://folio.googleplex.com/chrome-ux-specs-and-sources/Chrome%20browser%20(MD)/Secondary%20UI%20Previews%20and%20specs%20(exports)#%2FSPEC-secondary-UI-07-tabs.png%3Fz=width
SPEC-secondary-UI-07-tabs.png
107 KB View Download
Note that the focus ring on tabs has the outline drawn on the inside, and not on the outside as it is done for buttons. 
Also, a few things to note on your screenshots in #5

- The selected tab underline seems to be #4285F4, while the selected tab text looks darker. They should be the same value. 

- Can you confirm that all tab text has a medium font-weight?
#11:

Tab text is all medium font-weight, except that the selected tab text on Windows is bold font-weight instead.

I'll double-check the selected tab text but I think it's using the same color.
Project Member

Comment 13 by bugdroid1@chromium.org, Sep 7 2016

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

commit 30699ab305dc5f99307a44b59ecc50c58220e274
Author: ellyjones <ellyjones@chromium.org>
Date: Wed Sep 07 17:51:32 2016

views: use MEDIUM for TabbedPane tabs, not NORMAL

BUG= 635176 

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

[modify] https://crrev.com/30699ab305dc5f99307a44b59ecc50c58220e274/ui/views/controls/tabbed_pane/tabbed_pane.cc

#9: Looking over the spec to make sure I got everything, the section on animation says "according to polymer's paper-tabs guidelines" - do you know where these are?

The other missing feature is focus rings, it seems; they don't appear to be drawn right now.
Labels: -M-55 Proj-HarmonyControls M-56
Project Member

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

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

commit 5974cb6cb046616fdbff93b96cdbdcf9bb5e297e
Author: ellyjones <ellyjones@chromium.org>
Date: Fri Oct 07 20:58:20 2016

views: add focus to TabbedPane

This change:
1) Makes TabbedPane draw a focus ring on the selected tab when the
   containing TabbedPane has focus;
2) Adds bindings for left/right arrow keys to switch between tabs

BUG= 635176 

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

[modify] https://crrev.com/5974cb6cb046616fdbff93b96cdbdcf9bb5e297e/ui/views/controls/tabbed_pane/tabbed_pane.cc
[modify] https://crrev.com/5974cb6cb046616fdbff93b96cdbdcf9bb5e297e/ui/views/controls/tabbed_pane/tabbed_pane.h
[modify] https://crrev.com/5974cb6cb046616fdbff93b96cdbdcf9bb5e297e/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc
[modify] https://crrev.com/5974cb6cb046616fdbff93b96cdbdcf9bb5e297e/ui/views/focus/focus_traversal_unittest.cc

Project Member

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

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

commit 2941cbd2032c2cc48b610f7b529a1060bbe4f089
Author: ellyjones <ellyjones@chromium.org>
Date: Thu Oct 13 10:21:05 2016

TabbedPane: refactor away manual layout and selected_tab_index_

This change:
1) Makes TabStrip use a LayoutManager and removes TabStrip::Layout;
2) Makes TabStrip use an EmptyBorder for insets and removes most of
   TabStrip::GetPreferredSize;
3) Replaces TabStrip and MdTabStrip's OnPaint overrides with
   OnPaintBorder overrides to paint their custom borders;
4) Removes the concept of a selected tab index from TabbedPane,
   replacing it with directly querying the TabStrip;
5) Renames TabbedPane::selected_tab_index() to
   TabbedPane::GetSelectedTabIndex() and changes all uses of it.

BUG= 635176 

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

[modify] https://crrev.com/2941cbd2032c2cc48b610f7b529a1060bbe4f089/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views.cc
[modify] https://crrev.com/2941cbd2032c2cc48b610f7b529a1060bbe4f089/ui/views/controls/tabbed_pane/tabbed_pane.cc
[modify] https://crrev.com/2941cbd2032c2cc48b610f7b529a1060bbe4f089/ui/views/controls/tabbed_pane/tabbed_pane.h
[modify] https://crrev.com/2941cbd2032c2cc48b610f7b529a1060bbe4f089/ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc
[modify] https://crrev.com/2941cbd2032c2cc48b610f7b529a1060bbe4f089/ui/views/examples/tabbed_pane_example.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 13 2016

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

commit 8002657452fc6b81b73ce8c77713b938080c9a2d
Author: ellyjones <ellyjones@chromium.org>
Date: Thu Oct 13 18:24:35 2016

views: add selection change animation to TabbedPane

The Harmony spec calls for these to follow the Paper Tab animations,
which are as follows:

1) Over 0.15s, the selection bar grows to encompass the new selection,
   growing along the cubic bezier curve (0.4, 0.0, 1.0, 1.0), which matches
   gfx::Tween::FAST_OUT_LINEAR_IN;
2) Over 0.18s, the selection bar shrinks to encompass only the new
   selection, shrinking along the cubic bezier curve (0.0, 0.0, 0.2, 1.0),
   which matches gfx::Tween::LINEAR_OUT_SLOW_IN.

This change:
1) Removes border painting from MdTab altogether;
2) Changes MdTabStrip to paint the current selection bar;
3) Introduces TabStrip::OnSelectedTabChanged() as a callback for
   TabStrip to learn when the selected Tab has changed;
4) Adds expand and contract animations to MdTabStrip;
5) Grows and shrinks the selection bar in accordance with those
   animations.

BUG= 635176 

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

[modify] https://crrev.com/8002657452fc6b81b73ce8c77713b938080c9a2d/ui/views/controls/tabbed_pane/tabbed_pane.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 7 2016

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

commit 4ae1b75dca27b4a7d8e7e82b4f2f586e3251dd0f
Author: ellyjones <ellyjones@chromium.org>
Date: Mon Nov 07 18:48:22 2016

views: remove obsolete TabbedPane vertical padding

This was removed some weeks ago in an update to the Harmony spec,
but the code was never updated to match.

BUG= 635176 

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

[modify] https://crrev.com/4ae1b75dca27b4a7d8e7e82b4f2f586e3251dd0f/ui/views/controls/tabbed_pane/tabbed_pane.cc

Judging from the commits here, it sounds like this is mostly done.  Is there more to do?
Status: Fixed (was: Started)
This is done.

Sign in to add a comment