-
Notifications
You must be signed in to change notification settings - Fork 313
Fix typos in sample code #373
Conversation
* "bellow" -> "below" * "with will" -> "will not"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As commented.
with will should be changed to simply will the with is extra and simply doesn't need to be there.
| AppExtensionGeneratedPasswordRequireDigitsKey: (true), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is incorrect. If you pass no it will exclude symbols.
You can test this on a device in fact if you wish. Just change the setting to NO, and you'll see no symbols are in the generated password when registering or changing. If you leave it at YES, it will guarantee at least one symbol exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate you taking a look at this! I wouldn't want to put the wrong info in there. Unfortunately, when I test it, it looks like you're wrong. I hate to suggest that, so maybe there's something more complicated going on?
I tested using the master branch, and changing RegisterViewController.m in App Demo for iOS in Obj-C w/1Password 6.5.4 on a 7+. RequireSymbols seems to simply change the minimum number of symbols allowed between 0 and 1. With NO, I was still able to generate, save, & return to the app a password with symbols.
Is it possible the behavior differs based on the version of 1Password installed? I'd be really surprised if it depended on which sample app I use (although I only tested one). Maybe a bug in 1Password? Maybe just a mistake on your part?
If it looks like something more complicated, should we move this discussion to an issue?
Thanks again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @e28eta Thanks for writing back. I think here is fine for now, but if this next bit of discussions doesn't work we'll move it to a private discussion and figure it out. That way we're not potentially spamming others following the project with potentially unnecessary email :)
So, the first thing I want to make sure about is that as you're editing this in the "RegisterViewController" that you are clicking "Sign up" in the ACME app, because the Register view is handling is handling "sign up" in the app itself. Each view is hardcoding these separately, so if you change it in one place, but not the other, the other views will still behave with the YES, as that's default in each view in the samples.
The other thing is that I noticed the same thing you were seeing but after removing the ACME app from the simulator (or my device) and reinstalling the new build behaved as I outlined previously, that NO will not include any symbols. I have a feeling that the simulator/device has an out of date copy or perhaps Xcode isn't building or packaging properly with this small of a change.
Once I reinstall fully it behaves as I described:
YES = 1 symbol is always present
NO = no symbols present
Any chance that the behavior is more in line with this bit of info? If not, please shoot me an email support at agilebits dot com. If you mention my name "Kyle Swank" it should tag me in our system and notify me that you've written in. I try to check our support system's email for new messages every few hours.
Thanks for diving into this. We really do value when our community takes the time to help out and like you, I want to get this right so lets make sure we get it figured out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not seen 1Password disallow symbols as a result of changing RequireSymbols. So, email sent, thanks!
Note that I haven't been deleting/reinstalling 1Password (v6.5.4), just the Acme sample app.
| AppExtensionGeneratedPasswordRequireDigitsKey: (true), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise, with the previous comment.
| AppExtensionGeneratedPasswordRequireDigitsKey: @(YES), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here.
| AppExtensionGeneratedPasswordRequireDigitsKey: @(YES), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
| AppExtensionGeneratedPasswordRequireDigitsKey: @(YES), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here..
| AppExtensionGeneratedPasswordRequireDigitsKey: @(YES), | ||
|
|
||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list bellow). Passing NO with will exclude symbols from the generated password. | ||
| // If YES, the 1Password will guarantee that the generated password will contain at least one symbol (See the list below). Passing NO will not exclude symbols from the generated password. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and lastly here.
Uh oh!
There was an error while loading. Please reload this page.