CB -> LB : Create ActivityEditor for Lesson Edit Page #36627
Conversation
dmcavoy
changed the title
apps/src/lib/levelbuilder/lesson-editor/ActivitySectionCard.jsx
Outdated
Show resolved
Hide resolved
|
Just commenting to say I don't think I'll have much to add here. |
apps/src/lib/levelbuilder/lesson-editor/ActivitySectionCardButtons.jsx
Outdated
Show resolved
Hide resolved
| @@ -135,34 +136,34 @@ export class UnconnectedLevelTokenDetails extends Component { | |||
|
|
|||
| handleAddVariant = () => { | |||
| this.props.addVariant( | |||
| this.props.lessonGroupPosition, | |||
| this.props.lessonPosition, | |||
| this.props.activityPosition, | |||
There was a problem hiding this comment.
thoughts on whether we could take this opportunity to remove variants from our UI? I don't see them in any 2020 scripts. it could be fine to keep them for now, since they are already implemented... and we could discuss / decide before Jan if we want to launch with these or not.
There was a problem hiding this comment.
Variants are only really used to swap out a level mid year if something is wrong with the level right? Are we ok with engineering needed to be involved if this case comes up?
There was a problem hiding this comment.
Good point...I don't have a better solution for that scenario, so since this one is working, it seems better to keep it around. I do still think we can get rid of level variant experiments, but that's a separate issue.
| expect(wrapper.find('ToggleGroup').length).to.equal(1); | ||
| expect(wrapper.find('AddLevelFilters').length).to.equal(1); | ||
| expect(wrapper.find('AddLevelTable').length).to.equal(1); | ||
| //expect(wrapper.find('LevelToken').length).to.equal(2); |
There was a problem hiding this comment.
possibly not found due to shallow rendering. it's possible you could try mount instead.
apps/src/lib/levelbuilder/lesson-editor/ActivitySectionCardButtons.jsx
Outdated
Show resolved
Hide resolved
|
|
||
| /* | ||
| An activity section is a chunk of an activity. This could be a section | ||
| or text that explains to the teacher what to say or do to run the lesson or |
There was a problem hiding this comment.
"or text" -> "of text" ? please ignore me if I am mistaken
| {this.props.activitySection.tips.map(tip => { | ||
| return ( | ||
| <LessonTipIconWithTooltip | ||
| tip={tip} | ||
| key={tip.key} | ||
| onClick={this.handleEditTip} | ||
| /> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
nit: this can be written more compactly as:
{this.props.activitySection.tips.map(tip => (
<LessonTipIconWithTooltip
...
/>
))}| activityPosition, | ||
| targetActivitySectionPos | ||
| } = this.props; | ||
| if (targetActivitySectionPos === activitySection.position) { |
There was a problem hiding this comment.
Sorry to keep you in suspense here Dani. What I discovered is that when I drag to reorder a script level, the left hand side of the check on this line is null. Digging in a bit, it looks like handleDrag is trying to store information in the state of the parent react component:
code-dot-org/apps/src/lib/levelbuilder/lesson-editor/ActivitySectionCard.jsx
Lines 168 to 169 in 409f7f4
render method is called, which might not be right away.
Thoughts on how to proceed:
- just moving this into state within the ActivitySectionCard might not be enough, because setState calls do not happen immediately. there may be a way you can force them to be immediate but that might not be what you want since it could cause a re-render which could mess up dragging.
- ideally you would get your y coordinate from the handleDrag callback directly
- if that's not possible, you could try storing the clientY info on the react component outside of props/state. not ideal, but likely to be effective.
sorry if you knew most of this but wanted to get my thoughts down since I had spent some time looking at it.
Creates the ActivitiesEditor which is a GUI for editing activity details of a lesson. It is based on the Script Edit GUI and as part of this work moves the editing of Levels from the Script Edit GUI to this GUI. The GUI shows a side by side of the editing area and a preview of what the activities will look like in the Lesson Plan. Right now the GUI is only viewable in storybook but in the future it will live on the lesson edit page.
Data Model This Will Require
This ActivityEditor uses sample data for right now in the storybook component but that sample data suggests some new models and relationships.
Activity has many Activity Sections
Activity Sections has many Levels (it can have no levels)
Demo
Links
Testing story
I added a new unit test file for each new component and added a simple unit test. More tests are needed in these files but I was trying to not have this get any longer right now. Follow up task PLAT-289
Reviewer Checklist: