Add TabNavigator

sky/sdk/lib/widgets/tabs.dart

Index: sky/sdk/lib/widgets/tabs.dart
diff --git a/sky/sdk/lib/widgets/tabs.dart b/sky/sdk/lib/widgets/tabs.dart
index 09d03be9df8a2b0fa35f882b0701c291a97318b1..c1372cc4645f198ba9eed03f67510378997928ef 100644
--- a/sky/sdk/lib/widgets/tabs.dart
+++ b/sky/sdk/lib/widgets/tabs.dart
@@ -299,3 +299,58 @@ class TabBar extends Component {
     );
   }
 }
+
+class TabNavigatorView {
+  TabNavigatorView({ this.label, this.content });
+
+  final TabLabel label;
+  final Function content;

[Hixie, 2015/06/27 00:12:38]

Function doesn't have a return type. Make a typedef for this (we may already
have one).

Also, call it "builder".

[hansmuller, 2015/06/29 16:49:42]

Thanks, that's better than the names I had in mind.

We do have typedefs for Builder:

sky/sdk/lib/widgets/navigator.dart:typedef Widget Builder(Navigator navigator,
RouteBase route);
sky/sdk/lib/widgets/widget.dart:typedef Widget Builder();

I'll use the one from widget.dart however there may be an impending collision
here.

+
+  Widget buildContent() {
+    return content != null ? content() : null;

[abarth-chromium, 2015/06/27 00:27:36]

Should we just build unconditionally?  What does it mean for buildContent to
return null?  We don't let normal build() functions return null.

[hansmuller, 2015/06/29 16:49:42]

I suppose it could mean that there's no view for the corresponding tab. I didn't
really intend to support that case right now, so I'll just assert that builder
(nee content) and builder() are non null.

+  }
+}
+
+class TabNavigator extends Component {
+  TabNavigator({
+    String key,
+    this.views,
+    this.selectedIndex: 0,
+    this.onChanged
+  }) : super(key: key, stateful: true);
+
+  List<TabNavigatorView> views;
+  int selectedIndex;
+  SelectedIndexChanged onChanged;
+
+  void syncFields(TabNavigator source) {
+    views = source.views;
+    selectedIndex = source.selectedIndex;
+    onChanged = source.onChanged;
+  }
+
+  void _selectedTabIndexChanged(int tabIndex) {
+    setState(() {
+      selectedIndex = tabIndex;
+    });
+    if (onChanged != null)
+      onChanged(selectedIndex);

[abarth-chromium, 2015/06/27 00:27:36]

I'm not sure it makes sense to do both of these.  What component is the
authoritative source for the selected index?  The setState call implies that
this component is the authority.  However, calling onChanged and receiving
selectedIndex in the constructor implies that it's this component's parent that
is authoritative.

What's I'd recommend is making this component stateless (i.e., remove the
setState call) and make the parent component authoritative for selectedIndex.

+  }
+
+  Widget build() {
+    assert(views != null && views.isNotEmpty);
+    assert(selectedIndex >= 0 && selectedIndex < views.length);
+
+    TabBar tabBar = new TabBar(
+      labels: views.map((view) => view.label).toList(),

[Hixie, 2015/06/27 00:12:38]

Why toList()?

[hansmuller, 2015/06/29 16:49:42]

I've changed the type of TabBar.labels to Iterable<TabLabel>.

+      onChanged: _selectedTabIndexChanged,

[abarth-chromium, 2015/06/27 00:27:36]

If you make that change, you can just pass onChanged directly through here
without needing to hook the callback in this component.

+      selectedIndex: selectedIndex
+    );
+
+    Widget content = views[selectedIndex].buildContent();
+    return new Flex(
+      <Widget>[tabBar, new Flexible(child: content)],

[Hixie, 2015/06/27 00:12:38]

We don't usually bother type-annotating the children lists we pass to Flex(),
because that would be too awkward everywhere. I guess you should remove the
annotation. :-(

[hansmuller, 2015/06/29 16:49:41]

OK. As you know, without the annotation code like this compiles and passes
muster with the analyzer:

new Flex(["I am not a widget"]);

+      direction: FlexDirection.vertical
+    );
+  }
+}