From fa7ec92c672dfa4a08f395ef9aa6d1e63e03f362 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Tue, 7 Jul 2015 08:44:29 -0700 Subject: [PATCH] Make it harder to forget itemCount for FixedHeightScrollable Previously, if a subclass of FixedHeightScrollable didn't set itemCount, everything would silently work except that you wouldn't actully be able to scroll. This CL makes it harder to forget to provide the itemCount by making it an abstract getter. Now the analyzer will flag it as missing in subclasses and we'll throw an exception at runtime if you forget it. R=ianh@google.com, tonyg@google.com Review URL: https://codereview.chromium.org/1219933004 . --- sdk/example/demo_launcher/lib/main.dart | 6 ++-- sdk/example/stocks/lib/stock_home.dart | 24 ++++++++++++---- sdk/example/stocks/lib/stock_list.dart | 18 ++++-------- sdk/lib/widgets/fixed_height_scrollable.dart | 30 ++++++++++---------- 4 files changed, 41 insertions(+), 37 deletions(-) diff --git a/sdk/example/demo_launcher/lib/main.dart b/sdk/example/demo_launcher/lib/main.dart index 458f204c878..139325479f4 100644 --- a/sdk/example/demo_launcher/lib/main.dart +++ b/sdk/example/demo_launcher/lib/main.dart @@ -129,9 +129,9 @@ const double kCardHeight = 120.0; const EdgeDims kListPadding = const EdgeDims.all(4.0); class DemoList extends FixedHeightScrollable { - DemoList({ String key }) : super(key: key, itemHeight: kCardHeight, padding: kListPadding) { - itemCount = demos.length; - } + DemoList({ String key }) : super(key: key, itemHeight: kCardHeight, padding: kListPadding); + + int get itemCount => demos.length; Widget buildDemo(SkyDemo demo) { return new Listener( diff --git a/sdk/example/stocks/lib/stock_home.dart b/sdk/example/stocks/lib/stock_home.dart index b26e392780a..7adb18f314a 100644 --- a/sdk/example/stocks/lib/stock_home.dart +++ b/sdk/example/stocks/lib/stock_home.dart @@ -164,7 +164,7 @@ class StockHome extends StatefulComponent { _drawerController.close(); }); } - + Widget buildToolBar() { return new ToolBar( left: new IconButton( @@ -185,18 +185,30 @@ class StockHome extends StatefulComponent { int selectedTabIndex = 0; List portfolioSymbols = ["AAPL","FIZZ", "FIVE", "FLAT", "ZINC", "ZNGA"]; + Iterable _filterByPortfolio(Iterable stocks) { + return stocks.where((stock) => portfolioSymbols.contains(stock.symbol)); + } + + Iterable _filterBySearchQuery(Iterable stocks) { + if (_searchQuery == null) + return stocks; + RegExp regexp = new RegExp(_searchQuery, caseSensitive: false); + return stocks.where((stock) => stock.symbol.contains(regexp)); + } + + Widget buildMarketStockList() { + return new Stocklist(stocks: _filterBySearchQuery(stocks).toList()); + } + Widget buildPortfolioStocklist() { - return new Stocklist( - stocks: stocks.where((s) => portfolioSymbols.contains(s.symbol)).toList(), - query: _searchQuery - ); + return new Stocklist(stocks: _filterBySearchQuery(_filterByPortfolio(stocks)).toList()); } Widget buildTabNavigator() { List views = [ new TabNavigatorView( label: const TabLabel(text: 'MARKET'), - builder: () => new Stocklist(stocks: stocks, query: _searchQuery) + builder: buildMarketStockList ), new TabNavigatorView( label: const TabLabel(text: 'PORTFOLIO'), diff --git a/sdk/example/stocks/lib/stock_list.dart b/sdk/example/stocks/lib/stock_list.dart index 4b36783d186..6fd63806fb8 100644 --- a/sdk/example/stocks/lib/stock_list.dart +++ b/sdk/example/stocks/lib/stock_list.dart @@ -10,28 +10,20 @@ import 'stock_row.dart'; class Stocklist extends FixedHeightScrollable { - Stocklist({ - String key, - this.stocks, - this.query - }) : super(itemHeight: StockRow.kHeight, key: key); + Stocklist({ String key, this.stocks }) + : super(itemHeight: StockRow.kHeight, key: key); - String query; List stocks; + int get itemCount => stocks.length; + void syncFields(Stocklist source) { - query = source.query; stocks = source.stocks; super.syncFields(source); } List buildItems(int start, int count) { - var filteredStocks = stocks.where((stock) { - return query == null || - stock.symbol.contains(new RegExp(query, caseSensitive: false)); - }); - itemCount = filteredStocks.length; - return filteredStocks + return stocks .skip(start) .take(count) .map((stock) => new StockRow(stock: stock)) diff --git a/sdk/lib/widgets/fixed_height_scrollable.dart b/sdk/lib/widgets/fixed_height_scrollable.dart index 1a0997cf7f1..81cdb20cc9b 100644 --- a/sdk/lib/widgets/fixed_height_scrollable.dart +++ b/sdk/lib/widgets/fixed_height_scrollable.dart @@ -12,16 +12,20 @@ import 'scrollable.dart'; abstract class FixedHeightScrollable extends Scrollable { - final EdgeDims padding; - FixedHeightScrollable({ String key, this.itemHeight, Color backgroundColor, this.padding }) : super(key: key, backgroundColor: backgroundColor) { assert(itemHeight != null); } + EdgeDims padding; double itemHeight; + /// Subclasses must implement `get itemCount` to tell FixedHeightScrollable + /// how many items there are in the list. + int get itemCount; + void syncFields(FixedHeightScrollable source) { + padding = source.padding; itemHeight = source.itemHeight; super.syncFields(source); } @@ -29,18 +33,6 @@ abstract class FixedHeightScrollable extends Scrollable { ScrollBehavior createScrollBehavior() => new OverscrollBehavior(); OverscrollBehavior get scrollBehavior => super.scrollBehavior; - int _itemCount = 0; - int get itemCount => _itemCount; - void set itemCount (int value) { - if (_itemCount != value) { - _itemCount = value; - double contentsHeight = itemHeight * _itemCount; - if (padding != null) - contentsHeight += padding.top + padding.bottom; - scrollBehavior.contentsHeight = contentsHeight; - } - } - double _height; void _handleSizeChanged(Size newSize) { setState(() { @@ -57,10 +49,18 @@ abstract class FixedHeightScrollable extends Scrollable { return super.scrollTo(newScrollOffset); } + void _updateContentsHeight() { + double contentsHeight = itemHeight * itemCount; + if (padding != null) + contentsHeight += padding.top + padding.bottom; + scrollBehavior.contentsHeight = contentsHeight; + } + Widget buildContent() { + _updateContentsHeight(); + var itemShowIndex = 0; var itemShowCount = 0; - Matrix4 transform = new Matrix4.identity(); if (_height != null && _height > 0.0) {