Edit modal - add completion/duration dropdowns and mastery fields#3455
Edit modal - add completion/duration dropdowns and mastery fields#3455rtibbles merged 36 commits intolearningequality:unstablefrom
Conversation
… content viewed/complete duration
79b97f6 to
7835f4a
Compare
rtibbles
left a comment
There was a problem hiding this comment.
A little bit of cleanup from a quick read through.
One thing that would be good to keep in mind as you finalize the test suite would be to ensure that the tests focus on the input/output of the components, and don't test the inner state of the components themselves - this will allow us more flexibility if we want to refactor to simplify some of the nested conditionals.
contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/actions.js
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelEdit/components/edit/DetailsTabView.vue
Show resolved
Hide resolved
| return ''; | ||
| }, | ||
| set(duration) { | ||
| const update = {}; |
There was a problem hiding this comment.
Still not a huge fan of the complexity here in terms of the intensely nested if statements, and the use of isSwitchingFromAllContentToCompleteDuration and currentDurationDropdown state.
But let's get the tests in place so that we can refactor for simplification later down the line.
Ah, I had missed that in the spec - let's leave it as is - but yeah the styling update from the spec is definitely better! |
|
All issues addressed - there is one more issue where the value for the defaultUploadDuration is presented as a raw number of seconds, which isn't quite to spec, but I'll file a follow up issue. |
|
Follow up issue here: #3465 |
rtibbles
left a comment
There was a problem hiding this comment.
Manual testing checks out. Further iteration will be reviewed in the follow up testing PR.




Summary
Description of the change(s) you made
Added new
CompletionOptionscomponent, which does the following:MasteryDropdowncomponent and movesmastery_criteriainformation into theCompletionOptionscomponentManual verification steps performed
Screenshots (if applicable)
Does this introduce any tech-debt items?
fileUploadthat currently is just a dummy, static number (for uploading audio and video resources and the "Exact time" dropdown option), but that will need to be updated once we decide on how to determine how to access that number.Reviewer guidance
How can a reviewer test these changes?
Please manually try to break this, as well as suggest code changes; particularly around the states.
Are there any risky areas that deserve extra testing?
Everywhere, but most likely any time we are looking at documents, and having to switch between "All content viewed" and "Complete duration" in the completion dropdown.
References
ActivityDurationcomponent)Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)