Initial implementation #1

itsjeyd · 2015-10-30T17:18:57Z

VectorDraw XBlock

This XBlock allows course authors to define vector drawing exercises. To complete a vector drawing exercise, students must correctly place a set of predefined vectors (and points) on a board.

All data associated with this XBlock is stored in XBlock fields. Vectors, points, and expected result for an exercise must be defined using JSON, but we are also planning on adding WYSIWYG functionality to Studio that will allow authors to specify vectors and expected results by drawing them (cf. below). This functionality will not replace existing functionality for specifying elements that are part of the exercise; it will instead be offered as an alternative method for creating content. For a given exercise, student state consists of:

the set of vectors and points that were present on the board when student last submitted an answer
a list of checks to perform in order to obtain grade
result returned by the grader for the most recent answer (which contains info about correctness of answer and feedback message).

The amount of data to store for an exercise depends on how complex it is (i.e., how many vectors and points there are in total); based on examples from an existing JSInput implementation, it should not exceed 1-2K per problem, per student. Grading happens by comparing the set of elements on the board to a set of conditions specified by the course author ("expected result"). Minimally, this involves checking for each required element whether or not it is present in a given answer. Additional checks may include:

checking coordinates of points
checking coordinates of tail and/or tip of a vector
checking the length of a vector
checking the angle of a vector
checking whether a line passes through one or more points.

None of these operations are very expensive computationally, and the grader short-circuits as soon as it encounters a failing check (there is currently no notion of a partially correct answer), so we don't expect there to be any performance bottlenecks. We haven't performed any load testing.

We'd like to get this XBlock on edx.org for Davidson College by mid-December.

Testing

Install the XBlock on Devstack via:
```
pip install -r requirements.txt
```
Add vectordraw to the Advanced Module List in your course settings.
Add a "Vector Drawing" block to a unit.
Customize the Vector Drawing block. You can use one of the examples from the jsinput-vectordraw repository (e.g., 2_boxIncline_multiVector) for this purpose. Note that values of "Vectors", "Points", "Expected result" settings need to be valid JSON.

Partner information hosted on edx.org (Davidson College)
Merge deadline mid-December
JIRA Story OSPR-935
Confluence / Product Asset N/A
Sandbox URL LMS (Example), Studio (Example; see author notes below about a known glitch)
Dependencies N/A

PR Author(s) Notes / To-Do

Integration tests
Accessibility
Moving controls for interacting with the board above the active board (as per this request).
There's a glitch in Studio that keeps vectors from being rendered properly when first loading the page (you have to click "Reset" to get a proper preview). We'll have another opportunity to address this in the context of the next item.
WYSIWYG functionality for defining vectors and expected results

Reviewers

Code: @smarnach
Code: @nedbat
Code: @singingwolfboy
Accessibility: @cptvitamin

smarnach · 2015-11-01T21:19:38Z

vectordraw/grader.py

+                return {'ok': False, 'msg': result}
+        return {'ok': True, 'msg': self.success_message}
+
+    def cfn(self, e, ans):


I don't think this function is needed for the XBlock version – it looks quite JSInput-specific.

@smarnach Ah, yes - missed that somehow when I was cleaning up. Thanks!

smarnach · 2015-11-01T22:08:41Z

@itsjeyd Looks quite good so far! I did a first pass through the code, and added a few comments.

So far I didn't do any manual testing – will do this in the next iteration.

singingwolfboy · 2015-12-08T16:16:46Z

vectordraw/templates/html/vectordraw.html

+          <span class="reset-label" aria-hidden="true">{% trans "Reset" %}</span>
+          <span class="sr">{% trans "Reset board to initial state" %}</span>
+        </button>
+        <button class="redo" title="Redo">


The title attribute should be translated.

itsjeyd · 2015-12-09T15:05:51Z

@singingwolfboy I addressed all of your comments. Please let me know if there is anything else you would like me to address.

singingwolfboy · 2015-12-09T15:18:54Z

vectordraw/vectordraw.py

+                raise ValueError
+        # If we get to this point, it means that vector and point data is valid;
+        # the only thing left to check is whether data contains a list of checks:
+        return 'checks' in data


Shouldn't this be:

if 'checks' not in data: raise ValueError return True

Or do you mean to have this return either True or False?

@singingwolfboy Ah, yes, now that this method raises ValueError if it comes across invalid data, it doesn't actually need to return True or False. So this:

if 'checks' not in data: raise ValueError

should be enough. Thanks!

singingwolfboy · 2015-12-09T15:20:10Z

