Merge pull request #6687 from edx/anton/edxnotes-fix-clicking-bug
TNL-1116: Fix clicking bug.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
|
||||
@@ -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) {
|
||||
'</li>'
|
||||
].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;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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($('<span class="annotator-hl" />').appendTo(annotators[index].element));
|
||||
_.each(annotators, function(annotator) {
|
||||
highlights.push($('<span class="annotator-hl" />').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'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user