Closed kefahB closed 5 years ago
Hi!
Can you please remove the .history
folder from the commit?
Hi,
Sure...
@EddyVerbruggen .. I do
Thanks! I'll merge and test this shortly.
@kefahB I understand the issue with clearing the keychain is iOS-only, but this implementation should be added for Android as well. I'll take care of that.
@EddyVerbruggen may be you can find another function name ? instead of onFirstRun
or like you want ... and also may be make correction of me english on README.MD :-D
That is my first PR :-)
You did a great job! I'll change a few small things, but for a first PR I'd say: 💪👏
Hi @EddyVerbruggen, I remark an issue on the concept to do stuff with "First run", you declare this on constructor :
constructor() {
this.isFirst = applicationSettings.getBoolean(SecureStorageCommon.IS_FIRST_RUN, true);
if (this.isFirst) {
applicationSettings.setBoolean(SecureStorageCommon.IS_FIRST_RUN, false);
}
}
Since this is declared on constructor we can not do some think with the first run because you set immediately the SecureStorageCommon.IS_FIRST_RUN
to false, if I call the isFirstRun()
or clearAllOnFirstRun()
or clearAllOnFirstRunSync()
will return always false
cause t
it is already set on the constructor.
Maybe change refactor the tow method isFirstRunSync
and isFirstRunSync
and delete those lines from constructor :
public isFirstRunSync(): boolean {
if (this.isFirst) {
applicationSettings.setBoolean(SecureStorageCommon.IS_FIRST_RUN, false);
return true;
}
return false;
}
public isFirstRun(): Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
if (this.isFirst) {
applicationSettings.setBoolean(SecureStorageCommon.IS_FIRST_RUN, false);
resolve(true);
}
resolve(false);
});
}
Hi @EddyVerbruggen,
I added 3 methods on the plugin :
clearAllOnFirstRun
andclearAllOnFirstRunSync
will clearall
data due toremoveAll
andremoveAllSync
anddoOnFirstRun
give the user the ability to do other stuff (than remove data) on the first run...Excuse my English is bad, feel free to notify me about potential changes...