Skip to content
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

Merged
merged 39 commits into from
Feb 6, 2025
Merged

Conversation

Process-ing
Copy link
Contributor

@Process-ing Process-ing commented Jan 19, 2025

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

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

Sorry, something went wrong.

@Process-ing Process-ing changed the base branch from develop to ui/redesign January 19, 2025 01:33
@Process-ing Process-ing changed the title Create acadepic path courses page Create academic path courses page Jan 19, 2025
Copy link

codecov bot commented Jan 19, 2025

Codecov Report

Attention: Patch coverage is 2.51572% with 155 lines in your changes missing coverage. Please review.

Project coverage is 11%. Comparing base (380b135) to head (7a95fb1).
Report is 41 commits behind head on ui/redesign.

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     

@Process-ing Process-ing marked this pull request as ready for review January 19, 2025 12:35
@Process-ing Process-ing requested a review from a team January 19, 2025 12:50
@Process-ing Process-ing force-pushed the redesign/courses-page branch 2 times, most recently from 6e5d5c4 to b749fb7 Compare January 21, 2025 10:28
@DGoiana
Copy link
Collaborator

DGoiana commented Feb 3, 2025

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

@Adriano-7
Copy link
Member

Adriano-7 commented Feb 4, 2025

Great job! Just a couple of things I noticed:

  • I was testing it now, and for LEIC, I get ??? as the name. Also LEIC says "now", but i finished the bsc last year
  • Can you adjust the horizontal padding so that all components align? The selectors and the average bar should align with the course widgets.
  • The colors of the course selector are different from those in Figma. Can you correct that? Also, there seem to be three different shades of pink—can you change it to use the color palette one?
  • In Figma, we have three kinds of icons (one for each type of course: bachelor, master's, PhD). Do we have the necessary info from Sigarra for that?
  • Can you make it so that clicking on the course widget card takes us to the course page?

Copy link
Contributor

@Pinho13 Pinho13 left a 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! 🌱

@Process-ing Process-ing force-pushed the redesign/courses-page branch from 1319d73 to 7d28374 Compare February 6, 2025 16:12
Copy link
Contributor

@AugustoVSoares AugustoVSoares left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved! 🔥

@Process-ing Process-ing force-pushed the redesign/courses-page branch from 7d28374 to e456e38 Compare February 6, 2025 16:28
@Process-ing Process-ing force-pushed the redesign/courses-page branch from 5727360 to 0c1e2f5 Compare February 6, 2025 16:50
@Process-ing Process-ing merged commit b9b3384 into ui/redesign Feb 6, 2025
6 checks passed
@Process-ing Process-ing deleted the redesign/courses-page branch February 6, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Academic Path Courses page
5 participants