tomalabaster / flutter_app_lock

A widget to create lock screen like functionality for Flutter apps.
MIT License
26 stars 19 forks source link

Theme compatibility #8

Closed alejandrogiubel closed 11 months ago

alejandrogiubel commented 4 years ago

When I use this plugin, all my modals dialog take the default theme (blue theme). I guess this is because AppLock don't manipulate any ThemeData. When I modify your code and define the same ThemeData in AppLock all my app look ok.

I don't know if I'm using it correctly but I think a solution would be to define the ThemeData as an AppLock argument.

Thank. It is a useful plugin.

tomalabaster commented 4 years ago

Thanks for pointing this out @alejandrogiubel! I'll see what I can do🙂

tomalabaster commented 4 years ago

@alejandrogiubel If I were to add in properties for setting the ThemeData for AppLock, I'll need to add in all material variants and also support for Cupertino theming.

At that point, it's somewhat becoming a wrapper around MaterialApp and CupertinoApp when in reality I probably only wanted a WidgetsApp in the first place.

I'd rather find an alternative way of providing this functionality which makes it a bit more cohesive with any implementation and not require the passing around of multiple theme configurations. I'll give it some more thought over the coming week!

alejandrogiubel commented 4 years ago

@tomalabaster I think that the problem is that the plugin create another MaterialApp in AppLock widget and another context. If you don't define a ThemeData in theme property of the MaterialApp's widget it use the default theme (blue). Let me look my solution and think a way to propose it you.

purvajg commented 4 years ago

I have the same problem, debugBanner also shows up even when it is set up to false

alejandrogiubel commented 4 years ago

@purvajg maybe modifying the AppLock widget you can remove the banner.

@override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false, // ADD THIS LINE IN AppLock
      home: this.widget.enabled ? this._lockScreen : this.widget.builder(null),
      navigatorKey: _navigatorKey,
      routes: {
        '/lock-screen': (context) => this._lockScreen,
        '/unlocked': (context) =>
            this.widget.builder(ModalRoute.of(context).settings.arguments)
      },
    );
  }
purvajg commented 4 years ago

thanks @alejandrogiubel ..

alejandrogiubel commented 4 years ago

@purvajg did you solve the problem?

tomalabaster commented 4 years ago

Ok so a quick fix for this would be to allow the passing in of some ThemeData properties which would solve the problem you are both having but not affect anyone else.

Another problem with using a MaterialApp/CupertinoApp/WidgetsApp at the root of AppLock is that doing Navigator.of(context, rootNavigator: true) would reach AppLock's navigator. Allowing the passing in of ThemeData properties wouldn't solve this, but might be needed anyway even if AppLock doesn't have a MaterialApp/CupertinoApp/WidgetsApp at the root.

I'll add in the ability to pass in ThemeData.

alejandrogiubel commented 4 years ago

Yes. The first thing you mention is what I proposed to you.

class AppLock extends StatefulWidget {
  final Widget Function(Object) builder;
  final Widget lockScreen;
  final bool enabled;
  final Duration backgroundLockLatency;
  final ThemeData themeData; // Add this property to AppLock widget
  final ThemeData darkTheme;// For dark theme
...
//Your constructor
const AppLock({
    Key key,
    @required this.builder,
    @required this.lockScreen,
    this.enabled = true,
    this.backgroundLockLatency = const Duration(seconds: 0),
    this.themeData,//New property
    this.darkTheme,//New property
  }) : super(key: key);

@override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: widget.themeData, //The theme data
      darkTheme: widget.darkTheme,//This for dark theme
      home: this.widget.enabled ? this._lockScreen : this.widget.builder(null),
      navigatorKey: _navigatorKey,
      routes: {
        '/lock-screen': (context) => this._lockScreen,
        '/unlocked': (context) =>
            this.widget.builder(ModalRoute.of(context).settings.arguments)
      },
    );
  }

//In your main
runApp(AppLock(
    builder: (args) => MyApp(),
    lockScreen: Login(),
    enabled: requireLogin,
    themeData: AppTheme.deepPurpleTheme,
    darkTheme: AppTheme.darkTheme,
  ));

You just need a mechanism to change light theme to dark theme in real time. I use Hive db. (This is just for manual theme change into the app, it is not necessary).

The other thing you mention is not clear to me.

purvajg commented 4 years ago

@alejandrogiubel nope, I actually need to make this plugin work without the MaterialApp its using in app_lock.dart, because its affecting some of my other code too besides the theme(which I could work around with the suggestion you mentioned)..I am struggling with substituting the routes used in MaterialApp if no MaterialApp is used.

alejandrogiubel commented 4 years ago

Ah ok ok. Them you need to send the context or using it in some way in the plugin. If I find something I will let you know. 💪👍

purvajg commented 4 years ago

Thanks @alejandrogiubel :)

purvajg commented 4 years ago

Removing the MaterialApp removes the context and makes the findAncestorStateOfType unusable, making the enable() disable() methods useless. @tomalabaster can you suggest a work around?

tomalabaster commented 4 years ago

@purvajg I was working on something last night, give me a few hours and I'll ask you to try out something on my working copy which does NOT need or use a MaterialApp (or any other kind of app), which is ideal!

purvajg commented 4 years ago

@tomalabaster that would be awesome .. waiting for the update

tomalabaster commented 4 years ago

@purvajg so I can better assess whether any of my solutions would work for your scenario what other things is the MaterialApp affecting in your app?

purvajg commented 4 years ago

It is affecting my 'Navigator.pop(context)' in a class... Not sure how it is happening(maybe because of the context, not sure at all though), but it starts working when AppLock is removed and stops working when AppLock is introduced.

This code is supposed to work this way:

  1. User clicks a show map button
  2. dialog shows up as a placeholder until the map is loaded. (the dialog remains in the background until the user exits the map)
  3. When the user exits the map, the dialog pops using Navigator.pop(context);

if AppLock is used then the placeholder dialog appears on top of the Map visible to the user while if its not used it is in the background not visible to the user.

code snippet :

foo(){
Map<String, dynamic> userGeohashAndAddressMap = await getLocation();
List<String> userGeohash = userGeohashAndAddressMap[TextConfig.usersLocationCollectionGeoHashList];

/// for exiting dialog:
Navigator.pop(context);
}

/// getLocation:
getLocation() async{
    String userPhoneNo = await UserDetails().getUserPhoneNoFuture();
    String placeholder = BazaarConfig(category: widget.category, categoryData: widget.categoryData).getPickLocation();

    ///  dialog shows up as a placeholder until the map is loaded:
CustomShowDialog().main(context, BazaarConfig.loadingMap, barrierDismissible: false);
/// loading map and letting user pick a location
Map<String, dynamic> result = await ChangeLocationInSearch
     (userNumber: userPhoneNo,placeholder: placeholder, ).getNewUserGeohash(context);

    return result;
  }
tomalabaster commented 4 years ago

@purvajg what class is this in? Is this a widget which will be part of your MaterialApp or is it passed into AppLock.lockScreen?

tomalabaster commented 4 years ago

My current best solution is to move AppLock to be the home widget of your own Material/Cupertino/WidgetsApp, but that would require users of AppLock to move all of their named routes from their Material/Cupertino/WidgetsApp widget to a Navigator widget and make that a child of AppLock, for example:

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: AppLock(
        lockScreen: LockScreen(),
        enabled: true,
        backgroundLockLatency: const Duration(seconds: 0),
        builder: (args) => Navigator(
          onGenerateRoute: (settings) {
            Widget widget;

            switch (settings.name) {
              case '/':
                widget = MyHomePage(title: 'Flutter Demo Home Page');
                break;
              default:
            }

            return MaterialPageRoute(builder: (context) => widget);
          },
        ),
      ),
    );
  }
}

It wouldn't however solve the problem of using Navigator.of(context, rootNavigator: true). Currently using that would go to the Navigator which belongs to AppLock's MaterialApp, not the user'sMaterial/Cupertino/WidgetsApp`.

In the solution proposed above, it would in fact go to the user's Material/Cupertino/WidgetsApp, but that shouldn't have any routes on it otherwise AppLock won't be able to protect those routes. Realistically, when implementing nested Navigator widgets, GlobalKeys should be used to reference particular Navigators at different levels.

From the looks of things though, you're not needing to use Navigator.of(context, rootNavigator: true), only Navigator.of(context).

tomalabaster commented 3 years ago

After giving this some thought, AppLock doesn't need to use a Navigator internally and after working with Navigator widgets for the past couple of weeks it's actually going to be best that AppLock doesn't use one at all...

A fix for inheriting theme data from the root MaterialApp could be solved as follows:

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      routes: {
        '/': (context) => MyHomePage(title: 'Flutter Demo Home Page'),
        '/login': (context) => LoginPage(),
        ...
      },
      builder: (context, child) { // child here is either a `Navigator` or `Router` widget depending which navigation API you use
        return AppLock(
          lockScreen: LockScreen(), // I might even change this to be a builder function to return the `LockScreen()` as needed
          enabled: true,
          backgroundLockLatency: const Duration(seconds: 0),
          child: child,
        );
      },
    );
  }
}

This would solve:

This would not solve:

There are question marks around:

I need to look at scenarios where navigating to other screens from LockScreen() might be needed and how this new mechanism can still achieve similar concepts that currently exist (e.g. passing args upon initial unlock).

The proposed solution here is definitely a breaking change and would be part of a 2.0.0 release.

tomalabaster commented 2 years ago

Hey everyone following this issue! I've just published a pre-release version of AppLock which no longer uses MaterialApp under the hood and changes the implementation to be compatible with your own theming.

If you're able to check it out and it's compatibility with your projects that would be great!: https://pub.dev/packages/flutter_app_lock/versions/4.0.0-dev.0

tomalabaster commented 11 months ago

Closing as v4.0.0 resolves this :+1: