-
Notifications
You must be signed in to change notification settings - Fork 16
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 #434
feat: [FC-0047] Improved Dashboard Level Navigation #434
Conversation
Thanks for the pull request, @IvanStepanok! 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:
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. |
Hi @IvanStepanok, thanks for these changes. They look really good overall. I've noted all the minor visual issues I could find below. This is a huge usability/experience win regardless of these issues, so if any are difficult to resolve, most of them could be saved for a future cleanup. iOS:
Android:
Both iOS and Android:
|
Hi @sdaitzman, and thanks for paying attention to the details! I appreciate it🙌 ✅ 1. In light and dark mode, the Resume course quick action doesn’t need a border separating it from the other actions. ✅ 3. In the All Courses view in light mode, the filter options currently use a light gray border color, and keep that border color when selected. ✅ 5. When the Programs/Courses dropdown switcher is tapped, the dropdown icon should animate Simulator.Screen.Recording.-.iPhone.15.-.2024-05-13.at.23.55.13.mp4⛔️(3) Selected dashboard-level tabs in dark mode should switch over to using the Dark Theme Accent Text color |
Hey @IvanStepanok, thanks so much for these updates! This looks all good to me now, and we can hold (3) and (4) for future improvements |
referring to point 5 here. @sdaitzman shouldn't the Course/Program switcher be animated in the reverse order? |
@moiz994 great catch, thank you! Yes, the Course/Program switcher dropdown icon should be flipped. |
@sdaitzman @moiz994 Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-05-15.at.12.05.04.mp4 |
@IvanStepanok Maybe we want to scroll through the menu a bit when opening via the "View Assignments" button to see the currently selected item? |
@sdaitzman @moiz994 @IvanStepanok Just a question - should these options be available for offline (if we know it won't load offline anyway)? Screen.Recording.2024-05-15.at.17.54.41.mov |
@rnr thanks for checking about these options. For the first question: yes, I think it makes sense to scroll the course header navigation to the active tab when jumping there. For the second question, I think it depends on the plan for implementing offline course storage improvements. If those views are accessible offline in the future, the shortcuts to them should be as well. I would lean slightly towards keeping the quick actions for now and showing a message like the "Slow or no internet connection" message that appears on the bottom of course home in that video until the content is available offline in the future. I'm curious what others think about this question though— can bring it up next mobile design weekly call if that would be helpful. |
@saeedbashir Thank you for your review! Let’s move forward with this PR so we can continue developing the related features. It’s indeed odd that you don’t see a new primary course after xBlock completion; this might be a sandbox issue. However, this is not within the iOS area. All back-end issues will be addressed in the back-end PR, and we will cover any minor issues by the next PRs. For now PR looks good for merging to me. @GlugovGrGlib FYI sandbox issue. |
@volodymyr-chekyrta The PR isn't block because of this issue/requirements, I just needed documentation of this so that QA can update the test document. |
I'll provide it asap |
@IvanStepanok I'm not sure why but the program webview is not loading with course dashboard view. I've used the v3 of enrollments so the enrollments screen is not loading (any how this can happen anytime). I know programs are not available on your intances, what you can do is you can use any dummy URL for programs, e.g Another issue is, when I use pull to refresh the programs view, it calls the enrollments API instead of reloading the programs webview. The program's webview is working fine in list view. Screen.Recording.2024-05-24.at.10.38.29.PM.mov |
Hi @saeedbashir, unfortunately I can't access that youtrack link. I think the intended design is that the most recent course where a user has completed blocks is highlighted in the primary course card on their dashboard. |
@saeedbashir, please find the feature documentation below Dashboard Level Navigation: Primary course User Story Acceptance Criteria:
|
@volodymyr-chekyrta Is offline support also part of this PR. I'm asking this because it's not working. |
@saeedbashir, Yes, according to the documentation, the primary course screen should be cached. @IvanStepanok could you double-check what happened with caching? |
@saeedbashir, @volodymyr-chekyrta, I've made changes to handle the offline access issue by ensuring that the CoreData model is updated correctly. The main changes include: Clearing Old Data: Before saving new enrollments, all existing data in the CoreData model is deleted. This approach avoids potential migration issues and ensures data consistency. It's worth considering applying a similar approach to other parts of the project that interact with CoreData to ensure overall data consistency and integrity. We should review the rest of the project to identify and address any similar issues. |
@IvanStepanok Great! the offline mode is now working. Now the last item remaining in this PR is #434 (comment) after that we will be good to merge this PR. |
As an example, I try to add a base URL for Programs. They run the redirect flow, but if we comment out the block about redirecting, the programs view loads fine. Please check it on your side.🙌 |
@IvanStepanok I tried to capture the behaviour in the following screen recording for both case (new user with no enrollments and a user with enrollments). Please have a look, and if something isn't clear, we can connect on Slack. screen_recording.mov |
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 👍
@IvanStepanok 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Quick Preview
Screen.Recording.2024-05-10.at.16.18.16.mov
Drop down menu
They are enabled automatically when "Programs" is enabled in the configuration file.
Screen.Recording.2024-05-10.at.17.16.22.mov
Empty State
Horizontal Position
Screen.Recording.2024-05-10.at.16.47.06.mov
Feature enabled by default.
🚨 You should use key:
to enable a list style Dashboard (old style)
API
Since the new APIs are not available in the master branch, please use the sandbox: