Skip to content

Conversation

@HasinduWelarathne
Copy link

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

thoth-tech/documentation#597

Type of change

Migration

How Has This Been Tested?

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

Screenshots

Before:
image

After:
image

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

Copy link

@Pasindufdo98 Pasindufdo98 left a comment

Choose a reason for hiding this comment

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

Hi Hasindu,
I pulled your branch and tested locally, I was able to successfully build and run the application. When I was interacting with your component, I found that students are not correctly mapping when selecting a student in the list.
As an example,
when I selected student 1, it navigates to student 8 so on.
And rest of the functionalities looks good to me.

Screenshot (1588)

@HasinduWelarathne
Copy link
Author

Hi Hasindu, I pulled your branch and tested locally, I was able to successfully build and run the application. When I was interacting with your component, I found that students are not correctly mapping when selecting a student in the list. As an example, when I selected student 1, it navigates to student 8 so on. And rest of the functionalities looks good to me.

Screenshot (1588)

Hi @Pasindufdo98 ,

Thanks a lot for testing and pointing that out. You were absolutely right, the navigation was using student.id instead of the project’s id, which caused the mismatch. I’ve fixed it now so that the correct student opens when selecting a student.
Appreciate your thorough review!

@WaelAlahamdi
Copy link

Hi Hasindu,
I was able to build and run the branch successfully. The new student list view and navigation to individual student dashboards are working as intended.
One issue I observed:
• When selecting a student and then navigating back to the student list, the Stats progress bar in the list disappears. See Student_10 in the screenshot to clarify.
• The progress data only reloads after a full page refresh, which may affect usability for tutors monitoring students.
Overall, the functionality looks good, but this refresh issue should be addressed to ensure consistent display of student stats.
Feedback PR #389

… dashboard

- Added sessionStorage-based snapshot mechanism to restore student's taskStats.
- Ensures correct stats display when returning from student dashboard.
- Keeps other component behavior unchanged.
@HasinduWelarathne
Copy link
Author

Hi Hasindu, I was able to build and run the branch successfully. The new student list view and navigation to individual student dashboards are working as intended. One issue I observed: • When selecting a student and then navigating back to the student list, the Stats progress bar in the list disappears. See Student_10 in the screenshot to clarify. • The progress data only reloads after a full page refresh, which may affect usability for tutors monitoring students. Overall, the functionality looks good, but this refresh issue should be addressed to ensure consistent display of student stats. Feedback PR #389

Hi @WaelAlahamdi

Thank you for testing the branch and sharing your feedback. I’ve fixed the issue with the stats progress bar. It would be great if you could review the latest changes again and confirm everything is working as expected.

Thanks!

Copy link

@Pasindufdo98 Pasindufdo98 left a comment

Choose a reason for hiding this comment

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

LGTM

Hi @HasinduWelarathne ,

I retested your PR and confirmed that it now navigates correctly to each student when their name is clicked. Great work!
image

@WaelAlahamdi
Copy link

Hi @HasinduWelarathne,

I retested the branch and confirmed that the stats progress bar now works correctly and navigation to each student functions as expected. Everything looks great, well done!
Feedback-2 PR #389

@returnMarcco
Copy link

returnMarcco commented Sep 14, 2025

Hi @HasinduWelarathne,

I have tested the migration in the Chrome browser. The component functions as expected. There are no immediately obvious UX/UI, console, or API response errors.

  • The progress bar matches the line graph that can be viewed by clicking into a student record.
  • I am able to change a student's campus.
  • I am able to add/remove tutorials.
  • I am able to modify the student's target grade.
  • I am able to enrol students.
  • I am able to export the student list as a csv.

Well done on the migration, great work.

Copy link

@returnMarcco returnMarcco left a comment

Choose a reason for hiding this comment

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

Pre-approving this PR.

// Lottie animation module
// import {LottieModule, LottieCacheModule} from 'ngx-lottie';
import { FStudentsListComponent } from './units/states/students-list/students-list.component';

Choose a reason for hiding this comment

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

Can you please run the linter (Prettier) on all changed files to remove whitespaces and other inconsistencies. Cheers.

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