-
Notifications
You must be signed in to change notification settings - Fork 137
Feature: Grant Extension Form #285
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
Conversation
samindiii
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.
Great job on your work! I have reviewed it and have suggested some changes- if you want to discuss them with me, feel free to shoot me a message on Teams! :)
src/app/admin/modals/grant-extension-form/grant-extension-form.component.html
Outdated
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.html
Outdated
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.html
Outdated
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.html
Outdated
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.spec.ts
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.ts
Show resolved
Hide resolved
Just pushed some updates to the grant-extension-form:Added a list of students to the component to support testing of the dropdown and placeholder behaviour. The default “Select a student” option has been updated accordingly. Validation error messages are now shown for the student and reason fields when they're touched but not filled out, or when the "Grant Extension" button is clicked. The Additional Notes field now indicates that it’s optional. The "Grant Extension" button now disables on submit and resets all form fields after a successful submission. Added inline comments to clarify the form logic and improve maintainability. These updates reflect the requirements outlined in my assigned ticket, along with suggestions provided by Samindi. |
SahiruWithanage
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.
Great work! I went through everything and didn’t run into any issues – it all worked smoothly. I also checked the console log, and everything seemed to be working fine there as well.
I do have a couple of suggestions though. One is to consider adding a word limit to the "Reason" and "Additional Notes" fields so the input doesn’t exceed a certain limit.
Another idea is to include a “Cancel” or “Clear Form” button that resets everything back to the default state – that would be a nice touch for usability. Also, I think the suggestions made by Samindi were really great and worth considering as well. Overall, awesome job!
|
The current scaling of the form and text seems to be really small You should also be using Material UI components for elements like the select dropdown, slider, buttons, inputs, etc. This should reduce the amount of tailwind classes you're currently using I'm not familiar with the guidelines of this project, but perhaps this could be a modal/dialogue pop up, similar to the extension request form component that students are able to submit. Are you working off any existing documentation that you could link here for reference? |
Thanks for the feedback! |
This PR implements the Grant Extension form as an Angular Material Dialog.Please note: The form is accessible as a student for testing purposes. As there are issues preventing admins from accessing units on the 9.x branch. Once those issues are resolved, this will be updated so that only staff users can access the feature. Changes Made:Grant Extension form implemented as a Material Dialog Utilizes Material UI components (form fields, select dropdown, slider, textarea, buttons) Form validation for required fields Slider for extension duration (1-30 days) Temporary visibility for students to allow testing due to current admin access issues |
|
After testing the grant extension component (in the student area, which I understand is here just for testing), I was able to get a console output that my extension has been submitted to the student I had selected and see the number of days and reasoning. Everything mentioned above that the form should include was there, with the required fields working and coming up with a message underneath when not filled out. Along with this, the slider works from 1-30 days. After seeing the comments written above, I can see that a cancel button has been added and works. I did see there was an empty file, grant-extension-form.component.scss. If it's not needed in this component, I would recommend removing it so it doesn't cause any clutter. Overall, everything worked for me during testing. Good job :). |
SahiruWithanage
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.
Nice work on the changes – everything looks good to me!
b0ink
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 @JoeMacl
I wasn't able to navigate to the grant extension route as it kept redirecting me back to the home page, but i have left a few notes regarding your changes - looks all good to me, just a few areas that you could change so that when the API routes are added this component can be merged relatively smoothly without additional changes
src/app/admin/modals/grant-extension-form/grant-extension-form.component.html
Show resolved
Hide resolved
src/app/admin/modals/grant-extension-form/grant-extension-form.component.ts
Outdated
Show resolved
Hide resolved
...ashboard/directives/task-dashboard/directives/task-status-card/task-status-card.component.ts
Outdated
Show resolved
Hide resolved
a3b44c7 to
570433a
Compare
|
I'm closing this PR as PR #330 includes a more updated and integrated implementation of the Staff Grant Extension frontend work. |


Description
Features the new 'grant extension component' that allows staff to grant extensions to students via the 'grant extension' button located in the units page. Currently this component is frontend-only. No API integration has been implemented yet. Backend endpoints will be added in a future update.
Important: Currently, there's an issue preventing admins from accessing the units page. For testing purposes, the grant extension form is temporarily accessible to students. This will be restricted to admin users only in future updates.
The form includes the following fields:
Student: A dropdown selector.
Extension Duration: A slider ranging from 1 to 30 days.
Reason: A required text area input.
Additional Notes: Optional text area.
To View the Feature
Type of change
How Has This Been Tested?
Testing Checklist:
Checklist: