Skip to content

Conversation

@itsjeyd
Copy link
Member

@itsjeyd itsjeyd commented Sep 10, 2015

This PR introduces two new blocks and enables support for viewing and editing them in Studio. One of the blocks is a new version of MentoringBlock that supports explicit steps, the other one is a block that represents a step:

  • The new mentoring block only allows step blocks (any number) and mentoring-wide messages as direct children.
  • The step block hosts all blocks that can not be added to new mentoring block (MCQ, MRQ, Long Answer, etc.)

Acceptance criteria

Studio integration:

  • Make it possible to add new mentoring block to unit
  • Make it possible to add step blocks
  • Make it possible to edit step blocks
  • Make it possible to remove step blocks
  • Make it possible to add step contents
  • Previews at mentoring and step levels should function properly
  • inner blocks should not have a studio header (small glitch: at mentoring level, HTML block still has a header)
  • inner blocks should look more or less the same as they would in LMS
  • inner blocks should have no obvious styling glitches and pass other common-sense checks

Testing

  1. Enable new mentoring block for a course in Studio by adding pb-mentoring to advanced modules.
  2. Add new mentoring block to a (new or existing) unit by choosing "Mentoring Questions (with explicit steps)" from the list of "Advanced" blocks.
  3. Click "View" to edit the components of the block. Observe that the options for blocks to add are restricted to step and message blocks.
  4. Add a step block.
  5. Click "View" to edit the components of the block. Observe that the options for blocks to add include all relevant blocks that can not be added to the top level of the new mentoring block.
  6. Try adding and removing a few components to/from the step block.
  7. On the previous (mentoring) level, click "Edit" to change e.g. the title of the step block.
  8. Click the trash icon to remove the step block.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this a block specifically for handling steps, rather than a copy of the all-encompassing mentoring block? Ie maybe StepManager or StepBuilder?

I haven't looked closely, but it seem there is currently a lot of duplicated code with mentoring, currently? Imho we should only duplicate code we expect to be able to remove from Mentoring once we remove the assessments from there, to handle it exclusively with steps in StepBuilder. Can the rest of the duplicated code be removed or factorized? Otherwise maintaining twice the "almost the same code" is not going to be fun.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@antoviaque Thanks for the feedback! I DRYed out the changeset based on your suggestions (removed most of the duplicate code and factored the remaining code that was shared between the two mentoring blocks out into a shared superclass).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itsjeyd Thanks! Definitely better : )

@itsjeyd itsjeyd force-pushed the mentoring-step-xblock branch from d211797 to 66d9531 Compare September 10, 2015 18:55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@itsjeyd please use StudioContainerWithNestedXBlocksMixin - it would look something like that:

context['wrap_children'] = {
     'head': u'<div class="mentoring">',
     'tail': u'</div>'
}
fragment = super(MentoringWithExplicitStepsBlock, self).author_edit_view(context)
fragment.add_content(loader.render_template('templates/html/mentoring_url_name.html', {
        "url_name": self.url_name
 }))
 # css and js stuff

mentoring_edit.js is mostly not needed, except for ProblemBuilderUtil.transformClarifications(element); Not sure what does it do - @bradenmacdonald - some hint on that? Anyway, if you still need it, it would be better to have a new mentoring_with_steps_edit.js with the following contents:

function MentoringWithStepsEdit(runtime, element) {
    ProblemBuilderUtil.transformClarifications(element);
    StudioEditableXBlockMixin(runtime, element);
}

UPD: Actually, since MentoringWithSteps is only concerned about steps and steps does transformClarifications it might not be needed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transformClarifications is important - it parses the text/html content and injects the HTML needed for #44

@e-kolpakov
Copy link
Contributor

@itsjeyd There're some glitches of various severity and complexity:

  • "url_name for linking to this mentoring question set ..." used to be italicized, now it's not (now = in mentoring with steps)
  • Adding Steps and messages does not work - you've forgot to add CATEGORY constants
  • Might be a good idea to add some padding in preview view (though there aren't any in old mentoring, so if this takes more than 5 minutes it's ok to ignore it for now)
  • Multiple messages of the same type are allowed - should only allow one; xblock-utils does not support single-instance-per-boilerplate yet, so use the approach in old mentoring - probably in mentoring_edit.js
  • Rebase needed
  • Message hints used to be italicized and have smaller font in old mentoring:
    old_mentoring_message

Overall - looks like you've forgot to add mentoring-step and pb-mentoring to last rule in problem-builder-edit.css

@itsjeyd
Copy link
Member Author

itsjeyd commented Sep 14, 2015

@e-kolpakov As discussed in the hangout:

  • reverted answer.css
  • renamed lonely_step to lonely_child
  • moved sibling implementations for to subclasses of EnumerableChildMixin and added a note about providing a default implementation in the future

@itsjeyd itsjeyd force-pushed the mentoring-step-xblock branch from bb6dc8f to e8be9bf Compare September 14, 2015 15:57
@itsjeyd
Copy link
Member Author

itsjeyd commented Sep 14, 2015

@e-kolpakov FYI: I squashed the commits before rebasing but did preserve a copy of them just in case.

@e-kolpakov
Copy link
Contributor

@itsjeyd 👍 conditional the build passes.

itsjeyd added a commit that referenced this pull request Sep 14, 2015
New version of mentoring block that supports explicit steps
@itsjeyd itsjeyd merged commit f839d58 into master Sep 14, 2015
@xitij2000 xitij2000 deleted the mentoring-step-xblock branch December 13, 2019 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants