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

update customizedButton #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

beheobong
Copy link

customizedButton in confirmed mode

@beheobong
Copy link
Author

@naoki0719 please check this commit

@naoki0719
Copy link
Owner

Thanks for the new feature!
Please fix the README.md as well.
Version 9.1.0 is better for some compatibility.

@clragon
Copy link
Contributor

clragon commented Nov 6, 2023

this is a breaking API change and according to semantic versioning it shouldnt be a patch or a minor but a major.
9.0.0 -> 10.0.0
This is important because dart pub will automatically upgrade patch and minors, so if they break the API, users will have errors e.g. in their continous integration or when pulling a fresh repository.

Widget? customizedButtonChild,
VoidCallback? customizedButtonTap,
Widget Function(bool confirmed)? customizedButtonChild,
Function(bool confirmed, InputController controller)? customizedButtonTap,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a return type for the Function.

Suggested change
Function(bool confirmed, InputController controller)? customizedButtonTap,
void Function(bool confirmed, InputController controller)? customizedButtonTap,

@@ -107,7 +107,8 @@ class ScreenLock extends StatefulWidget {
final ValueChanged<int>? onMaxRetries;

/// Tapped for left side lower button.
final VoidCallback? customizedButtonTap;
final Function(bool confirmed, InputController controller)?
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final Function(bool confirmed, InputController controller)?
final void Function(bool confirmed, InputController controller)?

@clragon
Copy link
Contributor

clragon commented Nov 6, 2023

Additionally to the above, it should already be possible to to what you wish for without this PR.
For example:

onPressed: () async {
  bool confirmed = false;
  final controller = InputController();
  final subscription = controller.confirmed
      .listen((event) => confirmed = event);
  await screenLock(
    context: context,
    correctString: '1234',
    customizedButtonChild: StreamBuilder<bool>(
      stream: controller.confirmed,
      builder: (context, snapshot) => Icon(
        confirmed
            ? Icons.restart_alt_rounded
            : Icons.arrow_back,
      ),
    ),
    customizedButtonTap: () async {
      if (confirmed) {
        controller.unsetConfirmed();
      } else {
        Navigator.of(context).pop();
      }
    },
    onOpened: () async => await localAuth(context),
  );
  subscription.cancel();
},

So I dont think it is necessary to add it to the package itself.

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.

4 participants