Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions common/lib/xmodule/xmodule/imageannotation_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from xblock.core import Scope, String
from xmodule.annotator_mixin import get_instructions, html_to_text
from xmodule.annotator_token import retrieve_token
from xblock.fragment import Fragment

import textwrap

Expand Down Expand Up @@ -94,7 +95,7 @@ def _extract_instructions(self, xmltree):
""" Removes <instructions> from the xmltree and returns them as a string, otherwise None. """
return get_instructions(xmltree)

def get_html(self):
def student_view(self, context):
""" Renders parameters to template. """
context = {
'display_name': self.display_name_with_default,
Expand All @@ -105,7 +106,10 @@ def get_html(self):
'openseadragonjson': self.openseadragonjson,
}

return self.system.render_template('imageannotation.html', context)
fragment = Fragment(self.system.render_template('imageannotation.html', context))
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/tinymce.full.min.js")
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js")
return fragment


class ImageAnnotationDescriptor(AnnotatableFields, RawDescriptor): # pylint: disable=abstract-method
Expand Down
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/tests/test_imageannotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ def test_extract_instructions(self):
actual = self.mod._extract_instructions(xmltree) # pylint: disable=protected-access
self.assertIsNone(actual)

def test_get_html(self):
def test_student_view(self):
"""
Tests the function that passes in all the information in the context that will be used in templates/textannotation.html
"""
context = self.mod.get_html()
context = self.mod.student_view({}).content
for key in ['display_name', 'instructions_html', 'annotation_storage', 'token', 'tag', 'openseadragonjson']:
self.assertIn(key, context)
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/tests/test_textannotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ def test_extract_instructions(self):
actual = self.mod._extract_instructions(xmltree) # pylint: disable=W0212
self.assertIsNone(actual)

def test_get_html(self):
def test_student_view(self):
"""
Tests the function that passes in all the information in the context that will be used in templates/textannotation.html
"""
context = self.mod.get_html()
context = self.mod.student_view({}).content
for key in ['display_name', 'tag', 'source', 'instructions_html', 'content_html', 'annotation_storage', 'token']:
self.assertIn(key, context)
4 changes: 2 additions & 2 deletions common/lib/xmodule/xmodule/tests/test_videoannotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ def test_get_extension(self):
self.assertEqual(expectedyoutube, result2)
self.assertEqual(expectednotyoutube, result1)

def test_get_html(self):
def test_student_view(self):
"""
Tests to make sure variables passed in truly exist within the html once it is all rendered.
"""
context = self.mod.get_html()
context = self.mod.student_view({}).content
for key in ['display_name', 'instructions_html', 'sourceUrl', 'typeSource', 'poster', 'annotation_storage']:
self.assertIn(key, context)
9 changes: 6 additions & 3 deletions common/lib/xmodule/xmodule/textannotation_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from xblock.core import Scope, String
from xmodule.annotator_mixin import get_instructions
from xmodule.annotator_token import retrieve_token

from xblock.fragment import Fragment
import textwrap

# Make '_' a no-op so we can scrape strings
Expand Down Expand Up @@ -73,7 +73,7 @@ def _extract_instructions(self, xmltree):
""" Removes <instructions> from the xmltree and returns them as a string, otherwise None. """
return get_instructions(xmltree)

def get_html(self):
def student_view(self, context):
""" Renders parameters to template. """
context = {
'course_key': self.runtime.course_id,
Expand All @@ -85,7 +85,10 @@ def get_html(self):
'annotation_storage': self.annotation_storage_url,
'token': retrieve_token(self.user_email, self.annotation_token_secret),
}
return self.system.render_template('textannotation.html', context)
fragment = Fragment(self.system.render_template('textannotation.html', context))
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/tinymce.full.min.js")
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js")
return fragment


class TextAnnotationDescriptor(AnnotatableFields, RawDescriptor):
Expand Down
9 changes: 6 additions & 3 deletions common/lib/xmodule/xmodule/videoannotation_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from xblock.core import Scope, String
from xmodule.annotator_mixin import get_instructions, get_extension
from xmodule.annotator_token import retrieve_token
from xblock.fragment import Fragment

