Create census banner to show to teachers with nces school ids

[drewsamnick]

This creates a new simplified version of the census form that will be shown to teachers associated with an NCES school id.

Initial display:

With "yes" selected:

With "not yet":

When you opt to update school info:

[drewsamnick]

Teacher pop-up

[aoby]

note: you can avoid these explicit bindings by defining the callbacks with arrow functions. See above

[aoby]

note: you can define these callbacks as arrow functions to automatically bind this and remove the need to do it explicitly later. E.g.

onCountryChange = (_, event) => {
  const newCountry = event ? event.value : '';
  this.setState({country: newCountry});
}

Also, nit: I prefer the callback supplied to an onVerb handler to be called handleVerb (rather than again onVerb), like in the react examples.

[drewsamnick]

I'll change this.

[aoby]

Since this is all in React now, don't we have (or can construct) all the state in one place in the parent object and submit that? Why construct all these hidden fields (starting on line 276) and serializing the actual html form data? Is there something else we're gaining by doing that? This would be simpler, and more React-y, to maintain and submit state from a component, not render anything hidden (unless it's needed elsewhere), and not rely on finding an element by id. If we were to look for an element by id, using ref is generally preferred.

[drewsamnick]

I'm going to change this to construct the POST from the component's state. I did it this way simply because all of the other forms I've touched have worked like this and I didn't consider doing it differently. I agree that we don't need an actual form here.

[aoby]

Are these needed? See above

[aoby]

I suppose if this is simpler as a form with uncontrolled elements so we don't need a bunch of state management then it makes sense. In that case, please move the whole form into a different component (or at the very least a function), and use ref to find that DOM element.

This render method is getting a bit long and unwieldy, hard to keep track of the branches.

[drewsamnick]

I agree about long and unwieldy. I'll break it up to be more readable.

[aoby]

Is this needed? Will the dismissal also unmount the component? If we unmount the component on @DiSmiSSaL, and set default state (line 84) when it's new, then do we need to clear it on the way out?

[drewsamnick]

Yes, this is needed. This is handling the case where the user dismissed the "update your school" view, not the entire banner. In that case, there may have been partial state about the school changed. We do not want to use that partial state in the census submission so we need to reset to the previous values. I'll add a comment to make this clearer.

[drewsamnick]

Updated based on PR comments from Friday.

[aoby]

This appears to be the same as the initial state on line 79, plus the last 2 showSchoolInfo* fields. This raises a couple questions:

1. Should those 2 also be initialized on line 79?
2. If the 2 state objects are identical, then why don't we wrap these into a single static property, e.g. initialState and set both the initial state and this setState to that value?

[drewsamnick]

Yes, those should be the same. I changed it to reference a common definition of that state.

[drewsamnick]

I've been asked to hold off merging this until Jan.