Add co-teacher input field #54392
Add co-teacher input field #54392
Conversation
# Conflicts: # apps/src/templates/sectionsRefresh/SectionsSetUpContainer.jsx
# Conflicts: # dashboard/app/models/sections/section.rb
lfryemason
changed the title
| const handleAddEmail = e => { | ||
| e.preventDefault(); | ||
| const newEmail = inputValue; | ||
| console.log(sectionInstructors); |
There was a problem hiding this comment.
It looks like this console.log snuck in the final PR :)
| @@ -484,6 +484,13 @@ | |||
| "coteacherAdd": "Add Co-Teachers", | |||
| "coteacherAddTooltip": "Co-teachers have the same access as you in managing this section and viewing student work. Co-teachers will see the invitation on their teacher dashboard.", | |||
| "coteacherAddInfo": "Add co-teachers by entering the email address associated with their Code.org account in the field below. Each section can have up to five co-teachers.", | |||
| "coteacherAddNoEmail": "Please enter an email address.", | |||
| "coteacherAddInvalidEmail": "{email} is not a valid email address.", | |||
| "coteacherAddAlreadyExists": "{email} is already a co-teacher for this section.", | |||
There was a problem hiding this comment.
Just checking, I know this is more of a design question, but if I were a teacher who got this error, my next question question would be "now what?" - do we have a tool tip or something that directs a teacher to do something if they get this error? Something like "The co-teacher with this email address has already been notified and can find their invitations at X"?
There was a problem hiding this comment.
Below the input there will be a table including the names/emails and statuses of all the coteacher invitations. So the teacher would be able to look down at the table and see whether that teacher has already accepted, has declined or needs to accept.
apps/test/unit/templates/sectionsRefresh/CoteacherSettingsTest.jsx
Outdated
Show resolved
Hide resolved
apps/test/unit/templates/sectionsRefresh/CoteacherSettingsTest.jsx
Outdated
Show resolved
Hide resolved
apps/test/unit/templates/sectionsRefresh/CoteacherSettingsTest.jsx
Outdated
Show resolved
Hide resolved
|
|
||
| // coteacher count is teachers to add + the existing teachers - the primary teacher. | ||
| const numCoteachers = useMemo( | ||
| () => sectionInstructors.length + coteachersToAdd.length - 1, |
There was a problem hiding this comment.
- Is there any case where either
sectionInstructorsorcoteachersToAddmight be null or otherwise not-arrays, making this calculation problematic? - Will this be off by 1 when creating a new section and there isn't a
sectionInstructorsentry for the main instructor yet?
There was a problem hiding this comment.
coteachersToAddwill never be null, but I did need to add some checking forsectionInstructors- fixed
| () => <div>{i18n.coteacherAddInfo()}</div>, | ||
| () => ( | ||
| <CoteacherSettings | ||
| sectionInstructors={sections[0].sectionInstructors} |
There was a problem hiding this comment.
At first glance this looks suspiciously like it might fail when editing something other than one's first section, but then I see this pattern repeated elsewhere in this file. Is sections always a one-element array? If so, any reason we couldn't get rid of the array and switch all the sections[0] references to simply section?
There was a problem hiding this comment.
There was a problem hiding this comment.
This is because we eventually want to allow multiple sections to be created on the same form. Let's keep this around
| }; | ||
|
|
||
| // coteacher count is teachers to add + the existing teachers - the primary teacher. | ||
| const numCoteachers = useMemo( |
There was a problem hiding this comment.
Do you need to use useMemo here? It seems like this is getting passed down directly from the props. I should mention I have never used useMemo before so I am just applying it for the first time in this review. :)
There was a problem hiding this comment.
Hmm. I thought it was useful for every time you are making a computation that doesn't need to change every render, but it seems like it also isn't necessary for short and small computations. If you are interested, I thought this blog post was an interesting read
Co-authored-by: Eric Aderhold <eric.aderhold@code.org>
Adds an input field to add a coteacher to a section for edit and create new section.
Note: does not currently hook up to the validation endpoint. That will be a FLUP PR.
Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: