-
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: [FC-0047] Improved Dashboard Level Navigation #308
Changes from 22 commits
d9d0f95
1ecc961
fadede8
34ab0d5
69c7719
7854c1c
f86fb51
77d3249
7e219d2
baac26c
3c262ec
5e04ae6
58da684
83506fc
f9069a0
45aa988
b22bb30
e097f31
c03832f
a0496bc
e62b712
96b7795
9454c60
31b3843
7ef6b77
3fd6657
465bafa
2f02b39
0ee9702
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,7 @@ import org.openedx.course.presentation.unit.container.CourseUnitContainerFragmen | |
import org.openedx.course.presentation.unit.video.VideoFullScreenFragment | ||
import org.openedx.course.presentation.unit.video.YoutubeVideoFullScreenFragment | ||
import org.openedx.course.settings.download.DownloadQueueFragment | ||
import org.openedx.courses.presentation.AllEnrolledCoursesFragment | ||
import org.openedx.dashboard.presentation.DashboardRouter | ||
import org.openedx.discovery.presentation.DiscoveryRouter | ||
import org.openedx.discovery.presentation.NativeDiscoveryFragment | ||
|
@@ -122,13 +123,33 @@ class AppRouter : AuthRouter, DiscoveryRouter, DashboardRouter, CourseRouter, Di | |
replaceFragmentWithBackStack(fm, UpgradeRequiredFragment()) | ||
} | ||
|
||
override fun navigateToAllEnrolledCourses(fm: FragmentManager) { | ||
replaceFragmentWithBackStack(fm, AllEnrolledCoursesFragment()) | ||
} | ||
|
||
override fun getProgramFragmentInstance(): Fragment { | ||
return ProgramFragment(myPrograms = true, isNestedFragment = true) | ||
} | ||
|
||
override fun navigateToCourseInfo( | ||
fm: FragmentManager, | ||
courseId: String, | ||
infoType: String, | ||
) { | ||
replaceFragmentWithBackStack(fm, CourseInfoFragment.newInstance(courseId, infoType)) | ||
} | ||
|
||
override fun navigateToCourseOutline( | ||
fm: FragmentManager, | ||
courseId: String, | ||
courseTitle: String, | ||
enrollmentMode: String | ||
) { | ||
replaceFragmentWithBackStack( | ||
fm, | ||
CourseContainerFragment.newInstance(courseId, courseTitle, enrollmentMode) | ||
) | ||
} | ||
//endregion | ||
|
||
//region DashboardRouter | ||
|
@@ -138,15 +159,17 @@ class AppRouter : AuthRouter, DiscoveryRouter, DashboardRouter, CourseRouter, Di | |
courseId: String, | ||
courseTitle: String, | ||
enrollmentMode: String, | ||
openDates: Boolean, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It will be great if we can use EnumTypes for tabs instead a boolean. |
||
openBlock: String | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can use |
||
) { | ||
replaceFragmentWithBackStack( | ||
fm, | ||
CourseContainerFragment.newInstance(courseId, courseTitle, enrollmentMode) | ||
CourseContainerFragment.newInstance(courseId, courseTitle, enrollmentMode, openDates, openBlock) | ||
) | ||
} | ||
|
||
override fun navigateToEnrolledProgramInfo(fm: FragmentManager, pathId: String) { | ||
replaceFragmentWithBackStack(fm, ProgramFragment.newInstance(pathId)) | ||
replaceFragmentWithBackStack(fm, ProgramFragment(true)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need pathId for programs. |
||
} | ||
|
||
override fun navigateToNoAccess( | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,15 +11,16 @@ import androidx.viewpager2.widget.ViewPager2 | |
import kotlinx.coroutines.launch | ||
import org.koin.android.ext.android.inject | ||
import org.koin.androidx.viewmodel.ext.android.viewModel | ||
import org.openedx.app.adapter.MainNavigationFragmentAdapter | ||
import org.openedx.app.databinding.FragmentMainBinding | ||
import org.openedx.core.adapter.NavigationFragmentAdapter | ||
import org.openedx.core.config.Config | ||
import org.openedx.core.config.DashboardConfig | ||
import org.openedx.core.presentation.global.app_upgrade.UpgradeRequiredFragment | ||
import org.openedx.core.presentation.global.viewBinding | ||
import org.openedx.dashboard.presentation.DashboardFragment | ||
import org.openedx.dashboard.presentation.ListDashboardFragment | ||
import org.openedx.discovery.presentation.DiscoveryNavigator | ||
import org.openedx.discovery.presentation.DiscoveryRouter | ||
import org.openedx.discovery.presentation.program.ProgramFragment | ||
import org.openedx.learn.presentation.LearnFragment | ||
import org.openedx.profile.presentation.profile.ProfileFragment | ||
|
||
class MainFragment : Fragment(R.layout.fragment_main) { | ||
|
@@ -29,7 +30,7 @@ class MainFragment : Fragment(R.layout.fragment_main) { | |
private val router by inject<DiscoveryRouter>() | ||
private val config by inject<Config>() | ||
|
||
private lateinit var adapter: MainNavigationFragmentAdapter | ||
private lateinit var adapter: NavigationFragmentAdapter | ||
|
||
override fun onCreate(savedInstanceState: Bundle?) { | ||
super.onCreate(savedInstanceState) | ||
|
@@ -47,24 +48,19 @@ class MainFragment : Fragment(R.layout.fragment_main) { | |
|
||
binding.bottomNavView.setOnItemSelectedListener { | ||
when (it.itemId) { | ||
R.id.fragmentHome -> { | ||
viewModel.logDiscoveryTabClickedEvent() | ||
R.id.fragmentLearn -> { | ||
viewModel.logMyCoursesTabClickedEvent() | ||
binding.viewPager.setCurrentItem(0, false) | ||
} | ||
|
||
R.id.fragmentDashboard -> { | ||
viewModel.logMyCoursesTabClickedEvent() | ||
R.id.fragmentHome -> { | ||
viewModel.logDiscoveryTabClickedEvent() | ||
binding.viewPager.setCurrentItem(1, false) | ||
} | ||
|
||
R.id.fragmentPrograms -> { | ||
viewModel.logMyProgramsTabClickedEvent() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please trigger the analytics accordingly. |
||
binding.viewPager.setCurrentItem(2, false) | ||
} | ||
|
||
R.id.fragmentProfile -> { | ||
viewModel.logProfileTabClickedEvent() | ||
binding.viewPager.setCurrentItem(3, false) | ||
binding.viewPager.setCurrentItem(2, false) | ||
} | ||
} | ||
true | ||
|
@@ -105,18 +101,15 @@ class MainFragment : Fragment(R.layout.fragment_main) { | |
binding.viewPager.orientation = ViewPager2.ORIENTATION_HORIZONTAL | ||
binding.viewPager.offscreenPageLimit = 4 | ||
|
||
val discoveryFragment = DiscoveryNavigator(viewModel.isDiscoveryTypeWebView) | ||
.getDiscoveryFragment() | ||
val programFragment = if (viewModel.isProgramTypeWebView) { | ||
ProgramFragment(true) | ||
} else { | ||
InDevelopmentFragment() | ||
val discoveryFragment = DiscoveryNavigator(viewModel.isDiscoveryTypeWebView).getDiscoveryFragment() | ||
val dashboardFragment = when (config.getDashboardConfig().getType()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can follow the same approach for Dashboard fragment as we are doing for discoveryFragment using a navigator. |
||
DashboardConfig.DashboardType.LIST -> ListDashboardFragment() | ||
DashboardConfig.DashboardType.PRIMARY_COURSE -> LearnFragment() | ||
} | ||
|
||
adapter = MainNavigationFragmentAdapter(this).apply { | ||
adapter = NavigationFragmentAdapter(this).apply { | ||
addFragment(dashboardFragment) | ||
addFragment(discoveryFragment) | ||
addFragment(DashboardFragment()) | ||
addFragment(programFragment) | ||
addFragment(ProfileFragment()) | ||
} | ||
binding.viewPager.adapter = adapter | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import androidx.lifecycle.LifecycleOwner | |
import androidx.lifecycle.LiveData | ||
import androidx.lifecycle.MutableLiveData | ||
import androidx.lifecycle.viewModelScope | ||
import kotlinx.coroutines.FlowPreview | ||
import kotlinx.coroutines.flow.MutableSharedFlow | ||
import kotlinx.coroutines.flow.SharedFlow | ||
import kotlinx.coroutines.flow.asSharedFlow | ||
|
@@ -31,15 +32,17 @@ class MainViewModel( | |
|
||
val isDiscoveryTypeWebView get() = config.getDiscoveryConfig().isViewTypeWebView() | ||
|
||
val isProgramTypeWebView get() = config.getProgramConfig().isViewTypeWebView() | ||
omerhabib26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@OptIn(FlowPreview::class) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No usage found for this annotation. |
||
override fun onCreate(owner: LifecycleOwner) { | ||
super.onCreate(owner) | ||
notifier.notifier.onEach { | ||
if (it is NavigationToDiscovery) { | ||
_navigateToDiscovery.emit(true) | ||
notifier.notifier | ||
.onEach { | ||
if (it is NavigationToDiscovery) { | ||
_navigateToDiscovery.emit(true) | ||
} | ||
} | ||
}.distinctUntilChanged().launchIn(viewModelScope) | ||
.distinctUntilChanged() | ||
.launchIn(viewModelScope) | ||
} | ||
|
||
fun enableBottomBar(enable: Boolean) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,9 +29,11 @@ import org.openedx.course.presentation.unit.video.VideoUnitViewModel | |
import org.openedx.course.presentation.unit.video.VideoViewModel | ||
import org.openedx.course.presentation.videos.CourseVideoViewModel | ||
import org.openedx.course.settings.download.DownloadQueueViewModel | ||
import org.openedx.courses.presentation.AllEnrolledCoursesViewModel | ||
import org.openedx.courses.presentation.PrimaryCourseViewModel | ||
import org.openedx.dashboard.data.repository.DashboardRepository | ||
import org.openedx.dashboard.domain.interactor.DashboardInteractor | ||
import org.openedx.dashboard.presentation.DashboardViewModel | ||
import org.openedx.dashboard.presentation.ListDashboardViewModel | ||
import org.openedx.discovery.data.repository.DiscoveryRepository | ||
import org.openedx.discovery.domain.interactor.DiscoveryInteractor | ||
import org.openedx.discovery.presentation.NativeDiscoveryViewModel | ||
|
@@ -49,6 +51,7 @@ import org.openedx.discussion.presentation.search.DiscussionSearchThreadViewMode | |
import org.openedx.discussion.presentation.threads.DiscussionAddThreadViewModel | ||
import org.openedx.discussion.presentation.threads.DiscussionThreadsViewModel | ||
import org.openedx.discussion.presentation.topics.DiscussionTopicsViewModel | ||
import org.openedx.learn.presentation.LearnViewModel | ||
import org.openedx.profile.data.repository.ProfileRepository | ||
import org.openedx.profile.domain.interactor.ProfileInteractor | ||
import org.openedx.profile.domain.model.Account | ||
|
@@ -114,9 +117,12 @@ val screenModule = module { | |
} | ||
viewModel { RestorePasswordViewModel(get(), get(), get(), get()) } | ||
|
||
factory { DashboardRepository(get(), get(), get()) } | ||
factory { DashboardRepository(get(), get(), get(), get()) } | ||
factory { DashboardInteractor(get()) } | ||
viewModel { DashboardViewModel(get(), get(), get(), get(), get(), get(), get()) } | ||
viewModel { ListDashboardViewModel(get(), get(), get(), get(), get(), get(), get()) } | ||
viewModel { PrimaryCourseViewModel(get(), get(), get(), get(), get(), get(), get()) } | ||
viewModel { AllEnrolledCoursesViewModel(get(), get(), get(), get(), get(), get(), get()) } | ||
viewModel { LearnViewModel(get(), get()) } | ||
|
||
factory { DiscoveryRepository(get(), get(), get()) } | ||
factory { DiscoveryInteractor(get()) } | ||
|
@@ -192,10 +198,11 @@ val screenModule = module { | |
get() | ||
) | ||
} | ||
viewModel { (courseId: String, courseTitle: String, enrollmentMode: String) -> | ||
viewModel { (courseId: String, courseTitle: String, enrollmentMode: String, openBlock: String) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, |
||
CourseContainerViewModel( | ||
courseId, | ||
courseTitle, | ||
openBlock, | ||
enrollmentMode, | ||
get(), | ||
get(), | ||
|
@@ -224,6 +231,7 @@ val screenModule = module { | |
get(), | ||
get(), | ||
get(), | ||
get() | ||
) | ||
} | ||
viewModel { (courseId: String) -> | ||
|
@@ -265,6 +273,7 @@ val screenModule = module { | |
get(), | ||
get(), | ||
get(), | ||
get() | ||
) | ||
} | ||
viewModel { (courseId: String) -> BaseVideoViewModel(courseId, get()) } | ||
|
@@ -304,6 +313,7 @@ val screenModule = module { | |
get(), | ||
get(), | ||
get(), | ||
get() | ||
) | ||
} | ||
viewModel { (courseId: String, handoutsType: String) -> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
omerhabib26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<selector xmlns:android="http://schemas.android.com/apk/res/android"> | ||
<item android:state_checked="true" android:color="@color/checked_tab_item" /> | ||
<item android:state_checked="false" android:color="@color/unchecked_tab_item" /> | ||
</selector> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a line at EOF. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,38 +1,10 @@ | ||
<vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
android:width="24dp" | ||
android:height="24dp" | ||
android:viewportWidth="24" | ||
android:viewportHeight="24"> | ||
<group> | ||
<clip-path | ||
android:pathData="M0,0h24v24h-24z"/> | ||
<path | ||
android:pathData="M4,4H10V12H4V4Z" | ||
android:strokeLineJoin="round" | ||
android:strokeWidth="1.75" | ||
android:fillColor="#00000000" | ||
android:strokeColor="#3C68FF" | ||
android:strokeLineCap="round"/> | ||
<path | ||
android:pathData="M4,16H10V20H4V16Z" | ||
android:strokeLineJoin="round" | ||
android:strokeWidth="1.75" | ||
android:fillColor="#00000000" | ||
android:strokeColor="#3C68FF" | ||
android:strokeLineCap="round"/> | ||
<path | ||
android:pathData="M14,12H20V20H14V12Z" | ||
android:strokeLineJoin="round" | ||
android:strokeWidth="1.75" | ||
android:fillColor="#00000000" | ||
android:strokeColor="#3C68FF" | ||
android:strokeLineCap="round"/> | ||
<path | ||
android:pathData="M14,4H20V8H14V4Z" | ||
android:strokeLineJoin="round" | ||
android:strokeWidth="1.75" | ||
android:fillColor="#00000000" | ||
android:strokeColor="#3C68FF" | ||
android:strokeLineCap="round"/> | ||
</group> | ||
android:width="20dp" | ||
omerhabib26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
android:height="17dp" | ||
android:viewportWidth="20" | ||
android:viewportHeight="17"> | ||
<path | ||
android:pathData="M19.81,2.71C19.83,2.77 19.85,2.83 19.85,2.89C19.85,2.93 19.87,3 19.87,3V15.99C19.87,16 19.867,16.007 19.865,16.015C19.862,16.022 19.86,16.03 19.86,16.04C19.86,16.1 19.85,16.15 19.83,16.21C19.824,16.228 19.818,16.247 19.813,16.264C19.802,16.306 19.791,16.345 19.77,16.38C19.75,16.4 19.75,16.41 19.75,16.43L19.75,16.43C19.71,16.49 19.67,16.55 19.62,16.6L19.61,16.61C19.53,16.69 19.45,16.74 19.36,16.78C19.355,16.782 19.35,16.785 19.344,16.788C19.326,16.798 19.305,16.81 19.29,16.81C19.19,16.85 19.09,16.87 18.99,16.87C18.89,16.87 18.79,16.85 18.69,16.81C18.68,16.805 18.667,16.8 18.655,16.795C18.642,16.79 18.63,16.785 18.62,16.78L18.56,16.75C16.1,15.33 12.91,15.33 10.44,16.75C10.426,16.763 10.413,16.768 10.4,16.772C10.393,16.774 10.386,16.777 10.38,16.78C10.374,16.783 10.369,16.786 10.363,16.79C10.348,16.799 10.331,16.81 10.31,16.81C10.12,16.88 9.91,16.88 9.71,16.81C9.7,16.805 9.687,16.8 9.675,16.795C9.662,16.79 9.65,16.785 9.64,16.78L9.58,16.75C7.12,15.33 3.93,15.33 1.46,16.75C1.44,16.77 1.43,16.77 1.41,16.77C1.375,16.791 1.335,16.802 1.294,16.814C1.276,16.819 1.258,16.824 1.24,16.83C1.218,16.834 1.198,16.838 1.178,16.843C1.143,16.852 1.108,16.86 1.07,16.86C1.06,16.87 1.04,16.87 1.02,16.87C1,16.87 0.982,16.865 0.965,16.86C0.947,16.855 0.93,16.85 0.91,16.85C0.85,16.85 0.79,16.84 0.74,16.82C0.68,16.8 0.63,16.77 0.58,16.74L0.58,16.74C0.564,16.729 0.548,16.719 0.531,16.708C0.503,16.692 0.474,16.675 0.45,16.65C0.413,16.621 0.387,16.586 0.36,16.55C0.35,16.537 0.34,16.523 0.33,16.51C0.32,16.495 0.307,16.483 0.295,16.47C0.282,16.458 0.27,16.445 0.26,16.43C0.24,16.41 0.24,16.4 0.24,16.38C0.217,16.343 0.206,16.306 0.194,16.265C0.189,16.25 0.185,16.236 0.18,16.22C0.176,16.199 0.171,16.178 0.167,16.159C0.158,16.123 0.15,16.089 0.15,16.05C0.14,16.03 0.14,16 0.14,16V3C0.14,2.98 0.145,2.962 0.15,2.945C0.155,2.927 0.16,2.91 0.16,2.89C0.17,2.83 0.18,2.77 0.2,2.71C0.22,2.66 0.24,2.61 0.27,2.56C0.278,2.546 0.286,2.532 0.293,2.518C0.313,2.483 0.331,2.449 0.36,2.42C0.402,2.377 0.438,2.349 0.478,2.317C0.485,2.311 0.492,2.306 0.5,2.3C0.515,2.29 0.527,2.277 0.54,2.265C0.552,2.252 0.565,2.24 0.58,2.23C0.595,2.22 0.61,2.212 0.625,2.205C0.64,2.197 0.655,2.19 0.67,2.18C3.51,0.59 7.12,0.51 10.01,1.99C12.91,0.51 16.51,0.59 19.35,2.18C19.365,2.19 19.38,2.197 19.395,2.205C19.41,2.212 19.425,2.22 19.44,2.23C19.455,2.24 19.467,2.252 19.48,2.265C19.492,2.277 19.505,2.29 19.52,2.3C19.535,2.316 19.553,2.33 19.57,2.344C19.597,2.367 19.625,2.39 19.65,2.42C19.665,2.445 19.68,2.467 19.695,2.49C19.71,2.512 19.725,2.535 19.74,2.56C19.77,2.61 19.79,2.66 19.81,2.71L19.81,2.71ZM9.5,4.12C9.5,3.84 9.72,3.62 10,3.62C10.28,3.62 10.5,3.84 10.5,4.12V14.4C10.5,14.68 10.28,14.9 10,14.9C9.72,14.9 9.5,14.68 9.5,14.4V4.12Z" | ||
android:fillColor="#3F68F8" | ||
android:fillType="evenOdd"/> | ||
</vector> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,17 +14,17 @@ | |
app:layout_constraintStart_toStartOf="parent" | ||
app:layout_constraintTop_toTopOf="parent" /> | ||
|
||
|
||
<com.google.android.material.bottomnavigation.BottomNavigationView | ||
android:id="@+id/bottom_nav_view" | ||
android:layout_width="match_parent" | ||
android:layout_height="wrap_content" | ||
app:itemIconTint="@color/bottom_nav_color" | ||
omerhabib26 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
app:itemTextColor="@color/bottom_nav_color" | ||
app:labelVisibilityMode="labeled" | ||
app:layout_constraintBottom_toBottomOf="parent" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintHorizontal_bias="0.5" | ||
app:layout_constraintStart_toStartOf="parent" | ||
app:menu="@menu/bottom_view_menu" /> | ||
|
||
|
||
</androidx.constraintlayout.widget.ConstraintLayout> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a line at EOF. |
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.
instead of defining a getter for
ProgramFragment
please use ProgramFragment.newInstance() and pass the required flags as parameters with existing.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 added isNestedFragment to the class constructor because myPrograms was already there. Changing of ProgramFragment wasn't included in our SOW. We can revisit that later and refactor the code in a different scope.
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.
@PavloNetrebchuk please create a ticket for this and mentioned it for tracking.
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.
Please create a ticket for this and mentioned here so we can keep a track on this.