-
Notifications
You must be signed in to change notification settings - Fork 3
enrollments dashboard expand collapse #2195
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
Conversation
OpenAPI ChangesShow/hide No detectable change. |
|
@mbilalmughal I noticed that the Figma prototype has a smooth transition when you expand / collapse. Is that desired here? |
@gumaerc Not particularly, but some sliding animation that indicates to the user that the section is expandable—similar to an accordion. |
OpenAPI ChangesShow/hide No detectable change. |
OpenAPI ChangesShow/hide No detectable change. |
ChristopherChudzicki
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.
This all looks good except for the sorting issue I mentioned.
Additionally, I noticed an existing layout issue that these cards really highlight at some widths. https://github.com/mitodl/hq/issues/7097
| const { ended, started, notStarted } = sortEnrollments(enrolledCourses || []) | ||
| const sorted = [...started, ...notStarted, ...ended] | ||
| const shownEnrollments = [...started, ...notStarted] |
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.
My understanding of the sorting and collapse logic is that it should be:
- Started
- Not started
- Course over, user "Completed = Certificate Earned"
- other cases where course is over
1–3 should be always visible, and 4 should be hidden by default.
@sovsey Can you confirm?
Assuming ☝️ is correct, we'll need to modify sortEnrollments... right now it doesn't distinguish between cases (3) and (4)
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.
@gumaerc Separately, I think the term "Completed" is pretty confusing. I would be 100% in favor of renaming EnrollmentStatus.Completed to EnrollmentStatus.CertificateEarned.
OpenAPI ChangesShow/hide No detectable change. |
ChristopherChudzicki
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.
👍
OpenAPI ChangesShow/hide No detectable change. |
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/7042
Description (What does it do?)
This PR adds expand / collapse functionality to the My Learning dashboard. By default, only started or not started enrollments are shown. Ended enrollments are revealed by clicking the "Show all" link button.
Screenshots (if appropriate):
How can this be tested?
mit-learnenrollment-dashboardfeature flag