Add detection of drawer open and close in Scaffold widget as a callback method. by bikcrum · Pull Request #67249 · flutter/flutter

bikcrum · 2020-10-04T05:47:44Z

Description

The change contains the addition of drawer callbacks in Scaffold widget. These are useful for the detection of the drawer open or closed, implemented in Scaffold. This accounts for both drawer and endDrawer having their callbacks namely drawerCallback and endDrawerCallback respectively in Scaffold constructor properly. There might be multiple reason when we want to change the state of the widget according to the state of the drawer, open or closed. It is always better to have a simple callback in Scaffold rather than letting users explicitly write a bunch of code to handle this.

Implementation: drawerCallback and endDrawerCallback are added property in the constructor which provides with the state of the drawer whether opened or closed.

  return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      drawer: NavDrawer(),
      onDrawerChanged: (isOpen) {
        // write your callback implementation here
        print('drawer callback isOpen=$isOpen');
      },
      endDrawer: NavDrawerEnd(),
      onEndDrawerChanged: (isOpen) {
        // write your callback implementation here
        print('end drawer callback isOpen=$isOpen');
      },
      body:
      ...

Related Issues

#43512
#14510

Tests

I added the following tests:

I have used the above snippet to test the callback implementation. The following test applies for both drawer and endDrawer

Use case	Result
Hamburger menu is clicked	Drawer opens, prints "drawer callback isOpen=true"
Drawer is closed by clicking outside drawer	Drawer closes, prints "drawer callback isOpen=false"
Back button is pressed in drawer opened state	Drawer closes, prints "drawer callback isOpen=false"

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: https://flutter.dev/go/breaking-changes-template Replace this with a link to a pull request that adds the migration guide to https://flutter.dev/docs/release/breaking-changes

Sync forked repo with original repo

flutter-dashboard · 2020-10-04T05:47:47Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

chunhtai

I think this change looks good because we have exposed every other parameter in drawercontroller.

and the use case in #43512 seems valid. cc @Hixie

We need a test though.

packages/flutter/lib/src/material/scaffold.dart

Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>

chunhtai · 2020-11-23T23:02:56Z

Hi @bikcrum
The doc lint is complaining

