Add loginPromptManager #73
Conversation
webflows/src/main/java/com/schibsted/account/webflows/loginPrompt/LoginPromptManager.kt
Outdated
Show resolved
Hide resolved
webflows/src/main/java/com/schibsted/account/webflows/loginPrompt/LoginPromptManager.kt
Outdated
Show resolved
Hide resolved
| // placeholder TODO: add call to handleAuthenticationResponse | ||
| dismiss() | ||
| binding.loginPromptAuth.setOnClickListener { | ||
| startActivity(loginPromptConfig.client.getAuthenticationIntent(this.requireContext())) |
There was a problem hiding this comment.
decided to use Fragment context on all actions instead of using parent activity context. This should simplify logic quite a bit
There was a problem hiding this comment.
To be precise - Fragment is not a context, look on "requireContext()" implementation - it's just return hostActivity contxt (assuming fragment is attached)
| fun initializeLoginPrompt(): LoginPromptFragment { | ||
| var loginPromptFragment = LoginPromptFragment() | ||
| loginPromptFragment.loginPromptConfig = this.loginPromptConfig | ||
|
|
||
| return loginPromptFragment |
There was a problem hiding this comment.
Created a separate initializer function to allow sending the fragment object to the caller, instead of having the object in the LoginPromptManager
webflows/src/main/java/com/schibsted/account/webflows/loginPrompt/LoginPromptManager.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Approving with notes. As this is not a final state & merge to feature branch I don't want to block development, but I believe it's worth to address comments.
Refer to fragments documentation https://developer.android.com/guide/fragments/lifecycle. Topic is quite complex, and it's important to know how fragment classes behave (not fully as regular Kotlin objects).
| // placeholder TODO: add call to handleAuthenticationResponse | ||
| dismiss() | ||
| binding.loginPromptAuth.setOnClickListener { | ||
| startActivity(loginPromptConfig.client.getAuthenticationIntent(this.requireContext())) |
There was a problem hiding this comment.
To be precise - Fragment is not a context, look on "requireContext()" implementation - it's just return hostActivity contxt (assuming fragment is attached)
| * @param supportFragmentManager Calling entity's fragment manager. | ||
| */ | ||
| fun showLoginPrompt( | ||
| loginPromptFragment: LoginPromptFragment, |
There was a problem hiding this comment.
I think it's internal that can be hidden. You just pass FragmentManager and LoginPromptManager can check if there is loginPromptFragment already in it or need to create new instance. This will work when screens are recreated on configurationChange (like device rotation)
| class LoginPromptFragment : BottomSheetDialogFragment() { | ||
| private var _binding: LoginPromptBinding? = null | ||
| private val binding get() = _binding!! | ||
| lateinit var loginPromptConfig: LoginPromptConfig |
There was a problem hiding this comment.
Be caution with such fields. If fragment is recreated (after rotation, i.e.) this will be in uninitialized state. BAsically settin fields on fragment like this
fun initializeLoginPrompt(): LoginPromptFragment {
var loginPromptFragment = LoginPromptFragment()
loginPromptFragment.loginPromptConfig = this.loginPromptConfig
Is not a good practice. It's better to pass data via arguments (in bundle) or keep those in ViewModel.
* 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>
Add manager class responsible for showing the login prompt and for configuring prompt's behaviour.
Manager requires a
LoginPromptConfigparameter when calling the constructor which contains the client and context of the calling app as well as a boolean toggle which controls prompt's cancellable behaviour.