Skip to content

Conversation

@maheshj01
Copy link
Member

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

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@maheshj01 maheshj01 changed the title Navigation bar docs Adds Navigation bar dartpad example Jan 22, 2022
@flutter-dashboard flutter-dashboard bot added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Jan 22, 2022
@maheshj01 maheshj01 changed the title Adds Navigation bar dartpad example Add Navigation bar dartpad example Jan 22, 2022
@maheshj01 maheshj01 changed the title Add Navigation bar dartpad example Add NavigationBar dartpad example Jan 22, 2022
@maheshj01
Copy link
Member Author

The checks failure looks related to #89243

Copy link
Contributor

@HansMuller HansMuller left a 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

Copy link
Contributor

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],
    );
  }
}

Copy link
Member Author

@maheshj01 maheshj01 Feb 1, 2022

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.

Copy link
Contributor

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.

Copy link
Member Author

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

@maheshj01
Copy link
Member Author

The example will need a test, see examples/api/test

Thanks for reviewing, I have added basic tests. let me know if you want me to add more scenarios.

Copy link
Contributor

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.

@HansMuller
Copy link
Contributor

@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.

@HansMuller
Copy link
Contributor

@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?

| 00:41 +79: /tmp/flutter_customer_testing.flutter_packages.DODKXB/tests/packages/rfw/test/argument_decoders_test.dart: golden checks
| ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
| The following assertion was thrown while running async test code:
| Golden "goldens/argument_decoders_test.text.png": Pixel test failed, 0.10% diff detected.
| Failure feedback can be found at
| /tmp/flutter_customer_testing.flutter_packages.DODKXB/tests/packages/rfw/test/failures
|
| When the exception was thrown, this was the stack:
| #0      LocalFileComparator.compare (package:flutter_test/src/_goldens_io.dart:102:7)
| <asynchronous suspension>
| <asynchronous suspension>
| (elided one frame from package:stack_trace)
| ════════════════════════════════════════════════════════════════════════════════════════════════════
|
| 00:41 +79 -1: /tmp/flutter_customer_testing.flutter_packages.DODKXB/tests/packages/rfw/test/argument_decoders_test.dart: golden checks [E]
|   Test failed. See exception logs above.
|   The test description was: golden checks

 00:38 +39 -1: /tmp/flutter_customer_testing.flutter_svg.HCYFXP/tests/test/golden_svg_test.dart: SVG Rendering matches golden files [E]
|   Expected: pairwise components nearly equal to [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ...]
|     Actual: [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, ...]
|      Which: has <255> which is not components nearly equal to <232> at index 52161
|   /tmp/flutter_customer_testing.flutter_svg.HCYFXP/tests/golden/simple/text_5.png does not match rendered output of /tmp/flutter_customer_testing.flutter_svg.HCYFXP/tests/example/assets/simple/text_5.svg!
|
|   package:test_api                expect
|   test/golden_svg_test.dart 48:7  main.<fn>
|

@maheshj01
Copy link
Member Author

Hi, just checking if there is anything on my side to fix 1 failing checkmark.

@HansMuller
Copy link
Contributor

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.

@maheshj01
Copy link
Member Author

Oops! it's still failing here's what I did

git checkout master 
git pull remote master
git checkout navigation-bar-docs
git rebase master
/// resolved conflicts
git add .
git rebase --continue
git push  /// failed
git push -f

Let me know if I missed anything

Thanks

@dnfield
Copy link
Contributor

dnfield commented Feb 28, 2022

@maheshmnj - I think something like this should work:

git fetch remote
git rebase remote/master
# resolve conflicts
git push -f

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.

@dnfield
Copy link
Contributor

dnfield commented Feb 28, 2022

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.

@HansMuller
Copy link
Contributor

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.

@maheshj01
Copy link
Member Author

maheshj01 commented Mar 1, 2022

Thanks for the suggestion @dnfield luckily the rebase worked out.

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants