diff --git a/common/test/acceptance/tests/lms/test_lms_edxnotes.py b/common/test/acceptance/tests/lms/test_lms_edxnotes.py
index 206662767b..a366c7763d 100644
--- a/common/test/acceptance/tests/lms/test_lms_edxnotes.py
+++ b/common/test/acceptance/tests/lms/test_lms_edxnotes.py
@@ -699,9 +699,7 @@ class EdxNotesToggleSingleNoteTest(EdxNotesTestMixin):
And I move mouse out of the note
Then I see that the note is still shown
When I click on highlighted text in the second component
- Then I do not see any notes
- When I click again on highlighted text in the second component
- Then I see appropriate note
+ Then I see that the new note is shown
"""
note_1 = self.note_unit_page.notes[0]
note_2 = self.note_unit_page.notes[1]
@@ -712,9 +710,6 @@ class EdxNotesToggleSingleNoteTest(EdxNotesTestMixin):
note_2.click_on_highlight()
self.assertFalse(note_1.is_visible)
- self.assertFalse(note_2.is_visible)
-
- note_2.click_on_highlight()
self.assertTrue(note_2.is_visible)
diff --git a/lms/static/js/edxnotes/views/shim.js b/lms/static/js/edxnotes/views/shim.js
index 3c318c97e3..a17fe606ac 100644
--- a/lms/static/js/edxnotes/views/shim.js
+++ b/lms/static/js/edxnotes/views/shim.js
@@ -15,7 +15,7 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
if (!$.fn.addBack) {
$.fn.addBack = function (selector) {
return this.add(
- selector === null ? this.prevObject : this.prevObject.filter(selector)
+ selector === null ? this.prevObject : this.prevObject.filter(selector)
);
};
}
@@ -36,11 +36,11 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
**/
Annotator.Plugin.Auth.prototype.haveValidToken = function() {
return (
- this._unsafeToken &&
- this._unsafeToken.sub &&
- this._unsafeToken.exp &&
- this._unsafeToken.iat &&
- this.timeToExpiry() > 0
+ this._unsafeToken &&
+ this._unsafeToken.sub &&
+ this._unsafeToken.exp &&
+ this._unsafeToken.iat &&
+ this.timeToExpiry() > 0
);
};
@@ -91,6 +91,7 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
}
// Unbind onNoteClick from click
this.viewer.element.off('click', this.onNoteClick);
+ this.wrapper.off('click', '.annotator-hl');
}
);
@@ -114,19 +115,6 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
''
].join('');
- /**
- * Modifies Annotator._setupViewer to add a "click" event on viewer.
- **/
- Annotator.prototype._setupViewer = _.compose(
- function () {
- this.viewer.element.on('click', _.bind(this.onNoteClick, this));
- return this;
- },
- Annotator.prototype._setupViewer
- );
-
- Annotator.Editor.prototype.isShown = Annotator.Viewer.prototype.isShown;
-
/**
* Modifies Annotator.onHighlightMouseover to avoid showing the viewer if the
* editor is opened.
@@ -143,24 +131,42 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
Annotator.prototype._setupViewer
);
- $.extend(true, Annotator.prototype, {
- events: {
- '.annotator-hl click': 'onHighlightClick',
- '.annotator-viewer click': 'onNoteClick'
+ /**
+ * Modifies Annotator._setupViewer to add a "click" event on viewer.
+ **/
+ Annotator.prototype._setupViewer = _.compose(
+ function () {
+ this.viewer.element.on('click', _.bind(this.onNoteClick, this));
+ return this;
},
+ Annotator.prototype._setupViewer
+ );
+ /**
+ * Modifies Annotator._setupWrapper to add a "click" event on '.annotator-hl'.
+ **/
+ Annotator.prototype._setupWrapper = _.compose(
+ function () {
+ this.element.on('click', '.annotator-hl', _.bind(this.onHighlightClick, this));
+ return this;
+ },
+ Annotator.prototype._setupWrapper
+ );
+
+ Annotator.Editor.prototype.isShown = Annotator.Viewer.prototype.isShown;
+
+ $.extend(true, Annotator.prototype, {
isFrozen: false,
uid: _.uniqueId(),
onHighlightClick: function (event) {
- Annotator.Util.preventEventDefault(event);
-
- if (!this.isFrozen) {
- event.stopPropagation();
+ event.stopPropagation();
+ if (!this.editor.isShown()) {
+ this.unfreezeAll();
this.onHighlightMouseover.call(this, event);
+ Annotator.frozenSrc = this;
+ this.freezeAll();
}
- Annotator.frozenSrc = this;
- this.freezeAll();
},
onNoteClick: function (event) {
@@ -181,6 +187,7 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
$(document).on('click.edxnotes:freeze' + this.uid, _.bind(this.unfreeze, this));
this.isFrozen = true;
}
+ return this;
},
unfreeze: function () {
@@ -192,23 +199,27 @@ define(['jquery', 'underscore', 'annotator'], function ($, _, Annotator) {
'mouseout': this.startViewerHideTimer
});
this.viewer.hide();
- $(document).off('click.edxnotes:freeze'+this.uid);
+ $(document).off('click.edxnotes:freeze' + this.uid);
this.isFrozen = false;
Annotator.frozenSrc = null;
}
+ return this;
},
freezeAll: function () {
_.invoke(Annotator._instances, 'freeze');
+ return this;
},
unfreezeAll: function () {
_.invoke(Annotator._instances, 'unfreeze');
+ return this;
},
showFrozenViewer: function (annotations, location) {
this.showViewer(annotations, location);
this.freezeAll();
+ return this;
}
});
});
diff --git a/lms/static/js/spec/edxnotes/views/shim_spec.js b/lms/static/js/spec/edxnotes/views/shim_spec.js
index 25cf191da3..f1abd7aab9 100644
--- a/lms/static/js/spec/edxnotes/views/shim_spec.js
+++ b/lms/static/js/spec/edxnotes/views/shim_spec.js
@@ -29,15 +29,15 @@ define([
loadFixtures('js/fixtures/edxnotes/edxnotes_wrapper.html');
highlights = [];
annotators = [
- NotesFactory.factory($('div#edx-notes-wrapper-123').get(0), {
+ NotesFactory.factory($('#edx-notes-wrapper-123').get(0), {
endpoint: 'http://example.com/'
}),
- NotesFactory.factory($('div#edx-notes-wrapper-456').get(0), {
+ NotesFactory.factory($('#edx-notes-wrapper-456').get(0), {
endpoint: 'http://example.com/'
})
];
- _.each(annotators, function(annotator, index) {
- highlights.push($('').appendTo(annotators[index].element));
+ _.each(annotators, function(annotator) {
+ highlights.push($('').appendTo(annotator.element));
spyOn(annotator, 'onHighlightClick').andCallThrough();
spyOn(annotator, 'onHighlightMouseover').andCallThrough();
spyOn(annotator, 'startViewerHideTimer').andCallThrough();
@@ -54,6 +54,14 @@ define([
highlights[0].mouseover();
expect($('#edx-notes-wrapper-123 .annotator-editor')).not.toHaveClass('annotator-hide');
expect($('#edx-notes-wrapper-123 .annotator-viewer')).toHaveClass('annotator-hide');
+
+ it('clicking on highlights does not open the viewer when the editor is opened', function() {
+ spyOn(annotators[1].editor, 'isShown').andReturn(false);
+ highlights[0].click();
+ annotators[1].editor.isShown.andReturn(true);
+ highlights[1].click();
+ expect($('#edx-notes-wrapper-123 .annotator-viewer')).not.toHaveClass('annotator-hide');
+ expect($('#edx-notes-wrapper-456 .annotator-viewer')).toHaveClass('annotator-hide');
});
it('clicking a highlight freezes mouseover and mouseout in all highlighted text', function() {
@@ -65,10 +73,7 @@ define([
// in turn calls onHighlightMouseover.
// To test if onHighlightMouseover is called or not on
// mouseover, we'll have to reset onHighlightMouseover.
- expect(annotators[0].onHighlightClick).toHaveBeenCalled();
- expect(annotators[0].onHighlightMouseover).toHaveBeenCalled();
annotators[0].onHighlightMouseover.reset();
-
// Check that both instances of annotator are frozen
_.invoke(highlights, 'mouseover');
_.invoke(highlights, 'mouseout');
@@ -121,11 +126,13 @@ define([
checkClickEventsNotBound('edxnotes:freeze' + annotators[1].uid);
});
- it('should unbind onNotesLoaded on destruction', function() {
+ it('should unbind events on destruction', function() {
annotators[0].destroy();
expect($.fn.off).toHaveBeenCalledWith(
- 'click',
- annotators[0].onNoteClick
+ 'click', annotators[0].onNoteClick
+ );
+ expect($.fn.off).toHaveBeenCalledWith(
+ 'click', '.annotator-hl'
);
});
});