Skip to content

Conversation

@HasinduWelarathne
Copy link

Title

Migrate Students-List Component

Description

This PR migrates the existing students-list component from AngularJS/CoffeeScript to Angular 17/TypeScript, adopting Angular Material and Tailwind CSS. All original functionality (search/typeahead, filters, sorting, progress bars, flag icons, campus/tutorial selects, CSV export, enrol modal, pagination, and row navigation) remains behaviorally identical.

Component Review

Type of change

  • Migration

How Has This Been Tested?

  1. Verified typing in the search bar filters the list and shows autocomplete suggestions.
  2. Toggled “Advanced Filters” and applied “All Tutorials” vs “My Tutorials” filters.
  3. Clicked each sort button (Grade, Plagiarism, Portfolio) twice to confirm original and reverse ordering.
  4. Entered a term yielding no matches and saw the “No students found” alert.
  5. Scanned table rows to confirm avatars, usernames, names, progress bars, and flag icons display correctly.
  6. Opened campus and tutorial dropdowns to change selections and confirmed the list updates.
  7. Clicked on a student row or avatar and confirmed navigation to the student details view.
  8. Clicked “Export CSV” and verified the downloaded file’s headers and rows match the table.
  9. Clicked “Enrol Student” and ensured the enrolment modal opens.
  10. Used pagination controls to switch pages and change page size, ensuring the list updates.

Screenshots

Before:
before1

After:
after screenshot final

Testing Checklist

  • Tested in latest Chrome
  • Tested in latest Safari
  • Tested in latest Firefox

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@HasinduWelarathne HasinduWelarathne force-pushed the students-list-migration branch from 960cde3 to 5678ee9 Compare May 19, 2025 14:33
@HasinduWelarathne HasinduWelarathne force-pushed the students-list-migration branch from 5678ee9 to 96010fe Compare May 19, 2025 15:01
@chelaz1234
Copy link

I tried running the branch locally but encountered a couple of build-time Angular errors:

Can you please check and add modules where necessary?

Let me know once updated so I can re-review. Thanks!

<thead class="bg-gray-50">
<tr>
<th class="sticky left-0 px-3 py-2"></th>
<th class="px-3 py-2 cursor-pointer" font-bold (click)="sortTableBy('student.username')">
Copy link

Choose a reason for hiding this comment

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

Please remove font-bold from the element attributes as font-bold here is not a valid HTML and has no effect.

Copy link

@amriith amriith left a comment

Choose a reason for hiding this comment

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

Hi @HasinduWelarathne please have a look at the commented changes, let me know if you need any help with that. Rest everything looks good to me. Good Work

Copy link

@chelaz1234 chelaz1234 left a comment

Choose a reason for hiding this comment

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

This migration is done according to the migration guide and the old component has been removed, and the new component has been successfully linked and downgraded. Great work!

@HasinduWelarathne
Copy link
Author

Hi @amriith Thank you for the suggestion. I removed the unused font-bold attribute from the header elements and tightened up the SCSS to use consistent font-weight styling for the student-list table

@aNebula
Copy link

aNebula commented Jun 17, 2025

There's some difference between the before and after:

  1. Tabs selector looks different.
  2. Previous allows sorting by Username, new allows sorting by flag.

These needs to be resolved.

@HasinduWelarathne
Copy link
Author

Updated: unified tab selector styling and disabled flag sorting to match original behavior.

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.

4 participants