import textwrap

Expand Down Expand Up @@ -72,7 +73,7 @@ def _get_extension(self, src_url):
''' get the extension of a given url '''
return get_extension(src_url)

def get_html(self):
def student_view(self, context):
""" Renders parameters to template. """
extension = self._get_extension(self.sourceurl)

Expand All @@ -87,8 +88,10 @@ def get_html(self):
'annotation_storage': self.annotation_storage_url,
'token': retrieve_token(self.user_email, self.annotation_token_secret),
}

return self.system.render_template('videoannotation.html', context)
fragment = Fragment(self.system.render_template('videoannotation.html', context))
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/tinymce.full.min.js")
fragment.add_javascript_url("/static/js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js")
return fragment


class VideoAnnotationDescriptor(AnnotatableFields, RawDescriptor):
Expand Down
3 changes: 0 additions & 3 deletions lms/templates/imageannotation.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
<%namespace name='static' file='/static_content.html'/>
${static.css(group='style-vendor-tinymce-content', raw=True)}
${static.css(group='style-vendor-tinymce-skin', raw=True)}
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/tinymce.full.min.js', raw=True)}" />
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js', raw=True)}" />
</div>
<div id="catchDIV">
## Translators: Notes below refer to annotations. They wil later be put under a "Notes" section.
Expand Down Expand Up @@ -180,7 +178,6 @@
optionsOSDA:{},

};
tinymce.baseURL = "${settings.STATIC_URL}" + "js/vendor/tinymce/js/tinymce";
var imgURLRoot = "${settings.STATIC_URL}" + "js/vendor/ova/catch/img/";

if (typeof Annotator != 'undefined'){
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you still need to set the baseURL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed to still work without it on my version. If I'm not mistaken this gets initialized originally and Daniel had added that change before to use our version of tinymce instead of Edx's. Now that only one is loaded I figured that wasn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested it again and that line doesn't seem to be necessary now that there's only one TinyMCE to worry about.

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh. Well, I think you probably could have worked around the issue by just removing that line, then. Nonetheless, I think this code ends up better, so I'm happy to have the rest of the pr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, haha. Well that's good to know at least. I just fixed the tests below so I guess once the tests pass it will get merged?

Expand Down
3 changes: 0 additions & 3 deletions lms/templates/textannotation.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
<%namespace name='static' file='/static_content.html'/>
${static.css(group='style-vendor-tinymce-content', raw=True)}
${static.css(group='style-vendor-tinymce-skin', raw=True)}
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/tinymce.full.min.js', raw=True)}" />
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js', raw=True)}" />

<div class="annotatable-wrapper">
<div class="annotatable-header">
Expand Down Expand Up @@ -167,7 +165,6 @@
};

var imgURLRoot = "${settings.STATIC_URL}" + "js/vendor/ova/catch/img/";
tinymce.baseURL = "${settings.STATIC_URL}" + "js/vendor/tinymce/js/tinymce";

//remove old instances
if (Annotator._instances.length !== 0) {
Expand Down
4 changes: 0 additions & 4 deletions lms/templates/videoannotation.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
<%namespace name='static' file='/static_content.html'/>
${static.css(group='style-vendor-tinymce-content', raw=True)}
${static.css(group='style-vendor-tinymce-skin', raw=True)}
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/tinymce.full.min.js', raw=True)}" />
<script type="text/javascript" src="${static.url('js/vendor/tinymce/js/tinymce/jquery.tinymce.min.js', raw=True)}" />


<div class="annotatable-wrapper">
<div class="annotatable-header">
Expand Down Expand Up @@ -168,7 +165,6 @@
};

var imgURLRoot = "${settings.STATIC_URL}" + "js/vendor/ova/catch/img/";
tinymce.baseURL = "${settings.STATIC_URL}" + "js/vendor/tinymce/js/tinymce";

//remove old instances
if (Annotator._instances.length !== 0) {
Expand Down