Skip to content

Conversation

@mannat2634
Copy link

Description

This PR migrates the task-ilo-alignment-modal from AngularJS/CoffeeScript to Angular/TypeScript. This modal is used by convenors in the Unit Administration section to align specific Tasks to Intended Learning Outcomes (ILOs) and provide a rationale/rating.

Key Changes:

  • Created TaskIloAlignmentModalComponent and TaskIloAlignmentModalService.
  • Replaced the legacy Bootstrap modal with Angular Material MatDialog.
  • Implemented a custom circular rating UI (as the standalone rater component is not yet migrated) that fills from the left, matching the legacy visual style.
  • Fixed an issue where alignment data was not saving correctly by ensuring the within context is passed to the entity service.
  • Downgraded the new service in doubtfire-angularjs.module.ts to ensure compatibility with the existing AngularJS task-ilo-alignment-editor.
  • Updated SCSS to remove Bootstrap variable dependencies ($brand-info, etc.) and enforced font sizes (24px headers, 15px body) to match the application standards.

Fixes # (issue)

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

*Before (AngularJS/Bootstrap):
Screenshot from 2025-12-01 19-26-12
*

After (Angular Material):

Screenshot from 2025-12-01 23-06-41

Mobile View:
image

How Has This Been Tested?

I have tested this manually by logging in as a Admin and accessing the Unit Administration page.

  1. Navigate to Unit Admin > Task Alignment tab.
  2. Click on an alignment circle (red or filled).
  3. Verify: The new Angular Material modal opens.
  4. Test Action: Click a rating circle (1-5).
    • Expected: The circles fill blue up to the selection, and a "Saved" toast appears.
  5. Test Action: Click "Click to add rationale".
    • Expected: The text area appears.
    • Action: Type text and click "Done Editing".
    • Expected: The text renders as Markdown, and an "Updated" toast appears.
  6. Test Action: Click the Delete (Trash) icon.
    • Expected: The alignment is removed, and the modal closes.

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
  • I have requested a review from @macite and @jakerenzella on the Pull Request

@ishika021
Copy link

Reviewed the changes - the component, service and module imports look correct, and I don’t see any merge conflicts or structural issues.

@ishika021
Copy link

I have reviewed this pull request in detail, focusing on the scope of the AngularJS - Angular migration, module wiring, and potential integration issues with the existing Doubtfire hybrid frontend.

This PR introduces a new Angular implementation of the Task ILO Alignment Modal, including the TaskIloAlignmentModalComponent, its associated service, template, and SCSS styling. The component is correctly registered in doubtfire-angular.module.ts, and the downgrade setup in doubtfire-angularjs.module.ts exposes the modal to the AngularJS layer using the factory pattern, which aligns with the established migration approach used elsewhere in the codebase.

I verified that the necessary imports and providers are correctly declared and that removed legacy references do not appear to be used elsewhere. No merge conflict warnings are reported by GitHub for this branch, indicating that the changes are currently compatible with the base branch.

From a code-level review, the dialog handling using MatDialog, data flow between the component and service, and the updated styling logic appear consistent with existing UI patterns. I did not identify any obvious issues related to missing dependencies, incorrect injection, or broken module boundaries.

GitHub Actions CI has been approved and has completed successfully. All automated checks passed, providing confidence that the code compiles, integrates correctly, and does not introduce build or runtime issues.

Overall, the changes are well-structured and follow the project’s migration conventions. From both a manual code review and automated testing perspective, this PR looks good to merge.

Copy link

@BrianDangDev BrianDangDev left a comment

Choose a reason for hiding this comment

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

Approve for upstream PR. Please open a PR to the upstream repo

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.

3 participants