feat: [FC-0047] Course progress and collapsing sections by PavloNetrebchuk · Pull Request #323 · openedx/openedx-app-android

PavloNetrebchuk · 2024-05-21T20:03:33Z

https://www.figma.com/design/iZ56YMjbRMShCCDxqrqRrR/Open-edX-Mobile-App-All-Screens-v2.1?node-id=9146-62028&t=9uhsH3nbuxmXNajx-4

Light Theme	Dark Theme

Downloading icon

Not Downloaded	Downloading	Downloaded

Completion

Light Theme	Dark Theme

openedx-webhooks · 2024-05-21T20:03:38Z

Thanks for the pull request, @PavloNetrebchuk! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 · 2024-05-23T15:02:10Z

Hi @PavloNetrebchuk! Is this for an FC project with Axim?

PavloNetrebchuk · 2024-05-24T08:40:17Z

Hi @PavloNetrebchuk! Is this for an FC project with Axim?

Hello @mphilbrick211
Yes

…edx#262) chore: enhance app theme capability for prod edX theme/branding - Integrate Program config updates - theming/branding code improvements for light and dark modes - Force dark mode for the WebView (beta version) - No major change in the Open edX theme fixes: LEARNER-9783

* feat: Created calendar setting screen * feat: CalendarAccessDialog * feat: NewCalendarDialog * fix: Fixes according to PR feedback

* feat: Created Learn screen. Added course/program navigation. Added endpoint for UserCourses screen. * feat: Added primary course card * feat: Added start/resume course button * feat: Added alignment items * feat: Fix future assignment date, add courses list, add onSearch and onCourse clicks * feat: Add feature flag for enabling new/old dashboard screen, add UserCoursesScreen onClick methods * feat: Create AllEnrolledCoursesFragment. Add endpoint parameters * feat: AllEnrolledCoursesFragment UI * feat: Minor code refactoring, show cached data if no internet connection * feat: UserCourses screen data caching * feat: Dashboard * refactor: Dashboard type flag change, start course button change * feat: Added programs fragment to LearnFragment viewPager * feat: Empty states and settings button * fix: Number of courses * fix: Minor UI changes * fix: Fixes according to designer feedback * fix: Fixes after demo * refactor: Move CourseContainerTab * fix: Fixes according to PR feedback * fix: Fixes according to PR feedback * feat: added a patch from Omer Habib * fix: Fixes according to PR feedback

…and_sections_ui # Conflicts: # core/src/main/java/org/openedx/core/domain/model/Progress.kt # core/src/openedx/org/openedx/core/ui/theme/Colors.kt # course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineScreen.kt # course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt # course/src/main/java/org/openedx/course/presentation/section/CourseSectionFragment.kt # course/src/main/java/org/openedx/course/presentation/ui/CourseUI.kt # course/src/main/java/org/openedx/course/presentation/ui/CourseVideosUI.kt # dashboard/src/main/java/org/openedx/courses/presentation/DashboardGalleryView.kt # dashboard/src/main/res/values/strings.xml

volodymyr-chekyrta

Good overall 👍 , please address the following issues:

volodymyr-chekyrta · 2024-05-31T09:22:11Z

