Add facilitator application 2018-19 form #18461
Conversation
| /** | ||
| * Override in derived classes | ||
| * @type {Object} - map of control name to label | ||
| */ |
There was a problem hiding this comment.
Since this is commented out, is it just a reminder for people creating derived classes? or should there be a default implementation in here?
There was a problem hiding this comment.
I don't follow. What's commented out? It is initialized to an empty object here in the base class (see line 14). This jsdoc block is describing the static labels, which yes should be overridden in derived classes (and is in the Section#... pages) .
| state: 'WA', | ||
| zipCode: '98101', | ||
| genderIdentity: 'Male', | ||
| race: ['White'], |
There was a problem hiding this comment.
I think this would technically be OTHER (half-giant)... :)
There was a problem hiding this comment.
Indeed. Will update :)
There was a problem hiding this comment.
I believe that would actually be species, which we do not ask currently.
There was a problem hiding this comment.
Since humans and giants can interbreed, they are the same species.
| city: "City", | ||
| state: "State", | ||
| zipCode: "Zip Code", | ||
| genderIdentity: "Gender Identity", |
There was a problem hiding this comment.
Gender Identity vs Gender?
There was a problem hiding this comment.
@tanyaparker preference here?
| genderIdentity: "Gender Identity", | ||
| race: "Race", | ||
| institutionType: "What type of institution do you work for?", | ||
| currentEmployer: "Current employer?", |
There was a problem hiding this comment.
Suggest removing the question mark
| }; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Is this override necessary? Aren't we just returning the superclass?
There was a problem hiding this comment.
Good point. I was following the pattern of existing forms, but you're right this doesn't appear to be necessary. Will remove.
| import Facilitator1819FormComponent from "./Facilitator1819FormComponent"; | ||
| import {pageLabels} from './Facilitator1819Labels'; | ||
|
|
||
| const YES = "Yes"; |
There was a problem hiding this comment.
Is there an NPM library you can import with this definition? /s
| <h4>If selected for the program, you will be required to attend TeacherCon and a Facilitator-in-Training Weekend in the summer of 2018.</h4> | ||
|
|
||
| {this.checkBoxesWithAdditionalTextFieldsFor("csdCspTeacherconAvailability", { | ||
| "I'm not available for either TeacherCon. (please explain)" : "unavailableReason" |
dashboard/app/models/ability.rb
Outdated
| @@ -133,6 +135,7 @@ def initialize(user) | |||
| can :manage, RegionalPartner | |||
| can :report_csv, :peer_review_submissions | |||
| can :manage, Pd::RegionalPartnerMapping | |||
| can :manage, Pd::Application::FacilitatorApplication1819 | |||
There was a problem hiding this comment.
Probably a future change but regional partners should be able to view this applications, right?
There was a problem hiding this comment.
Yeah I figure we should add that permission when we create the API that requires it.
| NO = 'No'.freeze | ||
| NONE = 'None'.freeze | ||
|
|
||
| PROGRAMS = { |
There was a problem hiding this comment.
A lot of these seem like constants that we'll be reusing in other forms.
There was a problem hiding this comment.
True. I could make a common application constants file...
| = stylesheet_link_tag 'css/pd', media: 'all' | ||
| %script{src: minifiable_asset_path('js/pd/application/facilitator_application/new.js'), data: @script_data} | ||
|
|
||
| %h1 |
There was a problem hiding this comment.
Any reason this is not part of the react element? Is it just because it's completely static?
I feel like for some components we've put everything in the react element. If we have a standard / preferred way of doing this, lets follow it.
There was a problem hiding this comment.
Right, it's static and also rendered across all pages in the form. The React Form renders its own pages and doesn't have space for a header so I'd have to create a wrapper component with this header. It just seems like unnecessary overhead for some static content.
There was a problem hiding this comment.
I don't think we have a standard. Anything with client-side logic should be in React, but otherwise we tend to leverage rails & haml for surrounding static content so I think this is consistent.
Rename Pd::Application classes for better consistency
|
I would prefer Gender Identity.
…On Wed, Oct 18, 2017 at 1:22 PM, Andrew Oberhardt ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In apps/src/code-studio/pd/application/facilitator1819/
Facilitator1819Labels.jsx
<#18461 (comment)>
:
> +
+// Labels by page name
+const pageLabels = {
+ Section1QuestionsAboutYou: {
+ title: "Title",
+ firstName: "First Name",
+ preferredFirstName: "Preferred First Name",
+ lastName: "Last Name",
+ accountEmail: "Account Email",
+ alternateEmail: "If you use another email, enter it here:",
+ phone: "Phone",
+ address: "Home Address",
+ city: "City",
+ state: "State",
+ zipCode: "Zip Code",
+ genderIdentity: "Gender Identity",
@tanyaparker <https://github.com/tanyaparker> preference here?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18461 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB1OEmVuZAlijimobfi37D-AbZZZ1gLvks5stl4AgaJpZM4P99Zb>
.
--
Tanya Parker
Technical Product Manager
Code.org
+1 (562) 318-4137
|
Task
A few selected screenshots