-
Notifications
You must be signed in to change notification settings - Fork 137
Migrate Students-List Component #389
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
base: 9.x
Are you sure you want to change the base?
Migrate Students-List Component #389
Conversation
Pasindufdo98
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.
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.
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. |
… 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.
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.
LGTM
Hi @HasinduWelarathne ,
I retested your PR and confirmed that it now navigates correctly to each student when their name is clicked. Great work!

|
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.
Well done on the migration, great work. |
returnMarcco
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.
Pre-approving this PR.
| // Lottie animation module | ||
| // import {LottieModule, LottieCacheModule} from 'ngx-lottie'; | ||
| import { FStudentsListComponent } from './units/states/students-list/students-list.component'; | ||
|
|
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.
Can you please run the linter (Prettier) on all changed files to remove whitespaces and other inconsistencies. Cheers.




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?
Screenshots
Before:

After:

Testing Checklist
Checklist