-
Notifications
You must be signed in to change notification settings - Fork 18
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
Create academic path courses page #1446
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ui/redesign #1446 +/- ##
============================================
- Coverage 11% 11% -0%
============================================
Files 270 271 +1
Lines 7507 7622 +115
============================================
+ Hits 817 820 +3
- Misses 6690 6802 +112 |
6e5d5c4
to
b749fb7
Compare
Looking good! I'd just reduce the size of the course squircle on the top of the page and maybe remove the horizontal line below the filters |
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.
Very nice! It works! 🌱
1319d73
to
7d28374
Compare
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.
Approved! 🔥
7d28374
to
e456e38
Compare
5727360
to
0c1e2f5
Compare
Closes #1430
The base structure of the page is mostly done, but if you see something that needs improvement, let me know.
A few things to note:
In the original mockups, each course unit is associated with year of enrollment + year of conclusion. However, it is impossible (from what I know) to know the latter, so I just omitted it from the course selection.The "???" that appears on the course selection is a placeholder for the course abbreviation, since it is currently being fetched as null. It's best to discuss a fix/workaround in the next meeting.The page layout suggests that the course units are only the ones from the selected course. However, it is impossible to associate a curricular unit to a course (Sigarra does not do this association, and can be troublesome for Erasmus students, etc.), so it currently presents all the course units the user is enrolled in.NEW The average bar presents the finished ECTS and total ECTS of the course. The really stupid thing is that the Sigarra API provides the former, but not the latter, so we will also need a workaround for that 💀. Currently, it presents the total of ECTS a student is enrolled in, but that only works for people enrolled in only one course and in their last year.Review checklist
whatsnew/whatsnew-pt-PT
changelog.md
with the change