-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Pre-login Exploration Enrolment Handling #169
feat: Pre-login Exploration Enrolment Handling #169
Conversation
whatsnew/src/main/java/org/openedx/whatsnew/presentation/whatsnew/WhatsNewFragment.kt
Outdated
Show resolved
Hide resolved
I have started its review 💻 |
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.
The App is working fine as expected only some code improvements and suggestions.
Edge Case: What will be the flow of Enroll in a course if a user sign-in through the discovery screen and what's new is enabled?
auth/src/main/java/org/openedx/auth/presentation/signup/SignUpViewModel.kt
Show resolved
Hide resolved
discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryViewModel.kt
Outdated
Show resolved
Hide resolved
discovery/src/main/java/org/openedx/discovery/presentation/search/CourseSearchFragment.kt
Show resolved
Hide resolved
discovery/src/main/java/org/openedx/discovery/presentation/DiscoveryRouter.kt
Outdated
Show resolved
Hide resolved
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.
only a minor nit
course?.let { | ||
_uiState.value = CourseDetailsUIState.CourseData( | ||
course = it, | ||
isUserLoggedIn = corePreferences.user != null |
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.
we can use isUserLoggedIn
fixes: LEARNER-9751
- On successfully logging in or registering the user should return to the course about page in the native experience. fixes: LEARNER-9751
- Sticky bottom banner having the register and sign in buttons a they explore discovery without logging in fixes: LEARNER-9751
Fixes: LEARNER-9751
Taking on PR responsibilities during Farhan's leave for personal reasons. Ready to handle communication and address queries. |
2c18f1f
to
127880c
Compare
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.
LGTM 🚀
Pre-login Exploration Enrolment Handling
Screen_recording_20231218_174011.webm