+    @SerializedName("num_points_possible")
+    val numPointsPossible: Float?,
+) {
+    fun mapToDomain() = org.openedx.core.domain.model.AssignmentProgress(


Please move it to the imports section

volodymyr-chekyrta · 2024-05-31T09:23:28Z

+    @SerializedName("due")
+    val due: String?
 ) {
    fun mapToDomain(blockData: Map<String, org.openedx.core.data.model.Block>): Block {


Please move it to the imports section

volodymyr-chekyrta · 2024-05-31T09:23:55Z

+    @ColumnInfo("num_points_possible")
+    val numPointsPossible: Float?,
+) {
+    fun mapToDomain() = org.openedx.core.domain.model.AssignmentProgress(


Please move it to the imports section

volodymyr-chekyrta · 2024-05-31T09:27:29Z

+    fun getProgress(): Float = try {
+        assignmentsCompleted.toFloat() / totalAssignmentsCount.toFloat()
+    } catch (_: ArithmeticException) {
+        0f
+    }


What about replacing it with a computed property, let's say value, so we can call progress like this progress.value instead of progress.getProgress()?

volodymyr-chekyrta · 2024-05-31T09:37:28Z

    <string name="core_date_format_tomorrow" tools:ignore="MissingTranslation">Tomorrow</string>
    <string name="core_date_format_yesterday" tools:ignore="MissingTranslation">Yesterday</string>
    <string name="core_date_format_days_ago" tools:ignore="MissingTranslation">%1$s days ago</string>
+    <string name="core_date_format_past_due_assignment" tools:ignore="MissingTranslation">Past due assignment</string>


Unused resource core_date_format_past_due_assignment

volodymyr-chekyrta · 2024-05-31T09:40:18Z

+    <string name="core_date_format_assignment_due_in" tools:ignore="MissingTranslation">Due in %1$s days</string>
+    <string name="core_date_format_assignment_due_days_ago" tools:ignore="MissingTranslation">Due %1$s days ago</string>


Please use plurals here for correct translation into other languages

volodymyr-chekyrta · 2024-05-31T09:43:56Z

-    Column(Modifier
-        .clickable { onClick(block) }
-        .background(if (block.isCompleted()) MaterialTheme.appColors.surface else Color.Transparent)
+    val due = block.due?.let { TimeUtils.getAssignmentFormattedDate(LocalContext.current, it) }


If I'm not mistaken, it's better to use remember when you transform dates in compose because it creates a calendar instance on each recomposition.

volodymyr-chekyrta · 2024-05-31T09:45:08Z

+                    viewModel.courseRouter.navigateToDownloadQueue(
+                        fm = fragmentManager,
+                        viewModel.getDownloadableChildren(blockId)
+                            ?: arrayListOf()
+                    )


can we transform it to the one line viewModel.getDownloadableChildren(blockId) ?: arrayListOf() for better readability?

volodymyr-chekyrta · 2024-05-31T09:46:35Z

Can we use a vector here instead of PNG?
(We can address it in the Offline/Downloading task)

I don't know why, but the vector image I took from Figma shows up as a circle in Android Studio.

Okay, we'll figure it out in the next PRs related to downloading

volodymyr-chekyrta · 2024-05-31T09:58:04Z

Can we remove this odd ripple effect or at least limit it to the section heading where the user taps?

Screen_recording_20240531_125624.mp4

volodymyr-chekyrta · 2024-05-31T10:01:57Z

    <string name="course_delete_while_downloading_confirmation_text" formatted="false">Turning off the switch will stop downloading and delete all downloaded videos for \"%s\"?</string>
    <string name="course_delete_downloads_confirmation_text" formatted="false">Are you sure you want to delete all video(s) for \"%s\"?</string>
    <string name="course_delete_download_confirmation_text" formatted="false">Are you sure you want to delete video(s) for \"%s\"?</string>
+    <string name="course_assignments_complete">%1$s of %2$s  assignments complete</string>


Please use plurals for the word assignments

volodymyr-chekyrta · 2024-05-31T10:05:19Z

 - **WHATS_NEW_ENABLED:** Enables the "What's New" feature to present the latest changes to the user.
 - **SOCIAL_AUTH_ENABLED:** Enables SSO buttons on the SignIn and SignUp screens.
- **COURSE_NESTED_LIST_ENABLED:** Enables an alternative visual representation for the course structure.
+- **COURSE_DROPDOWN_NAVIGATION_ENABLED:** Enables an alternative visual representation for the course structure.


Enables an alternative visual representation for the course structure. -> Enables an alternative navigation through units.

volodymyr-chekyrta · 2024-05-31T11:57:49Z

@PavloNetrebchuk please update the config structure according to the #319

…and_sections_ui # Conflicts: # core/src/main/java/org/openedx/core/config/Config.kt # course/src/main/java/org/openedx/course/presentation/dates/CourseDatesViewModel.kt # course/src/main/java/org/openedx/course/presentation/outline/CourseOutlineViewModel.kt # course/src/main/java/org/openedx/course/presentation/ui/CourseUI.kt # course/src/main/java/org/openedx/course/presentation/unit/container/CourseUnitContainerViewModel.kt # course/src/main/java/org/openedx/course/presentation/videos/CourseVideoViewModel.kt # course/src/test/java/org/openedx/course/presentation/outline/CourseOutlineViewModelTest.kt # course/src/test/java/org/openedx/course/presentation/videos/CourseVideoViewModelTest.kt # default_config/dev/config.yaml # default_config/prod/config.yaml # default_config/stage/config.yaml

volodymyr-chekyrta

Looks good to me.
PR is ready for product/design review.

sdaitzman · 2024-06-03T17:48:28Z

Hi @PavloNetrebchuk, this looks great overall! A couple of design questions/feedback I wanted to check on:

In light mode, sections should use the light theme shade color #F9FAFB as their background color. In the last screenshot, it looks like it might be using a darker color?
In dark mode, the download icon should use the lighter dark theme accent text color #879FF5 for contrast/accessibility reasons

PavloNetrebchuk · 2024-06-04T09:27:06Z

Hello @sdaitzman,
Thank you for your feedback.

I double-checked the section color, and it is correct. It may appear darker in the screenshot due to compression.
You are right; I used the wrong color previously. It has been corrected now.

sdaitzman · 2024-06-05T14:38:37Z

Thanks, @PavloNetrebchuk! Everything else looks great to me from a design standpoint.

k1rill

LGTM, just nits, they can be ignored

k1rill · 2024-06-06T11:05:20Z

-                viewModel.saveDownloadModels(
-                    FileUtil(context).getExternalAppDir().path, it.id
-                )
+        onDownloadClick = { blocksIds ->


nit: Maybe it's better to move all this to the separate method in the viewModel

k1rill · 2024-06-06T11:07:28Z

+    val subsectionsDownloadedStates = downloadedStateMap.filterKeys { key ->
+        key in (courseSubSections?.map { it.id } ?: emptyList())
+    }.values.toList()
+    val downloadedState =


maybe it's better to use when here

openedx-webhooks · 2024-06-06T12:01:29Z

@PavloNetrebchuk 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

HamzaIsrar12 · 2024-06-07T10:26:21Z

Hey @PavloNetrebchuk, I pulled this branch and noticed that the download button seems a bit buggy. Could you please take a look?

Screenrecorder-2024-06-07-14-45-26-461.mp4

PavloNetrebchuk · 2024-06-07T11:05:19Z

Hello, @HamzaIsrar12
You're right. I only had one downloadable item in the course when I tested it. I'll create a PR with the fix ASAP.

feat: Course Home progress bar

eb64023

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label May 21, 2024

PavloNetrebchuk force-pushed the feat/progress_bar_and_sections_ui branch from e853ad8 to dd56b78 Compare May 22, 2024 11:42

feat: Collapsing course sections

ff0bfd1

PavloNetrebchuk force-pushed the feat/progress_bar_and_sections_ui branch from dd56b78 to ff0bfd1 Compare May 23, 2024 15:24

feat: New download icons

78628f5

volodymyr-chekyrta changed the title ~~Feat: Progress bar and collapsing course sections~~ feat: [FC-0047] Progress bar and collapsing course sections May 24, 2024

PavloNetrebchuk and others added 11 commits May 28, 2024 17:56

feat: show CourseContainerFragment if COURSE_NESTED_LIST_ENABLED false

3d5ff50

fix: course progress bar updating

9f75619

feat: Renamed COURSE_NESTED_LIST_ENABLE feature flag

4989c52

feat: Course home. Moved certificate access.

2ca4f5f

feat: [FC-0047] Calendar main screen and dialogs (openedx#322)

d04c020

* feat: Created calendar setting screen * feat: CalendarAccessDialog * feat: NewCalendarDialog * fix: Fixes according to PR feedback

fix: DiscussionTopicsViewModelTest.kt jUnit test

442496d

fix: assignment dates

23dcabf

fix: Assignment date string

d0feb18

fix: Lint error

b72fc59

PavloNetrebchuk force-pushed the feat/progress_bar_and_sections_ui branch from a9ef278 to b72fc59 Compare May 30, 2024 15:12

PavloNetrebchuk added 2 commits May 30, 2024 18:15

fix: Assignment date string

84dfdba

volodymyr-chekyrta requested changes May 31, 2024

View reviewed changes

volodymyr-chekyrta reviewed May 31, 2024

View reviewed changes

volodymyr-chekyrta marked this pull request as ready for review May 31, 2024 10:18

volodymyr-chekyrta added the product review PR requires product review before merging label May 31, 2024

fix: Fixes according to PR feedback

8eb71c7

PavloNetrebchuk requested a review from volodymyr-chekyrta May 31, 2024 10:46

volodymyr-chekyrta changed the title ~~feat: [FC-0047] Progress bar and collapsing course sections~~ feat: [FC-0047] Course progress and collapsing sections May 31, 2024

volodymyr-chekyrta previously approved these changes May 31, 2024

View reviewed changes

fix: Fixes according to designer feedback

6df86fc

PavloNetrebchuk dismissed volodymyr-chekyrta’s stale review via 6df86fc June 4, 2024 09:16

volodymyr-chekyrta requested review from k1rill and omerhabib26 June 6, 2024 09:31

k1rill previously approved these changes Jun 6, 2024

View reviewed changes

fix: Fixes according to PR feedback

1d3203b

PavloNetrebchuk dismissed k1rill’s stale review via 1d3203b June 6, 2024 11:49

volodymyr-chekyrta approved these changes Jun 6, 2024

View reviewed changes

volodymyr-chekyrta merged commit 1bc1d4c into openedx:develop Jun 6, 2024

volodymyr-chekyrta deleted the feat/progress_bar_and_sections_ui branch June 6, 2024 12:01

GlugovGrGlib mentioned this pull request Jun 13, 2024

feat: [FC-0047] Extend mobile API with course progress and primary courses on dashboard view openedx/openedx-platform#34848

Merged

		<string name="core_date_format_assignment_due_in" tools:ignore="MissingTranslation">Due in %1$s days</string>
		<string name="core_date_format_assignment_due_days_ago" tools:ignore="MissingTranslation">Due %1$s days ago</string>

Uh oh!

Conversation

PavloNetrebchuk commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mphilbrick211 commented May 23, 2024

Uh oh!

PavloNetrebchuk commented May 24, 2024

Uh oh!

volodymyr-chekyrta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

volodymyr-chekyrta May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

volodymyr-chekyrta May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

volodymyr-chekyrta commented May 31, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

volodymyr-chekyrta commented May 31, 2024

Uh oh!

volodymyr-chekyrta left a comment

Choose a reason for hiding this comment

Uh oh!

sdaitzman commented Jun 3, 2024

Uh oh!

PavloNetrebchuk commented Jun 4, 2024

Uh oh!

sdaitzman commented Jun 5, 2024

Uh oh!

k1rill left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Jun 6, 2024

Uh oh!

HamzaIsrar12 commented Jun 7, 2024

Uh oh!

PavloNetrebchuk commented Jun 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

PavloNetrebchuk commented May 21, 2024 •

edited

Loading

openedx-webhooks commented May 21, 2024 •

edited

Loading

volodymyr-chekyrta May 31, 2024 •

edited

Loading

volodymyr-chekyrta May 31, 2024 •

edited

Loading