-
Notifications
You must be signed in to change notification settings - Fork 479
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
Principal app feedback #25078
Principal app feedback #25078
Conversation
A teacher at your school, {this.props.teacherApplication.name}, has applied to be a part | ||
of <a href="https://code.org/educate/professional-learning-2019" target="_blank">Code.org’s Professional Learning Program</a> | ||
{' '} in order to teach the <a href={`https://code.org/educate/${courseSuffix}`} target="_blank">{this.props.teacherApplication.course} curriculum</a> | ||
{' '} during the {YEAR} school year. Your approval is required for the |
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.
This seems odd. Do we need the extra space? The {' '}
and the space after it?
Also nit: move the of
on line 242 to the previous line so the link stands on its own. Also please move the 2nd link on 243 to its own line. I find it more readable when the html tags are on their own line. It's easier to spot them.
@@ -87,7 +67,10 @@ class PrincipalApprovalApplicationsControllerTest < ::ActionController::TestCase | |||
assert_response :success | |||
end | |||
|
|||
assert_equal 'Yes: CodeHS, CS50', teacher_application.reload.sanitize_form_data_hash[:wont_replace_existing_course] | |||
assert_equal( | |||
'Yes, it will replace an existing computer science course.: CodeHS, CS50', |
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.
Kind of weird to have a period and then a colon here
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.
Good point. Here: #25123
Bug bash feedback on principal approval