dartdoc:stdout: Generating docs for library material from package:flutter/material.dart...
dartdoc:stderr:   error: unresolved doc reference [Scaffold. endDrawer]
dartdoc:stderr:     from material.Scaffold.endDrawerCallback: (file:///b/s/w/ir/k/flutter%20sdk/packages/flutter/lib/src/material/scaffold.dart:1266:24)
dartdoc:stderr:     in documentation inherited from material.Scaffold.endDrawerCallback: (file:///b/s/w/ir/k/flutter%20sdk/packages/flutter/lib/src/material/scaffold.dart:1266:24)

and we need a test as well.

Sync with base master

Sync with master

bikcrum · 2020-11-25T15:28:39Z

Hi @bikcrum
The doc lint is complaining

dartdoc:stdout: Generating docs for library material from package:flutter/material.dart...
dartdoc:stderr:   error: unresolved doc reference [Scaffold. endDrawer]
dartdoc:stderr:     from material.Scaffold.endDrawerCallback: (file:///b/s/w/ir/k/flutter%20sdk/packages/flutter/lib/src/material/scaffold.dart:1266:24)
dartdoc:stderr:     in documentation inherited from material.Scaffold.endDrawerCallback: (file:///b/s/w/ir/k/flutter%20sdk/packages/flutter/lib/src/material/scaffold.dart:1266:24)

and we need a test as well.

@chunhtai Thank you for suggesting what went wrong. I have made a few changes to the file and now it has passed all the tests.

chunhtai · 2020-11-25T17:27:21Z

@bikcrum Hi, we need a test in order to merge this PR

bikcrum · 2020-11-25T17:40:45Z

@bikcrum Hi, we need a test in order to merge this PR

Yes, of course. If there is anything that is to be done from my side, please do let me know. Thank you.

chunhtai · 2020-11-25T17:56:22Z

You can write a test similar to this one in the same file

flutter/packages/flutter/test/material/scaffold_test.dart

Line 240 in e118e1a

testWidgets('Drawer scrolling', (WidgetTester tester) async {

The goal is to verify the callback is called when drawer opens or closes

bikcrum · 2020-11-26T10:34:48Z

You can write a test similar to this one in the same file

flutter/packages/flutter/test/material/scaffold_test.dart

Line 240 in e118e1a

testWidgets('Drawer scrolling', (WidgetTester tester) async {

The goal is to verify the callback is called when the drawer opens or closes

I have added a test to the file. As per the automatic checks, I am not sure why 'Mac framework_tests' is failing. Can you provide me any suggestions?

Sync with master

chunhtai

LGTM

bikcrum · 2020-12-01T04:18:58Z

LGTM

Thank you for approving the PR. So, are we waiting for flutter-build to be fixed before merging this PR?

Hixie · 2020-12-01T07:28:31Z

I was going to put the "waiting for tree to go green" label on (which automatically lands the patch when flutter-build goes green) but actually can we make a slight change first? For consistency with our style guide, the properties should be called "on Event", as in onDrawerChanged or onEndDrawerChanged or some such.

Hixie · 2020-12-01T07:29:09Z

(see https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#naming-rules-for-typedefs-and-function-variables)

bikcrum · 2020-12-01T09:38:13Z

I was going to put the "waiting for tree to go green" label on (which automatically lands the patch when flutter-build goes green) but actually can we make a slight change first? For consistency with our style guide, the properties should be called "on Event", as in onDrawerChanged or onEndDrawerChanged or some such.

Thank you for your suggestion. I have renamed the parameters accordingly.

brendonanderson · 2021-01-19T17:26:26Z

When will this change make it into the stable channel?

bikcrum · 2021-03-04T11:24:02Z

When will this change make it into the stable channel?

It is released now in Flutter version 2.0.0

bikcrum added 2 commits October 4, 2020 11:02

Merge pull request #1 from flutter/master

441c887

Sync forked repo with original repo

Sync with master

ec46e40

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Oct 4, 2020

googlebot added the cla: yes label Oct 4, 2020

HansMuller requested a review from chunhtai October 8, 2020 23:35

chunhtai reviewed Nov 17, 2020

View reviewed changes

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

bikcrum force-pushed the feature/drawer-callback branch from 5423b45 to ec46e40 Compare November 19, 2020 13:02

Move drawer callbacks below setState

aa5d10c

chunhtai reviewed Nov 19, 2020

View reviewed changes

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

packages/flutter/lib/src/material/scaffold.dart Outdated Show resolved Hide resolved

bikcrum and others added 2 commits November 20, 2020 14:42

Update packages/flutter/lib/src/material/scaffold.dart

a875fd4

Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>

Update packages/flutter/lib/src/material/scaffold.dart

ec4c612

Co-authored-by: chunhtai <47866232+chunhtai@users.noreply.github.com>

bikcrum added 4 commits November 24, 2020 18:01

Merge pull request #2 from flutter/master

f90e692

Sync with base master

Merge pull request #3 from bikcrum/master

e32c97a

Sync with master

make callback function optional

5a737d9

fix unresolve doc reference

729a966

add a widget test function

2856cbe

bikcrum force-pushed the feature/drawer-callback branch from 486b61d to 2856cbe Compare November 26, 2020 09:23

bikcrum force-pushed the feature/drawer-callback branch from c23b8c1 to 2856cbe Compare November 27, 2020 04:11

bikcrum added 3 commits November 27, 2020 09:56

fix Specify type annotations

6ebde0f

Merge pull request #4 from flutter/master

2151a9f

Sync with master

Merge pull request #5 from bikcrum/master

dcc27bf

Sync with master

chunhtai approved these changes Nov 30, 2020

View reviewed changes

Hixie added waiting for tree to go green and removed waiting for tree to go green labels Dec 1, 2020

change callback property name to comply with style guide

4a4c913

chunhtai added the waiting for tree to go green label Dec 1, 2020

fluttergithubbot merged commit dc432d7 into flutter:master Dec 1, 2020

AlexHartford mentioned this pull request Feb 25, 2021

Request: Expose callback for when Drawer finishes closing #14510

Closed

Conversation

bikcrum commented Oct 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

flutter-dashboard bot commented Oct 4, 2020

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chunhtai commented Nov 23, 2020

Uh oh!

bikcrum commented Nov 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunhtai commented Nov 25, 2020

Uh oh!

bikcrum commented Nov 25, 2020

Uh oh!

chunhtai commented Nov 25, 2020

Uh oh!

bikcrum commented Nov 26, 2020

Uh oh!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

bikcrum commented Dec 1, 2020

Uh oh!

Hixie commented Dec 1, 2020

Uh oh!

Hixie commented Dec 1, 2020

Uh oh!

bikcrum commented Dec 1, 2020

Uh oh!

brendonanderson commented Jan 19, 2021

Uh oh!

bikcrum commented Mar 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bikcrum commented Oct 4, 2020 •

edited

Loading

bikcrum commented Nov 25, 2020 •

edited

Loading