Show answer for imageresponse

[polesye]

BLD-21

"Show answer" on imageresponse should highlight the area that would have earned full credit if the student had left her marker there.

Example of XML:

<problem>
  <imageresponse>
    <imageinput src="https://studio.edx.org/c4x/edX/DemoX/asset/Dog-and-Cat.jpg" width="640" height="400" rectangle="(385,98)-(600,337);(185,18)-(400,120)" regions="[[[10,10], [20,30], [40, 10]], [[100,100], [120,130], [110,150], [110,350], [50,50]]]"/>
  </imageresponse>
  <solution>
    <div class="detailed-solution">
      <p>Explanation</p>
      <p>The animal on the right is a kitten. The animal on the left is a puppy, not a kitten.</p>
    </div>
  </solution>
</problem>

@jmclaus and @auraz please review.

[auraz]

remove this )

[polesye]

Removed.

[auraz]

👍

[auraz]

@wedaly please take a look on added tests
@jtauber we are adding new library: Jasmine image diff here.

[auraz]

please add example of answers of different types to docstring

[polesye]

Added.

[wedaly]

The unit testing approach seems reasonable, as long as @jtauber says it's okay to use the new library.

The change itself isn't well covered by the acceptance test suite, so please make sure you test this in all supported browsers on stage before we deploy in the next release.

[jtauber]

js-imagediff license is fine.

[jmclaus]

@polesye Instead of putting all this HTML markup here, why not put it in a separate html file in the fixtures directory and load it with 'read' function of jasmine-jquery?

[polesye]

It's not a full template, just a responsetype specific part, that will be prepended below in the code.
Also, I did so in style that is used in this spec.

[jmclaus]

@polesye I know. The read function will return a string ,not append a fixture to the DOM. And sure, you are consistent with the style in the spec. I just find that chunk a bit ugly and distracting.

[polesye]

@jmclaus Ok, I will fix that.

[jmclaus]

@polesye Thanks.

[polesye]

@jmclaus Done.

[auraz]

@jmclaus Short notation for regions was intended by product owner. Because rectangles are used more widely. May be we can improve it by having same internal and different externals formats?

[jmclaus]

@auraz I think that whatever format we choose, it should be consistent between rectangles, defined by a set of two point, and regions, defined by a set of three or more points. In rectangles, point coordinates are surrounded by parenthesis. In regions, it's brackets. Different rectangles are separated by semi-columns. In regions, it's commas.
If we want to make things more readable, sure, lets keep the rectangle notation, and, for example, define regions like this:

(100,20)-(30,110)-(40, 50); (200,25)-(130,110)-(40, 50)

[jmclaus]

@polesye As discussed on HipChat, these tests, that are specific to image input, should not be added to

common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee

[polesye]

@jmclaus please continue review.

[jmclaus]

@polesye We finally decided against "As discussed on HipChat, these tests, that are specific to image input, should not be added to common/lib/xmodule/xmodule/js/spec/capa/display_spec.coffee"

And my comment on format should be done in another PR, if ever, as it implies a significant amount of code change. So, 👍 once Jenkins agrees.

[valera-rozuvan]

@polesye 👍 good to merge!