added small group generator #851
Conversation
|
some squiggly time issues: first, the bug in #758 is still apparent here. UPDATED: that second issue occurs only when the default form value displayed for instructional hours and due date are not updated; if due date is re-set it shows the correct due date (but time shows 12am). We should address this with a separate ticket -- if there isn't one already -- regarding how to display independent learning properly (due date instead of time block, labeled as independent, etc). See #810 |
| classNames: ['offering-editor-learnergroups'], | ||
|
|
||
| revisedLearnerGroups: computed('learnerGroups', { | ||
| get() { |
There was a problem hiding this comment.
How does having a get() method and a readonly() extension work here?
There was a problem hiding this comment.
It's the latest getter for CPs. You can also have a set but we are not explicitly setting the CP so not necessary. readonly() will block all attempts to set it.
|
I think we're missing a piece of functionality from the learner group chooser. The selected learner groups (on the left) is a flat list with the parent group shown via the name, but not indented. This is OK except it means we're missing the ability to remove a whole set of children by clicking their parent. |
| isMultiDay: false, | ||
| filter: '', | ||
|
|
||
| startDate: new Date().setHours(8, 0, 0), |
There was a problem hiding this comment.
These properties should probably be called defaultStartDate / defaultEndDate unless they are getting set somewhere?
|
@jrjohnson The chooser is flattened knowingly: there are some usability challenges which are obviated by doing this, and it simplifies both code and patterns. I think the additional clickety click to remove selected groups rather than bulk deselecting is for now worth the upside of the change. |
| instructorGroups: [], | ||
| learnerGroups: [], | ||
| startDate: moment().format('MM/DD/YYYY'), | ||
| endDate: moment().add(1, 'days').format('MM/DD/YYYY') |
There was a problem hiding this comment.
endDate should default to one hour after start date.
There was a problem hiding this comment.
Default end-date should be whichever the date that comes after the default start-date (or even maybe a picked-date).
|
this is looking great. we are still a little challenged in the time-picker area as noted. a couple of things:
If we can't get around this problem with the current control, can we add a default blank value for time, and use form validation to force a selection? Also, assuming we will do this on the form validation side as well, but we need to make sure we don't allow end times which precede start times. |
| @@ -1,7 +1,12 @@ | |||
| import moment from 'moment'; | |||
|
|
|||
| /* global moment */ | |||
There was a problem hiding this comment.
global moment can go away as the import statement is on line 4.
|
👍 |
|
@saschaben please review and merge if ready. @djvoa12 will work separately on the remaining time / date issues, but I want to get the majority of this merged so we can be certain it will be in the release next week and so the time / date can be a more focused discrete PR. |
|
@jrjohnson OK. Will eview fo primary functionality and UX less the time bug. |
Added an editor that can create a single offering or multiple offerings at once (small groups).
time-pickeroffering-managerfixes #697
fixes #758 (may need to replace time-picker with other addons due to rigid data-binding)