-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add NavigationBar dartpad example #97046
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
Conversation
|
The checks failure looks related to #89243 |
HansMuller
left a comment
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.
The example will need a test, see examples/api/test
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.
There are some trivial formatting issues here and the code can be simplified a little
import 'package:flutter/material.dart';
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({ Key? key }) : super(key: key);
@override
Widget build(BuildContext context) {
return const MaterialApp(home: NavigationExample());
}
}
class NavigationExample extends StatefulWidget {
const NavigationExample({Key? key}) : super(key: key);
@override
State<NavigationExample> createState() => _NavigationExampleState();
}
class _NavigationExampleState extends State<NavigationExample> {
int currentPageIndex = 0;
@override
Widget build(BuildContext context) {
return Scaffold(
bottomNavigationBar: NavigationBar(
onDestinationSelected: (int index) {
setState(() {
currentPageIndex = index;
});
},
selectedIndex: currentPageIndex,
destinations: const <Widget>[
NavigationDestination(
icon: Icon(Icons.explore),
label: 'Explore',
),
NavigationDestination(
icon: Icon(Icons.commute),
label: 'Commute',
),
NavigationDestination(
selectedIcon: Icon(Icons.bookmark),
icon: Icon(Icons.bookmark_border),
label: 'Saved',
),
],
),
body: <Widget>[
Container(
color: Colors.red,
alignment: Alignment.center,
child: const Text('Page 1'),
),
Container(
color: Colors.green,
alignment: Alignment.center,
child: const Text('Page 2'),
),
Container(
color: Colors.blue,
alignment: Alignment.center,
child: const Text('Page 3'),
),
][currentPageIndex],
);
}
}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.
Could you please specify what formatting needs to be done? Regarding the sample code, I have simplified it further.
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 had reformatted your example per https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo. That was the comment (above) was. Please try and align your updated example with what I'd written.
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.
Thanks, I have formatted the sample code as per above
Thanks for reviewing, I have added basic tests. let me know if you want me to add more scenarios. |
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 had reformatted your example per https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo. That was the comment (above) was. Please try and align your updated example with what I'd written.
|
@maheshmnj - the test failures aren't related to this PR. Please sync with master and push an empty commit to get the tests to run again. |
|
@dnfield, @jason-simmons - this docs-only PR, which AFAICT should have no effect on Flutter's contributed tests, has been failing the following rfw and svg tests for at least a week. Maybe it has something to do with flutter/tests#124? |
|
Hi, just checking if there is anything on my side to fix 1 failing checkmark. |
|
Per @dnfield - the source of the problem is likely the logic used to pick a commit for customer testing. A merge will not fix the issue, this PR needs to be rebased. |
|
Oops! it's still failing here's what I did Let me know if I missed anything Thanks |
|
@maheshmnj - I think something like this should work: Alternatively, just start a fresh PR with these changes that's based on the most recent commit from master. The only other time I've seen these two failures (on flutter_svg and rfw) it was related to this issue. The branch was forked from a commit behind where the customer testing was updated to account for golden file changes. |
|
This PR is still getting a relatively old commit of flutter/tests - flutter/tests@1943e74 - a properly done rebase should resolve that, but sometimes git is finnicky about that. If another rebase attempt does not fix this I would just close the PR and open a new one with these changes applied on a fresh branch. |
|
My git skills are limited and so I have to admit that I'd take Dan's advice about just creating a fresh PR - #97046 (comment). Sometimes that's the easiest way to deal with this kind of thing. |
Co-authored-by: Viren Khatri <[email protected]>
|
Thanks for the suggestion @dnfield luckily the rebase worked out. |
HansMuller
left a comment
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.
LGTM
This PR adds a dartpad example for Material 3 NavigationBar
Addresses #96750
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.