From 4f4e161063328da71e04dfda062c1b106d07d255 Mon Sep 17 00:00:00 2001 From: Nikolas Rimikis Date: Thu, 2 Nov 2023 15:37:46 +0100 Subject: [PATCH] perf(neon_news): do not use the spread operator for building lists Signed-off-by: Nikolas Rimikis --- .../neon/neon_news/lib/dialogs/add_feed.dart | 25 +- .../neon/neon_news/lib/pages/article.dart | 4 +- .../neon_news/lib/widgets/articles_view.dart | 46 ++-- .../neon_news/lib/widgets/feeds_view.dart | 236 +++++++++--------- .../neon_notes/lib/dialogs/create_note.dart | 27 +- 5 files changed, 166 insertions(+), 172 deletions(-) diff --git a/packages/neon/neon_news/lib/dialogs/add_feed.dart b/packages/neon/neon_news/lib/dialogs/add_feed.dart index 4e99e8e1..53730abd 100644 --- a/packages/neon/neon_news/lib/dialogs/add_feed.dart +++ b/packages/neon/neon_news/lib/dialogs/add_feed.dart @@ -71,30 +71,29 @@ class _NewsAddFeedDialogState extends State { submit(); }, ), - if (widget.folderID == null) ...[ + if (widget.folderID == null && folders.hasError) Center( child: NeonError( folders.error, onRetry: widget.bloc.refresh, ), ), + if (widget.folderID == null) Center( child: NeonLinearProgressIndicator( visible: folders.isLoading, ), ), - if (folders.hasData) ...[ - NewsFolderSelect( - folders: folders.requireData, - value: folder, - onChanged: (final f) { - setState(() { - folder = f; - }); - }, - ), - ], - ], + if (widget.folderID == null && folders.hasData) + NewsFolderSelect( + folders: folders.requireData, + value: folder, + onChanged: (final f) { + setState(() { + folder = f; + }); + }, + ), ElevatedButton( onPressed: folders.hasData ? submit : null, child: Text(NewsLocalizations.of(context).feedAdd), diff --git a/packages/neon/neon_news/lib/pages/article.dart b/packages/neon/neon_news/lib/pages/article.dart index d088fe11..70b854d1 100644 --- a/packages/neon/neon_news/lib/pages/article.dart +++ b/packages/neon/neon_news/lib/pages/article.dart @@ -146,7 +146,7 @@ class _NewsArticlePageState extends State { ); }, ), - if (widget.url != null) ...[ + if (widget.url != null) IconButton( onPressed: () async { await launchUrlString( @@ -157,6 +157,7 @@ class _NewsArticlePageState extends State { tooltip: NewsLocalizations.of(context).articleOpenLink, icon: const Icon(Icons.open_in_new), ), + if (widget.url != null) IconButton( onPressed: () async { await Share.share(await _getURL()); @@ -164,7 +165,6 @@ class _NewsArticlePageState extends State { tooltip: NewsLocalizations.of(context).articleShare, icon: const Icon(Icons.share), ), - ], ], ), body: SafeArea( diff --git a/packages/neon/neon_news/lib/widgets/articles_view.dart b/packages/neon/neon_news/lib/widgets/articles_view.dart index 1a66b5e7..19f1d08e 100644 --- a/packages/neon/neon_news/lib/widgets/articles_view.dart +++ b/packages/neon/neon_news/lib/widgets/articles_view.dart @@ -63,30 +63,20 @@ class _NewsArticlesViewState extends State { isExpanded: true, value: selectedFilterTypeSnapshot.data, items: [ - FilterType.all, - FilterType.unread, - if (widget.bloc.listType == null) ...[ - FilterType.starred, - ], - ].map>( - (final a) { - late final String label; - switch (a) { - case FilterType.all: - label = NewsLocalizations.of(context).articlesFilterAll; - case FilterType.unread: - label = NewsLocalizations.of(context).articlesFilterUnread; - case FilterType.starred: - label = NewsLocalizations.of(context).articlesFilterStarred; - default: - throw Exception('FilterType $a should not be shown'); - } - return DropdownMenuItem( - value: a, - child: Text(label), - ); - }, - ).toList(), + _buildDropdownItem( + FilterType.all, + NewsLocalizations.of(context).articlesFilterAll, + ), + _buildDropdownItem( + FilterType.unread, + NewsLocalizations.of(context).articlesFilterUnread, + ), + if (widget.bloc.listType == null) + _buildDropdownItem( + FilterType.starred, + NewsLocalizations.of(context).articlesFilterStarred, + ), + ], onChanged: (final value) { widget.bloc.setFilterType(value!); }, @@ -99,6 +89,11 @@ class _NewsArticlesViewState extends State { ), ); + DropdownMenuItem _buildDropdownItem(final FilterType value, final String label) => DropdownMenuItem( + value: value, + child: Text(label), + ); + Widget _buildArticle( final BuildContext context, final news.Article article, @@ -116,13 +111,12 @@ class _NewsArticlesViewState extends State { : Theme.of(context).textTheme.titleMedium!.copyWith(color: Theme.of(context).disabledColor), ), ), - if (article.mediaThumbnail != null) ...[ + if (article.mediaThumbnail != null) NeonUrlImage( url: article.mediaThumbnail!, size: const Size(100, 50), fit: BoxFit.cover, ), - ], ], ), subtitle: Row( diff --git a/packages/neon/neon_news/lib/widgets/feeds_view.dart b/packages/neon/neon_news/lib/widgets/feeds_view.dart index f36b6672..d57ba8ad 100644 --- a/packages/neon/neon_news/lib/widgets/feeds_view.dart +++ b/packages/neon/neon_news/lib/widgets/feeds_view.dart @@ -42,128 +42,132 @@ class NewsFeedsView extends StatelessWidget { final BuildContext context, final news.Feed feed, final List folders, - ) => - ListTile( - title: Text( - feed.title, - style: feed.unreadCount! == 0 - ? Theme.of(context).textTheme.titleMedium!.copyWith(color: Theme.of(context).disabledColor) - : null, + ) { + Widget trailing = PopupMenuButton( + itemBuilder: (final context) => [ + PopupMenuItem( + value: NewsFeedAction.showURL, + child: Text(NewsLocalizations.of(context).feedShowURL), ), - subtitle: feed.unreadCount! > 0 - ? Text(NewsLocalizations.of(context).articlesUnread(feed.unreadCount!)) - : const SizedBox(), - leading: NewsFeedIcon(feed: feed), - trailing: Row( - mainAxisSize: MainAxisSize.min, - children: [ - if (feed.updateErrorCount > 0) ...[ - IconButton( - onPressed: () async { - await showDialog( - context: context, - builder: (final context) => NewsFeedUpdateErrorDialog( - feed: feed, - ), - ); - }, - tooltip: NewsLocalizations.of(context).feedShowErrorMessage, - iconSize: 30, - icon: Text( - feed.updateErrorCount.toString(), - style: TextStyle( - color: Theme.of(context).colorScheme.error, - ), - ), - ), - ], - PopupMenuButton( - itemBuilder: (final context) => [ - PopupMenuItem( - value: NewsFeedAction.showURL, - child: Text(NewsLocalizations.of(context).feedShowURL), - ), - PopupMenuItem( - value: NewsFeedAction.delete, - child: Text(NewsLocalizations.of(context).actionDelete), - ), - PopupMenuItem( - value: NewsFeedAction.rename, - child: Text(NewsLocalizations.of(context).actionRename), - ), - if (folders.isNotEmpty) ...[ - PopupMenuItem( - value: NewsFeedAction.move, - child: Text(NewsLocalizations.of(context).actionMove), - ), - ], - ], - onSelected: (final action) async { - switch (action) { - case NewsFeedAction.showURL: - await showDialog( - context: context, - builder: (final context) => NewsFeedShowURLDialog( - feed: feed, - ), - ); - case NewsFeedAction.delete: - if (!context.mounted) { - return; - } - if (await showConfirmationDialog( - context, - NewsLocalizations.of(context).feedRemoveConfirm(feed.title), - )) { - bloc.removeFeed(feed.id); - } - case NewsFeedAction.rename: - if (!context.mounted) { - return; - } - final result = await showRenameDialog( - context: context, - title: NewsLocalizations.of(context).feedRename, - value: feed.title, - ); - if (result != null) { - bloc.renameFeed(feed.id, result); - } - case NewsFeedAction.move: - if (!context.mounted) { - return; - } - final result = await showDialog>( - context: context, - builder: (final context) => NewsMoveFeedDialog( - folders: folders, - feed: feed, - ), - ); - if (result != null) { - bloc.moveFeed(feed.id, result[0]); - } - } - }, - ), - ], + PopupMenuItem( + value: NewsFeedAction.delete, + child: Text(NewsLocalizations.of(context).actionDelete), ), - onLongPress: () { - if (feed.unreadCount! > 0) { - bloc.markFeedAsRead(feed.id); - } - }, - onTap: () async { - await Navigator.of(context).push( - MaterialPageRoute( - builder: (final context) => NewsFeedPage( - bloc: bloc, + PopupMenuItem( + value: NewsFeedAction.rename, + child: Text(NewsLocalizations.of(context).actionRename), + ), + if (folders.isNotEmpty) ...[ + PopupMenuItem( + value: NewsFeedAction.move, + child: Text(NewsLocalizations.of(context).actionMove), + ), + ], + ], + onSelected: (final action) async { + switch (action) { + case NewsFeedAction.showURL: + await showDialog( + context: context, + builder: (final context) => NewsFeedShowURLDialog( feed: feed, ), + ); + case NewsFeedAction.delete: + if (!context.mounted) { + return; + } + if (await showConfirmationDialog( + context, + NewsLocalizations.of(context).feedRemoveConfirm(feed.title), + )) { + bloc.removeFeed(feed.id); + } + case NewsFeedAction.rename: + if (!context.mounted) { + return; + } + final result = await showRenameDialog( + context: context, + title: NewsLocalizations.of(context).feedRename, + value: feed.title, + ); + if (result != null) { + bloc.renameFeed(feed.id, result); + } + case NewsFeedAction.move: + if (!context.mounted) { + return; + } + final result = await showDialog>( + context: context, + builder: (final context) => NewsMoveFeedDialog( + folders: folders, + feed: feed, + ), + ); + if (result != null) { + bloc.moveFeed(feed.id, result[0]); + } + } + }, + ); + if (feed.updateErrorCount > 0) { + trailing = Row( + mainAxisSize: MainAxisSize.min, + children: [ + IconButton( + onPressed: () async { + await showDialog( + context: context, + builder: (final context) => NewsFeedUpdateErrorDialog( + feed: feed, + ), + ); + }, + tooltip: NewsLocalizations.of(context).feedShowErrorMessage, + iconSize: 30, + icon: Text( + feed.updateErrorCount.toString(), + style: TextStyle( + color: Theme.of(context).colorScheme.error, + ), ), - ); - }, + ), + trailing, + ], ); + } + + return ListTile( + title: Text( + feed.title, + style: feed.unreadCount! == 0 + ? Theme.of(context).textTheme.titleMedium!.copyWith(color: Theme.of(context).disabledColor) + : null, + ), + subtitle: feed.unreadCount! > 0 + ? Text(NewsLocalizations.of(context).articlesUnread(feed.unreadCount!)) + : const SizedBox(), + leading: NewsFeedIcon(feed: feed), + trailing: trailing, + onLongPress: () { + if (feed.unreadCount! > 0) { + bloc.markFeedAsRead(feed.id); + } + }, + onTap: () async { + await Navigator.of(context).push( + MaterialPageRoute( + builder: (final context) => NewsFeedPage( + bloc: bloc, + feed: feed, + ), + ), + ); + }, + ); + } } enum NewsFeedAction { diff --git a/packages/neon/neon_notes/lib/dialogs/create_note.dart b/packages/neon/neon_notes/lib/dialogs/create_note.dart index ea521f22..6f1c3add 100644 --- a/packages/neon/neon_notes/lib/dialogs/create_note.dart +++ b/packages/neon/neon_notes/lib/dialogs/create_note.dart @@ -53,28 +53,25 @@ class _NotesCreateNoteDialogState extends State { submit(); }, ), - if (widget.category == null) ...[ + if (widget.category == null && notes.hasError) Center( child: NeonError( notes.error, onRetry: widget.bloc.refresh, ), ), - Center( - child: NeonLinearProgressIndicator( - visible: notes.isLoading, - ), + if (widget.category == null && notes.isLoading) + const Center( + child: NeonLinearProgressIndicator(), + ), + if (widget.category == null && notes.hasData) + NotesCategorySelect( + categories: notes.requireData.map((final note) => note.category).toSet().toList(), + onChanged: (final category) { + selectedCategory = category; + }, + onSubmitted: submit, ), - if (notes.hasData) ...[ - NotesCategorySelect( - categories: notes.requireData.map((final note) => note.category).toSet().toList(), - onChanged: (final category) { - selectedCategory = category; - }, - onSubmitted: submit, - ), - ], - ], ElevatedButton( onPressed: submit, child: Text(NotesLocalizations.of(context).noteCreate),