-
-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: #1022 - tap to switch to search card ("Welcome to Open Food Facts") #1502
feat: #1022 - tap to switch to search card ("Welcome to Open Food Facts") #1502
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heyy, the code looks good, thanks for this feature @cli1005. just some small comments but feel free to ignore them.
@@ -60,7 +60,7 @@ class ContinuousScanModel with ChangeNotifier { | |||
_states.clear(); | |||
_latestScannedBarcode = null; | |||
await refreshProductList(); | |||
for (final String barcode in _productList.barcodes.reversed) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that fix #1139
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -34,6 +35,7 @@ class PageManagerState extends State<PageManager> { | |||
}; | |||
|
|||
BottomNavigationTab _currentPage = BottomNavigationTab.Scan; | |||
List<Widget> _tabs = <Widget>[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything stopping you from directly adding all the _buildOffstageNavigator's up here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be frank, I don't have a MUST reason to put it here😄
} | ||
} | ||
|
||
class _InheritedDataManagerProvider extends InheritedWidget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using Provider for state managment, what about using it to keep the code consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to pass a flag from page_manager to scan_carousel, InheritedWidget might be a better approach to prevent adding params to constructors or rebuilding the whole tree
What
Screenshot
rpreplay-final1649246863_pA2IhwoI.mp4
Fixes bug(s)
Part of