-
Notifications
You must be signed in to change notification settings - Fork 45
Feat: Home tab[0092] #462
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: Home tab[0092] #462
Conversation
|
Thanks for the pull request, @PavloNetrebchuk! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
222b7f0 to
36e49ac
Compare
133f489 to
42eda19
Compare
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
course/src/test/java/org/openedx/course/presentation/home/CourseHomeViewModelTest.kt
Fixed
Show fixed
Hide fixed
IvanStepanok
left a comment
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
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.
In case we will use this file for tests only, probably we can move it to core/src/test or to debug/testFixtures?
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.
Not only tests. Also preview functions
|
|
||
| val downloadableBlocks = blocks.filter { it.isDownloadable } | ||
| val downloadingBlocks = blocksIds.filter { isBlockDownloading(it) } | ||
| val isAllBlocksDownloaded = downloadableBlocks.all { isBlockDownloaded(it.id) } |
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.
Will return true in case when we have downloadableBlocks empty. Is it okay?
| fun formatToDayMonth(date: Date): String { | ||
| val sdf = SimpleDateFormat("MMM dd", Locale.getDefault()) | ||
| return sdf.format(date) | ||
| } |
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.
It returns Month to Day but named as a DayMonth
| scaffoldState = scaffoldState, | ||
| backgroundColor = MaterialTheme.appColors.background | ||
| ) { | ||
| val screenWidth by remember(key1 = windowSize) { |
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.
screenWithModifier?
c28f338 to
40aa01f
Compare
0472ff6 to
6381aca
Compare
a6a85e2 to
f7992ae
Compare
|
@PavloNetrebchuk nice work! A few notes: For the Assignments carousel card, for upcoming assignment, can you add the header "Next Assignment": Also for the assignments card: just want to make sure the text is correct: larger text is subsection name, smaller is the section name For the first and third cards (course completion, assignments), can the background color be the same as the page dark color: For the Video card: if the video has not been started, the text should read "Next Video". Continue watching is for videos that are started but not yet finished. |
f7992ae to
57c3f93
Compare
|
@edschema
|
a80ed4d to
ff18188
Compare
|
@stan-neichev that’s for past due assignments. For upcoming assignments, we wouldn’t say “past due” |
|
sorry, let me check with the team |
ff18188 to
47a5ba0
Compare
|
@edschema seems like we had no design for other states and therefore we decided to do it this way: Also, our QA noticed that we had another background color for this element Please don't change the design on the fly because it's hard to trace it back to the initial state and have a more or less stable scope of work |
47a5ba0 to
9e98256
Compare
|
All changes have been made✅ |
|
Thank you @PavloNetrebchuk. Is this ready for code review? Thank you! |
RawanMatar89
left a comment
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👍🏻Thank you for the effort 👏🏻
| fun getCourseProgress( | ||
| courseId: String, | ||
| isRefresh: Boolean, | ||
| getOnlyCacheIfExist: Boolean |
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.
will you please consider documenting this new parameter's behavior clearly
9e98256 to
71cc7de
Compare
course/src/main/java/org/openedx/course/presentation/unit/container/CourseViewMode.kt
Fixed
Show fixed
Hide fixed
course/src/main/java/org/openedx/course/presentation/unit/container/CourseViewMode.kt
Fixed
Show fixed
Hide fixed
b9117cf to
762ffe2
Compare








Portrait
Screen.Recording.2025-09-04.at.18.12.34.mov
Landscape
Screen.Recording.2025-09-04.at.18.15.32.mov
Tablet
Screen.Recording.2025-09-04.at.18.17.28.mov
Dart Theme