Skip to content
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

Backdrop scaffoldkey #61

Closed
wants to merge 4 commits into from
Closed

Backdrop scaffoldkey #61

wants to merge 4 commits into from

Conversation

kannel-outis
Copy link
Contributor

Included a scaffoldkey incase one needs to access let's say a snackbar or the bottomsheet of the backdrop scaffold to avoid creating multiple scaffolds :)

@WieFel
Copy link
Collaborator

WieFel commented Oct 1, 2020

Thanks for the contribution.
I am not sure whether we need an additional property backDropScaffoldKey as we already have the Key key inside the constructor of BackdropScaffold. That key is then passed to super(key: key), which means it is used for the BackdropScaffold instance.

Maybe we could use the same key in

    return WillPopScope(
      onWillPop: () => _willPopCallback(context),
      child: Scaffold(
        key: scaffoldKey,
        key: widget.key ?? scaffoldKey,

Or do we need an extra property backDropScaffoldKey for that?
@daadu @kannel-outis what do you think?

@kannel-outis
Copy link
Contributor Author

OMG I never really checked for the "key"(I needed it so badly that I had to add it) . Yes the key can be used instead of backdropScaffoldKey.

@WieFel
Copy link
Collaborator

WieFel commented Oct 1, 2020

But does it make a difference if we pass it to Scaffold(key: key, ...) or not?

@kannel-outis
Copy link
Contributor Author

No it does not and really sorry for making this PR. I should have checked.

@daadu
Copy link
Member

daadu commented Oct 2, 2020

@kannel-outis key property is a recent feature update. You must have tried on the older version earlier, that is why missed it!!

@daadu
Copy link
Member

daadu commented Oct 2, 2020

@WieFel key property of any widget is for that widget itself, we shouldn't use it for other widget (might upset flutter!!), instead additional key like scaffoldKey could be added for Scaffold widget.

I suggest adding scaffoldKey to BackdropScaffold which could be used with Scaffold widget if provided, else create a GlobalKey. This can help to get underlying ScaffoldState without needing BuildContext (as of now user can do it by, Backdrop.of(context).scaffoldKey).

I saw similar pattern in MaterialApp widget they have provided navigatorKey option - so that BuildContext could be avoided to access NavigatorState.

@daadu
Copy link
Member

daadu commented Oct 2, 2020

@WieFel or @kannel-outis I suggest file new PR for this.

@kannel-outis
Copy link
Contributor Author

@kannel-outis key property is a recent feature update. You must have tried on the older version earlier, that is why missed it!!

Maybe

@kannel-outis
Copy link
Contributor Author

@WieFel or @kannel-outis I suggest file new PR for this.

Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants