Add visible stable course edit ui #34433
Conversation
|
@dmcavoy would you be willing to take the first look at this PR? |
| /> | ||
| <p> | ||
| If checked, this unit will be eligible to be the recommended version |
There was a problem hiding this comment.
I think this should say course instead of unit? "If checked, this course will be eligible to be the recommended version of the course. The most recent eligible version will be the recommended version."
There was a problem hiding this comment.
We should probably add this string to i18n list
There was a problem hiding this comment.
On course / unit - yea, copy paste error. I'll fix.
On adding string to i18n list - what does that mean exactly / how do I do that?
There was a problem hiding this comment.
Nevermind Dave said we didn't need to worry about it here.
| </label> | ||
| <PilotExperiment | ||
| value={this.state.pilotExperiment} | ||
| onChange={e => this.setState({pilotExperiment: e.target.value})} |
There was a problem hiding this comment.
What happens if visible was set and then you start typing in a pilot experiment?
There was a problem hiding this comment.
The same thing happens - the checkbox gets greyed out, and then it's set as not visible. The behavior should be the same as what's on the script edit UI.
|
I did a small refactor to reduce code and test duplication, which also switched the order of the fields slightly for both the script and course edit UI. (See pictures in updated description.) |
When something is entered for pilot experiment, it automatically changes to:
Which should be the same behavior as on the script edit UI.
I did a small refactor to create a new component which combines VisibleInTeacherDashboard and PilotExperiment, which now handles greying out visible if pilot experiment is set. Then it can be used more cleanly between both the script edit and course edit UI. As a result the ordering in the UI is changed slightly - pilot experiment is now between visible and stable.
Testing story
Reviewer Checklist: