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

Fix for issue Neumorphic button issue #170

Merged
merged 3 commits into from Oct 6, 2020
Merged

Conversation

ashank96
Copy link
Contributor

Fix for issue
The issue is because of the following line:
final appBarTheme = NeumorphicTheme.of(context).current.appBarTheme;
This should work, if I am using NeumorphicApp as the parent widget, which initialises NeumorphicTheme() but I do not want to use NeumorphicApp as my parent widget, hence NeumorphicTheme.of(context) will always be null.

@@ -177,7 +177,7 @@ class _NeumorphicButtonState extends State<NeumorphicButton> {

Widget _build(BuildContext context) {
final appBarPresent = NeumorphicAppBarTheme.of(context) != null;
final appBarTheme = NeumorphicTheme.of(context).current.appBarTheme;
final appBarTheme = NeumorphicTheme.of(context)?.current?.appBarTheme;

Choose a reason for hiding this comment

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

I'd suggest doing this instead:

final appBarTheme = NeumorphicTheme.currentTheme(context).appBarTheme;

Reason being that the NeumorphicTheme.of(context)?.current?.appBarTheme; still has a possibility of returning null (which will still throw a null reference exception). NeumorphicTheme.currentTheme is guaranteed to not return null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have done the required changes @josiahsrc

Copy link

@josiahsrc josiahsrc left a comment

Choose a reason for hiding this comment

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

LGTM

@ashank96
Copy link
Contributor Author

Can we please merge this? I need it in my production code.

@josiahsrc
Copy link

@ashank96 If you need it right now, you can create a dependency in your pubspec.yaml and point to your fork of this repository.

dependencies:
  package1:
    git:
      url: git://github.com/flutter/packages.git
      path: packages/package1

Source: https://flutter.dev/docs/development/packages-and-plugins/using-packages

@florent37
Copy link
Member

thanks, sorry for the delay, I publish it !

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