I only found one last little issue, and everything else looks good. After you address it, 👍 from me. ~~(I'll check the checkbox in the PR description, as well.)~~ Looks like I don't have permission to check that checkbox, oh well.

itsjeyd · 2015-12-09T15:47:30Z

@singingwolfboy That's great, thank you! The fix for the last issue is here: c03551d

cptvitamin · 2015-12-09T16:10:37Z

vectordraw/templates/html/vectordraw.html

+  <div id="vectordraw">
+    <div class="menu" style="width: {{ self.width }}px;">
+      <div class="controls">
+        <span class="sr" id="element-list-add-label">{% trans "Select element to add to board" %}</span>


@itsjeyd can we make this a label element and change the aria-labelledby attribute below to a for attribute? It's better supported and won't trigger test failures.

cptvitamin · 2015-12-09T16:18:14Z

@itsjeyd can you make the :focus state on all of the buttons match the :hover state? Currently, there is no way for a keyboard user to know what button the are currently focused on.

cptvitamin · 2015-12-09T16:19:54Z

vectordraw/public/css/vectordraw.css

+
+/* Make sure screen-reader content is hidden in the workbench: */
+.vectordraw_block .sr {
+    display: none;


@itsjeyd you cannot have display:none on SR text. display none is the same as aria-hidden, so this text is not exposed to a screen reader user. all of the other styles below are effective for exposing it only to screen reader users.

cptvitamin · 2015-12-09T16:24:49Z

@itsjeyd this is looking great. I love the addition of the properties panel for keyboard only users. I also think it will help users with dexterity issues as well as all users who want to position things precisely. My only suggestion is that some sort of grid can be toggled into view. While attempting to use the manual controls I found it very difficult to position vectors using trial and error.

nedbat · 2015-12-09T16:33:40Z

👍

itsjeyd · 2015-12-10T11:31:55Z

@cptvitamin Thanks for the feedback! I addressed your comments: 461371b. As for your suggestion about making available a grid to help keyboard users place vectors, this is already available as an opt-in feature. Course authors can set an "Axis" option to "True" in Studio to get something like this:

To promote the use of this feature I changed the default for the "Axis" option to "True" (it was disabled by default before) and added a comment to the help text that points out that enabling the option makes an exercise more accessible for keyboard users. It would also be possible to add functionality for toggling the axis on demand but this would require more substantial changes (JSXGraph does not provide an API for toggling the axis after the drawing board has been created), so this is not something we can implement right now (considering that we are close to the merge deadline for this work). Please let me know if there is anything else I should address.

sarina · 2015-12-10T14:49:35Z

Mark is out sick today, and possibly tomorrow as well, so I don't think we
will be able to get more from him until next week.

On Thu, Dec 10, 2015 at 6:31 AM, Tim Krones [email protected]
wrote:

@cptvitamin https://github.com/cptvitamin Thanks for the feedback! I
addressed your comments: 461371b
461371b.
As for your suggestion about making available a grid to help keyboard users
place vectors, this is already available as an opt-in feature. Course
authors can set an "Axis" option to "True" in Studio to get something like
this:

[image: axis-enabled]
https://cloud.githubusercontent.com/assets/961441/11714281/57c1dd44-9f39-11e5-99e2-098ee45ffc0b.png

To promote the use of this feature I changed the default for the "Axis"
option to "True" (it was disabled by default before) and added a comment to
the help text that points out that enabling the option makes an exercise
more accessible for keyboard users. It would also be possible to add
functionality for toggling the axis on demand but this would require more
substantial changes (JSXGraph does not provide an API for toggling the axis
after the drawing board has been created), so this is not something we can
implement right now (considering that we are close to the merge deadline
for this work). Please let me know if there is anything else I should
address.

—
Reply to this email directly or view it on GitHub
#1 (comment)
.

smarnach · 2015-12-10T15:08:30Z

@sarina Oh, that's really unfortunate! We'd really like to get these XBlocks into the release, since the client wants to start using them, and the next release will be in January. Is there anyone who could fill in for Mark?

Alternatively, since we already have done a full accessibility review cycle on these XBlocks, addressed Mark's comments and have +1s from the other code reviews, it might be reasonable to just merge them now given the circumstances. If it turns out we need further accessibility tweaks, we could add them later, but things should be looking pretty good already. On the ActiveTable XBlock, there are no open questions left and only a formal +1 is missing, and on this one it's only Tim's last comment that's waiting for an answer from Mark.

sarina · 2015-12-10T15:20:22Z

OK, I would be fine with you merging this in, as long as you make certain
to follow up with Mark next week and address any final issues he may have.

On Thu, Dec 10, 2015 at 10:08 AM, Sven Marnach [email protected]
wrote:

@sarina https://github.com/sarina Oh, that's really unfortunate! We'd
really like to get these XBlocks into the release, since the client wants
to start using them, and the next release will be in January. Is there
anyone who could fill in for Mark?

Alternatively, since we already have done a full accessibility review
cycle on these XBlocks, addressed Mark's comments and have +1s from the
other code reviews, it might be reasonable to just merge them now given the
circumstances. If it turns out we need further accessibility tweaks, we
could add them later, but things should be looking pretty good already. On
the ActiveTable XBlock, there are no open questions left and only a formal
+1 is missing, and on this one it's only Tim's last comment that's waiting
for an answer from Mark.

—
Reply to this email directly or view it on GitHub
#1 (comment)
.

smarnach · 2015-12-10T15:22:00Z

@sarina Thanks a lot! That's a great relief for us. :)

itsjeyd · 2015-12-10T15:25:36Z

@sarina Yes, and we'll make sure to follow up with Mark next week.

Initial implementation

itsjeyd added 6 commits October 28, 2015 10:21

Add skeleton created by XBlock SDK.

6cb67fc

Allow to display and interact with exercises in the LMS.

9a9a324

Add support for exercise descriptions.

dd06d81

Add grading functionality.

aa7c134

Save and restore state.

c19b0e4

Clean up.

6c1b897

smarnach reviewed Nov 1, 2015
View reviewed changes

Address review comments.

6b87e0a

itsjeyd force-pushed the initial-implementation branch 2 times, most recently from f8162db to 0a934ed Compare November 2, 2015 12:54

itsjeyd added 2 commits November 2, 2015 14:48

Configure Travis.

c71e96b

Port tests.

a838256

itsjeyd force-pushed the initial-implementation branch from cc17da2 to a838256 Compare November 2, 2015 13:50

Fix PEP8 and pylint issues.

273232b

itsjeyd force-pushed the initial-implementation branch from d14939a to 273232b Compare November 2, 2015 16:03

Render template for board and menu on the server.

06d0cbb

itsjeyd force-pushed the initial-implementation branch from df52b71 to 06d0cbb Compare November 3, 2015 14:54

singingwolfboy reviewed Dec 8, 2015
View reviewed changes

Address upstream review comments.

645115f

singingwolfboy reviewed Dec 9, 2015
View reviewed changes

Address upstream review comments.

c03551d

cptvitamin reviewed Dec 9, 2015
View reviewed changes

itsjeyd force-pushed the initial-implementation branch from 70abbb5 to 685b588 Compare December 9, 2015 18:23

Address upstream review comments.

461371b

itsjeyd force-pushed the initial-implementation branch from 685b588 to 461371b Compare December 9, 2015 22:46

itsjeyd added a commit that referenced this pull request Dec 10, 2015

Merge pull request #1 from open-craft/initial-implementation

ded76fc

Initial implementation

itsjeyd merged commit ded76fc into master Dec 10, 2015

itsjeyd mentioned this pull request Dec 10, 2015

Add xblock-vectordraw to requirements. openedx/edx-platform#10932

Merged

bradenmacdonald deleted the initial-implementation branch March 18, 2016 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial implementation #1

Initial implementation #1

itsjeyd commented Oct 30, 2015

smarnach Nov 1, 2015

itsjeyd Nov 2, 2015

smarnach commented Nov 1, 2015

singingwolfboy Dec 8, 2015

itsjeyd commented Dec 9, 2015

singingwolfboy Dec 9, 2015

itsjeyd Dec 9, 2015

singingwolfboy commented Dec 9, 2015

itsjeyd commented Dec 9, 2015

cptvitamin Dec 9, 2015

cptvitamin commented Dec 9, 2015

cptvitamin Dec 9, 2015

cptvitamin commented Dec 9, 2015

nedbat commented Dec 9, 2015

itsjeyd commented Dec 10, 2015

sarina commented Dec 10, 2015

smarnach commented Dec 10, 2015

sarina commented Dec 10, 2015

smarnach commented Dec 10, 2015

itsjeyd commented Dec 10, 2015

Initial implementation #1

Initial implementation #1

Conversation

itsjeyd commented Oct 30, 2015

VectorDraw XBlock

smarnach Nov 1, 2015

Choose a reason for hiding this comment

itsjeyd Nov 2, 2015

Choose a reason for hiding this comment

smarnach commented Nov 1, 2015

singingwolfboy Dec 8, 2015

Choose a reason for hiding this comment

itsjeyd commented Dec 9, 2015

singingwolfboy Dec 9, 2015

Choose a reason for hiding this comment

itsjeyd Dec 9, 2015

Choose a reason for hiding this comment

singingwolfboy commented Dec 9, 2015

itsjeyd commented Dec 9, 2015

cptvitamin Dec 9, 2015

Choose a reason for hiding this comment

cptvitamin commented Dec 9, 2015

cptvitamin Dec 9, 2015

Choose a reason for hiding this comment

cptvitamin commented Dec 9, 2015

nedbat commented Dec 9, 2015

itsjeyd commented Dec 10, 2015

sarina commented Dec 10, 2015

smarnach commented Dec 10, 2015

sarina commented Dec 10, 2015

smarnach commented Dec 10, 2015

itsjeyd commented Dec 10, 2015