-
Notifications
You must be signed in to change notification settings - Fork 14
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
#3697 - View Assessment Updates #3813
Conversation
offeringIntensity: OfferingIntensity.partTime, | ||
}, | ||
]; | ||
|
||
export const PartTimeAwardTypesObject = { |
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.
👍
@@ -1,7 +1,7 @@ | |||
<template> | |||
<body-header | |||
title="Summary" | |||
subTitle="Below is the summary from your assessment. To view your entire assessment, click on View assessment." |
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.
Should this period in the end to be removed?
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.
I think it's fine to keep the period. The AC is to replace the sentence, and it makes sense to keep the period at the end for a sentence.
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.
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.
I thought there was a period at the end, but there wasn't. It's updated.
@@ -55,22 +55,22 @@ export const AWARDS: AwardDetail[] = [ | |||
}, | |||
{ | |||
awardType: PartTimeAwardTypes.CSLP, | |||
description: "Canada Student Loan for Part-time Studies", | |||
description: "Canada Student Loan for Part-Time Students", |
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.
Part-time with lower case "t".
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.
I know the AC has upper-case but all the system uses lower case. Please check.
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 catch. It's this one place where "time" is capitalized. It's now updated.
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.
Looks good to me. Just minor comments about the wording.
Nice work @lewischen-aot. Please take a look at the comments. |
if (awardValue === undefined) { | ||
return "(Not eligible)"; | ||
} | ||
return getFormattedAwardValue(awardValue); |
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.
As we know that the dynamic award value with key as award name is definitely a number, I would say we can force cast it. like this.
const awardValue = props.awardDetails[
`${props.identifier}${awardType.toLowerCase()}`
] as number;
It will allow us to set the type of award value as number.
Thanks for making the changes. Just few minor comments. |
if (awardValue === undefined) { | ||
return "(Not eligible)"; | ||
} | ||
return getFormattedMoneyValue(awardValue); |
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.
return awardValue === undefined ? getFormattedMoneyValue(awardValue) : "(Not eligible)";
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.
It is the opposite @andrepestana-aot when undefined "(Not eligible)"
needs to return. Are you saying this as a suggestion?
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.
Thanks for doing the changes. Looks good 👍
Quality Gate passedIssues Measures |
Screenshot for the first disbursement for part-time applications
Screenshot for the first disbursement for full-time applications