Scrollable TabBar Version 0

Scrollable TabBar Version 0

TODO:
- Make fling-scrolling work
- Tune scrolling performance
- When a tab is selected, snap it to the center if possible

R=abarth@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/989d3289c66b29c7d48dce78b3153ea2a8487324

[hansmuller, 2015-07-02 00:00:18]

Patchset #1 (id:1) has been deleted

[hansmuller, 2015-07-02 00:03:34]

Patchset #1 (id:20001) has been deleted

[hansmuller, 2015-07-02 00:04:09]

hansmuller@chromium.org changed reviewers:
+ hansmuller@chromium.org

[hansmuller, 2015-07-02 00:04:10]

PTAL

[abarth-chromium, 2015-07-02 15:27:20]

abarth@chromium.org changed reviewers:
+ abarth@chromium.org

[abarth-chromium, 2015-07-02 15:27:21]

lgtm

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/scr...
File sky/sdk/lib/widgets/scrollable.dart (right):

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/scr...
sky/sdk/lib/widgets/scrollable.dart:23: }
eventVelocity.clamp(-config.kMaxFlingVelocity, config.kMaxFlingVelocity) /
_kMillisecondsPerSecond

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
File sky/sdk/lib/widgets/tabs.dart (right):

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:118: double width = scrollable ? maxWidth :
maxWidth * childCount;
Why is this different when we're scrollable?

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:139: child.layout(tabConstraints, parentUsesSize:
onLayoutChanged != null);
We don't really use the child size, even when onLayoutChanged != null.  Instead
of us passing performLayout: true here, maybe the logic for performLayout()
should know that fixed-width tabs have a width that's easy to compute... 
Passing performLayout: true is expensive.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:177: if (size != layoutSize || widths !=
layoutWidths) {
We should add an assert(childIndex == widths.length) to make sure we actually
fill up |widths|.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:199: reportLayoutChanged();
reportLayoutChangedIfNeeded()

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:416: return new Transform(child: tabBarWrapper,
transform: transform);
Don't we need a clip too?

[hansmuller, 2015-07-06 17:21:46]

Thanks for the feedback.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/scr...
File sky/sdk/lib/widgets/scrollable.dart (right):

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/scr...
sky/sdk/lib/widgets/scrollable.dart:23: }
On 2015/07/02 15:27:21, abarth-chromium wrote:
> eventVelocity.clamp(-config.kMaxFlingVelocity, config.kMaxFlingVelocity) /
> _kMillisecondsPerSecond

Done.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
File sky/sdk/lib/widgets/tabs.dart (right):

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:118: double width = scrollable ? maxWidth :
maxWidth * childCount;
On 2015/07/02 15:27:21, abarth-chromium wrote:
> Why is this different when we're scrollable?

When we're scrollable, I'm assuring that the widest tab label will fit. For
example if the labels were "a", "b", "cdef", "g", the TabBar needs to be as wide
as "cdef", since you can scroll to see all of the tabs.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:139: child.layout(tabConstraints, parentUsesSize:
onLayoutChanged != null);
On 2015/07/02 15:27:21, abarth-chromium wrote:
> We don't really use the child size, even when onLayoutChanged != null. 
Instead
> of us passing performLayout: true here, maybe the logic for performLayout()
> should know that fixed-width tabs have a width that's easy to compute... 
> Passing performLayout: true is expensive.

In this case onLayoutChanged is typically null, but you're right. I've updated
reportLayoutChangedIfNeeded() to compute the fixed tab width, and used
parentUsesSize: false here.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:177: if (size != layoutSize || widths !=
layoutWidths) {
On 2015/07/02 15:27:21, abarth-chromium wrote:
> We should add an assert(childIndex == widths.length) to make sure we actually
> fill up |widths|.

Done.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:199: reportLayoutChanged();
On 2015/07/02 15:27:21, abarth-chromium wrote:
> reportLayoutChangedIfNeeded()

Done.

https://codereview.chromium.org/1221673006/diff/40001/sky/sdk/lib/widgets/tab...
sky/sdk/lib/widgets/tabs.dart:416: return new Transform(child: tabBarWrapper,
transform: transform);
On 2015/07/02 15:27:21, abarth-chromium wrote:
> Don't we need a clip too?

I would have thought so. Adding a ClipRect parent didn't appear to have any
effect (the TabBar's contents are clipped anyway), so I took it out.

I do not understand this :-).

[hansmuller1, 2015-07-06 17:23:41]

Committed patchset #2 (id:60001) manually as
989d3289c66b29c7d48dce78b3153ea2a8487324 (presubmit successful).