search

openedx / xblock-image-explorer

GNU Affero General Public License v3.0
9 stars 39 forks source link

Address PR feedback #1

Closed chrisndodge closed 10 years ago

chrisndodge commented 10 years ago

@antoviaque

This addresses a number of your feedback points that you raised in the PR to add this to the list of requirements.txt

Note, I couldn't get the conversion to add_css_url() and add_javascript_url() to work properly. Basically - in STUDIO only - while the JS loaded (I saw in my network list), the JS entry point wasn't called, so the xblock was basically not working from a JS interaction side. The LMS appeared to work fine, so I'm chalking this up to still some work TBD in Studio and xBlocks.

So I kept the add_css() and add_javascript().

antoviaque commented 10 years ago

Otherwise looks good!

chrisndodge commented 10 years ago

@antoviaque ok, added feedback. I did manual testing of the negative case