-
-
Notifications
You must be signed in to change notification settings - Fork 279
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: #678 - added bottom navigation bar to product page #679
Merged
monsieurtanuki
merged 4 commits into
openfoodfacts:develop
from
monsieurtanuki:feature/#678
Nov 25, 2021
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
30e6841
feat: #678 - added bottom navigation bar to product page
monsieurtanuki 683dc9a
feat: #678 - explicitly selecting the bottom navigation tab
monsieurtanuki 49b21a6
feat: #678 - (forgot that one)
monsieurtanuki 5b51a2a
feat: #678 - renaming getCurrentPage to getDefaultPage
monsieurtanuki File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
packages/smooth_app/lib/pages/smooth_bottom_navigation_bar.dart
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:provider/provider.dart'; | ||
| import 'package:smooth_app/data_models/user_preferences.dart'; | ||
| import 'package:smooth_app/pages/history_page.dart'; | ||
| import 'package:smooth_app/pages/scan/scan_page.dart'; | ||
| import 'package:smooth_app/pages/user_preferences_page.dart'; | ||
|
|
||
| class _Page { | ||
| const _Page({required this.name, required this.icon, required this.body}); | ||
| final String name; | ||
| final IconData icon; | ||
| final Widget body; | ||
| } | ||
|
|
||
| class SmoothBottomNavigationBar extends StatelessWidget { | ||
| static const List<_Page> _pages = <_Page>[ | ||
| _Page( | ||
| name: 'Profile', // TODO(monsieurtanuki): translate | ||
| icon: Icons.account_circle, | ||
| body: UserPreferencesPage(), | ||
| ), | ||
| _Page( | ||
| name: 'Scan or Search', | ||
| icon: Icons.search, | ||
| body: ScanPage(), | ||
| ), | ||
| _Page( | ||
| name: 'History', | ||
| icon: Icons.history, | ||
| body: HistoryPage(), | ||
| ), | ||
| ]; | ||
|
|
||
| static Widget getCurrentPage(final BuildContext context) { | ||
| final UserPreferences userPreferences = context.watch<UserPreferences>(); | ||
| return _pages[userPreferences.bottomTabIndex].body; | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final UserPreferences userPreferences = context.watch<UserPreferences>(); | ||
| return BottomNavigationBar( | ||
| showSelectedLabels: false, | ||
| showUnselectedLabels: false, | ||
| selectedItemColor: Colors.white, | ||
| backgroundColor: Theme.of(context).appBarTheme.backgroundColor, | ||
| currentIndex: userPreferences.bottomTabIndex, | ||
| onTap: (final int index) async { | ||
| userPreferences.setBottomTabIndex(index); | ||
| await Navigator.push<Widget>( | ||
| context, | ||
| MaterialPageRoute<Widget>( | ||
| builder: (BuildContext context) => _pages[index].body, | ||
| ), | ||
| ); | ||
| }, | ||
| items: _pages | ||
| .map( | ||
| (_Page p) => BottomNavigationBarItem( | ||
| icon: Icon(p.icon, size: 28), | ||
| label: p.name, | ||
| ), | ||
| ) | ||
| .toList(), | ||
| ); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 0 additions & 9 deletions
9
packages/smooth_ui_library/lib/navigation/models/smooth_bottom_navigation_bar_item.dart
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 haven't read through the whole code yet, but I don't see the value in storing the index. Especially now where besides the scanner the other two pages don't provide that much value anymore.
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.
@M123-dev The point of storing the index is to use exactly the same navigation bar in all cases: the legacy cases (the former home page, divided in 3 tabs) and the new cases (purpose of this issue, and the product page is dealt with in this PR). And without too much complex code: just add
bottomNavigationBar: SmoothBottomNavigationBar()to theScaffold.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.
Hey this PR is pretty close to my solution for this as well, so it looks good to me.
I don't really see why we need the current index though? Is it so if a user closes the app and comes back, they land on the tab they we on when they closed the app ? In that case, it was discussed that whenever a user opens the app they should see the scan page with "Welcome to Open food facts" text on the search box.
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.
Oh, I didn't know that.
Behind saving the current index, there was also the question (already asked): which tab should look "selected" when we're not on one tab page in particular - e.g. when we are on the product page?
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.
So in a nutshell, this is what I understand the behavior to be:
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.
More precisely, navigation bar was added to the product page. I'm not sure if there are other pages beyond profile, scan, history and product, but if so, the navigation bar should be added there too.
No, because you asked me to remove the tab index from the preferences and that's where I kept its value. If you go to a product page, the navigation bar will display the default tab as selected: the scan tab. Anyway I didn't get the answer to "which tab should be selected when we are on the product page", and in the meanwhile I decided to select the scan tab.
No, if you click on the history tab you always go to the history page.
This PR is only about issue #678, and does not implement the not-even-existing-yet "Make tabs stateful" issue evoked in #655.
That's correct, the default tab (which is opened in
main.dart) is the scan tab.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.
Hey, my previous message was more around generally what the navigation behavior should be like and not about how it would be after this PR. Sorry for misunderstanding. This PR adds the feature it's meant to add, so thank you for that.
There's also the settings page, but that one is not that important, also doesn't feel like we should make the settings page sticky. Let's skip it for now. If we have to add the bar to it later, we can do so.
So if the user goes to a tab (lets say history tab) and clicks around on the page and lands on some other view (lets say product page), he is considered to still be on the history tab, so the history tab should appear as selected. I was thinking this should be doable without storing the tab index in user prefs. if we instantiate with
bottomNavigationBar: const SmoothBottomNavigationBar(tab=SmoothBottomNavigationTab.History)in new_product_page