call loginPrompt from Client #84
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clean main activity Run requestLoginPrompt in background thread
xserxses
approved these changes
Aug 9, 2023
| supportFragmentManager: FragmentManager, | ||
| isCancelable: Boolean = true | ||
| ) { | ||
| val loginPromptManager = LoginPromptManager(LoginPromptConfig(this, isCancelable)) |
There was a problem hiding this comment.
Yeah this code will work but I would personally for mit differently, extract userHasSessionOnDevice as function and create loginPromptManager only if it's needed. sth like
if( userHasSessionOnDevice(context) ) {
LoginPromptManager(LoginPromptConfig(this, isCancelable)).showLoginPrompt(supportFragmentManager)
}
It's not blocking comment as it's a matter of preference & style :).
BTW I assume there will be more conditions there? Or deciding on when to call requestLoginPrompt will rest on clinet hands?
There was a problem hiding this comment.
i think the only other check that'd be needed would be to not show this prompt if user is already logged in, and I think this should be done by mobile app before calling requestLoginPrompt, because for that you need to have access to User.isLoggedIn() which we don't have under Client
There was a problem hiding this comment.
But from the product perspective moving other conditions (like only 3'rd time when app is opened, every week etc.) this will result in different enviroment of showing prompt in different apps. That will make product metrics like conversion uncombable between brands (I mean i.e. Blocket & Aftonbladet).
Technically it's not an issue to code our conditions it on Hermes side but we are not collaborating with all SDK users.
bogdan-niculescu-sch
added a commit
that referenced
this pull request
Oct 6, 2023
* feat: login prompt content provider implementation * refactor: content provider interaction logic moved to SessionInfoManager class * feat: fetching content provider authorities from the package manager; checking is session on the device exists * chore: refactored loginPrompt classes code style * refactor: move sessionInfoManager to sharedPreferencesStorage class * refactor: fix compatibility issues with PackageManager.MATCH_ALL * refactor: login prompt content provider refactor on xserxses comments * Add loginPromptManager (#73) * Add loginPromptManager * Add translations (#82) * call loginPrompt from Client (#84) * call loginPrompt from Client clean main activity Run requestLoginPrompt in background thread * Login promp tracking (#80) * Propose SchibstedAccountTracking public and internal API * Present tracking API in ExampleApp * Document API * More readable logging * Initial events for show/hide login prompt * Tracking events for clicks * Update events (#86) * Add final tracking events --------- Co-authored-by: filip-misztal <filip.misztal@schibsted.com> Co-authored-by: bogdan-niculescu-sch <104439589+bogdan-niculescu-sch@users.noreply.github.com> * Fill in readme (#89) - apply outstanding review remark * Throw different error type if user cancels login (#83) * Throw different error type if user cancels login * Check error before state - throw NotAuthed.CancelledByUser error * Fix typo * Update webflows/src/main/java/com/schibsted/account/webflows/util/Util.kt Co-authored-by: Filip Misztal <filip.jan.misztal@gmail.com> * Review remarks before merge (#90) * initial cleanup * make tracking thread safe - small review remarks * cleanup layout * code cleanup * add localized logos * Update logos * fix dialog showing check * apply review remark * Fix query period on content provider getSessions * Change DB primary key to packageName for content provider (#91) * Change db primary key to packagename for content provider * On conclict - replace with new values * user writable database for writting --------- Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * Use "use" to be more safe in case of failures + Nice syntax (#92) * Use use to be more safe * Even more idiomatic Kotlin --------- Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * Send cancel event on eid user cancel (#93) * Send cancel event on eid user cancel * Small Readme update * Login prompt crash (#94) * Pass intent via argument instead of whole client * Prevent adding twice --------- Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * add support for norsk bokmal and norsk nynorsk (#96) * add serverUrl to content provider query (#95) * check for local session before showing login prompt (#97) * check for local session before showing login prompt * apply review remark * Check also for presence - not only callback type (#98) Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * Dismiss prompt when login is initiated (#99) Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * Remove login promp on login click (#100) * Dismiss prompt when login is initiated * Better place * This is no longer needed --------- Co-authored-by: filip-misztal <filip.misztal@schibsted.com> * add extra properties for events (#101) * add extra properties for events * Update readme and minor cleanup --------- Co-authored-by: wbaklazec-sch <105283956+wbaklazec-sch@users.noreply.github.com> Co-authored-by: bogdan-niculescu-sch <104439589+bogdan-niculescu-sch@users.noreply.github.com> Co-authored-by: filip-misztal <filip.misztal@schibsted.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
clean main activity
Run requestLoginPrompt in background thread