Skip to content
Snippets Groups Projects

Bugfix: Embedded PAPasscodeViewController in UINavigationController so it has...

1 unresolved thread

Bugfix: Embedded PAPasscodeViewController in UINavigationController so it has a navigation bar, which has a cancel button (closes #330 (closed))

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
91 91 if (passcodeOn) {
92 92 PAPasscodeViewController *passcodeLockController = [[PAPasscodeViewController alloc] initForAction:PasscodeActionSet];
93 93 passcodeLockController.delegate = self;
94 [self.navigationController presentViewController:passcodeLockController animated:YES completion:nil];
94 UINavigationController *navigationController = [[UINavigationController alloc] initWithRootViewController:passcodeLockController];
95 [self.navigationController presentViewController:navigationController animated:YES completion:nil];
  • What would you think about adding a UIBarButtonItem instead of creating a new navigationController and all?

    I think it would be easier this way.

    Additionally, with an UIBarButtonItem, the check below should stay the same.

  • I thought about doing that at first but by embedding it to navigation controller we got an orange navigation bar which keeps the UI consistent, and matches the rest of the app.

    Another thing, PAPAsscodeController is a third party library which takes care of cancel action and setting title on its own when added in a navigation controller(Which saves us from adding a title label and cancel button and action in settings view controller) and gives more customisation options if we required in future.

    Third thing without a navigation controller the presentation animation of the passcode controller is a little weird which looks like its starting from bottom left corner.

    Added before and after screen recordings and screen shots

    Before -

    Before

    After -

    After

    UI Before -

    Screenshot_2019-02-04_at_7.38.36_PM

    After -

    Screenshot_2019-02-04_at_7.37.22_PM

  • Please register or sign in to reply
  • Hi @bubu, @fkuehne hope you are doing well. Please let me know if there is anything pending from my side or if my implementation lacks something. Or should I just wait?

    Thanks and have a nice day.

  • Hello @nishiths23, checked on the library side, looks like this is the recommended way of doing so LGTM.

    The whole logic/integration of PAPasscode integration could be simplified a bit but this is unreleated to the MR and can be addressed later on.

    If @fkuehne is okay with the changes, we can merge!

  • Maintainer

    @bubu LGTM push on master and cherry-pick to 3.0 please And this also requires an update to the about screen then.

    @nishiths23 thank you very much for the addition!

  • Merged with 7b10ce49, thank you for the MR.

  • closed

  • Please register or sign in to reply
    Loading