Commit 79c9e832 authored by Phil Hughes's avatar Phil Hughes Committed by Clement Ho

Merge branch '33483-fix-note-highlight-being-lost-on-note-update' into 'master'

Fix note highlight being lost after real time update

Closes #33483

See merge request !12098
parent c15078e0
...@@ -56,6 +56,7 @@ const normalizeNewlines = function(str) { ...@@ -56,6 +56,7 @@ const normalizeNewlines = function(str) {
this.toggleCommitList = this.toggleCommitList.bind(this); this.toggleCommitList = this.toggleCommitList.bind(this);
this.postComment = this.postComment.bind(this); this.postComment = this.postComment.bind(this);
this.clearFlashWrapper = this.clearFlash.bind(this); this.clearFlashWrapper = this.clearFlash.bind(this);
this.onHashChange = this.onHashChange.bind(this);
this.notes_url = notes_url; this.notes_url = notes_url;
this.note_ids = note_ids; this.note_ids = note_ids;
...@@ -127,7 +128,9 @@ const normalizeNewlines = function(str) { ...@@ -127,7 +128,9 @@ const normalizeNewlines = function(str) {
$(document).on('ajax:success', '.js-main-target-form', this.resetMainTargetForm); $(document).on('ajax:success', '.js-main-target-form', this.resetMainTargetForm);
$(document).on('ajax:complete', '.js-main-target-form', this.reenableTargetFormSubmitButton); $(document).on('ajax:complete', '.js-main-target-form', this.reenableTargetFormSubmitButton);
// when a key is clicked on the notes // when a key is clicked on the notes
return $(document).on('keydown', '.js-note-text', this.keydownNoteText); $(document).on('keydown', '.js-note-text', this.keydownNoteText);
// When the URL fragment/hash has changed, `#note_xxx`
return $(window).on('hashchange', this.onHashChange);
}; };
Notes.prototype.cleanBinding = function() { Notes.prototype.cleanBinding = function() {
...@@ -148,6 +151,7 @@ const normalizeNewlines = function(str) { ...@@ -148,6 +151,7 @@ const normalizeNewlines = function(str) {
$(document).off('ajax:success', '.js-main-target-form'); $(document).off('ajax:success', '.js-main-target-form');
$(document).off('ajax:success', '.js-discussion-note-form'); $(document).off('ajax:success', '.js-discussion-note-form');
$(document).off('ajax:complete', '.js-main-target-form'); $(document).off('ajax:complete', '.js-main-target-form');
$(window).off('hashchange', this.onHashChange);
}; };
Notes.initCommentTypeToggle = function (form) { Notes.initCommentTypeToggle = function (form) {
...@@ -298,8 +302,27 @@ const normalizeNewlines = function(str) { ...@@ -298,8 +302,27 @@ const normalizeNewlines = function(str) {
Notes.prototype.setupNewNote = function($note) { Notes.prototype.setupNewNote = function($note) {
// Update datetime format on the recent note // Update datetime format on the recent note
gl.utils.localTimeAgo($note.find('.js-timeago'), false); gl.utils.localTimeAgo($note.find('.js-timeago'), false);
this.collapseLongCommitList(); this.collapseLongCommitList();
this.taskList.init(); this.taskList.init();
// This stops the note highlight, #note_xxx`, from being removed after real time update
// The `:target` selector does not re-evaluate after we replace element in the DOM
Notes.updateNoteTargetSelector($note);
this.$noteToCleanHighlight = $note;
};
Notes.prototype.onHashChange = function() {
if (this.$noteToCleanHighlight) {
Notes.updateNoteTargetSelector(this.$noteToCleanHighlight);
}
this.$noteToCleanHighlight = null;
};
Notes.updateNoteTargetSelector = function($note) {
const hash = gl.utils.getLocationHash();
$note.toggleClass('target', hash && $note.filter(`#${hash}`).length > 0);
}; };
/* /*
...@@ -597,13 +620,12 @@ const normalizeNewlines = function(str) { ...@@ -597,13 +620,12 @@ const normalizeNewlines = function(str) {
$noteEntityEl = $(noteEntity.html); $noteEntityEl = $(noteEntity.html);
$noteEntityEl.addClass('fade-in-full'); $noteEntityEl.addClass('fade-in-full');
this.revertNoteEditForm($targetNote); this.revertNoteEditForm($targetNote);
gl.utils.localTimeAgo($('.js-timeago', $noteEntityEl));
$noteEntityEl.renderGFM(); $noteEntityEl.renderGFM();
$noteEntityEl.find('.js-task-list-container').taskList('enable');
// Find the note's `li` element by ID and replace it with the updated HTML // Find the note's `li` element by ID and replace it with the updated HTML
$note_li = $('.note-row-' + noteEntity.id); $note_li = $('.note-row-' + noteEntity.id);
$note_li.replaceWith($noteEntityEl); $note_li.replaceWith($noteEntityEl);
this.setupNewNote($noteEntityEl);
if (typeof gl.diffNotesCompileComponents !== 'undefined') { if (typeof gl.diffNotesCompileComponents !== 'undefined') {
gl.diffNotesCompileComponents(); gl.diffNotesCompileComponents();
......
...@@ -126,6 +126,7 @@ import '~/notes'; ...@@ -126,6 +126,7 @@ import '~/notes';
const deferred = $.Deferred(); const deferred = $.Deferred();
spyOn($, 'ajax').and.returnValue(deferred.promise()); spyOn($, 'ajax').and.returnValue(deferred.promise());
spyOn(this.notes, 'revertNoteEditForm'); spyOn(this.notes, 'revertNoteEditForm');
spyOn(this.notes, 'setupNewNote');
$('.js-comment-button').click(); $('.js-comment-button').click();
deferred.resolve(noteEntity); deferred.resolve(noteEntity);
...@@ -136,6 +137,46 @@ import '~/notes'; ...@@ -136,6 +137,46 @@ import '~/notes';
this.notes.updateNote(updatedNote, $targetNote); this.notes.updateNote(updatedNote, $targetNote);
expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote); expect(this.notes.revertNoteEditForm).toHaveBeenCalledWith($targetNote);
expect(this.notes.setupNewNote).toHaveBeenCalled();
});
});
describe('updateNoteTargetSelector', () => {
const hash = 'note_foo';
let $note;
beforeEach(() => {
$note = $(`<div id="${hash}"></div>`);
spyOn($note, 'filter').and.callThrough();
spyOn($note, 'toggleClass').and.callThrough();
});
it('sets target when hash matches', () => {
spyOn(gl.utils, 'getLocationHash');
gl.utils.getLocationHash.and.returnValue(hash);
Notes.updateNoteTargetSelector($note);
expect($note.filter).toHaveBeenCalledWith(`#${hash}`);
expect($note.toggleClass).toHaveBeenCalledWith('target', true);
});
it('unsets target when hash does not match', () => {
spyOn(gl.utils, 'getLocationHash');
gl.utils.getLocationHash.and.returnValue('note_doesnotexist');
Notes.updateNoteTargetSelector($note);
expect($note.toggleClass).toHaveBeenCalledWith('target', false);
});
it('unsets target when there is not a hash fragment anymore', () => {
spyOn(gl.utils, 'getLocationHash');
gl.utils.getLocationHash.and.returnValue(null);
Notes.updateNoteTargetSelector($note);
expect($note.toggleClass).toHaveBeenCalledWith('target', null);
}); });
}); });
...@@ -189,9 +230,13 @@ import '~/notes'; ...@@ -189,9 +230,13 @@ import '~/notes';
Notes.isUpdatedNote.and.returnValue(true); Notes.isUpdatedNote.and.returnValue(true);
const $note = $('<div>'); const $note = $('<div>');
$notesList.find.and.returnValue($note); $notesList.find.and.returnValue($note);
const $newNote = $(note.html);
Notes.animateUpdateNote.and.returnValue($newNote);
Notes.prototype.renderNote.call(notes, note, null, $notesList); Notes.prototype.renderNote.call(notes, note, null, $notesList);
expect(Notes.animateUpdateNote).toHaveBeenCalledWith(note.html, $note); expect(Notes.animateUpdateNote).toHaveBeenCalledWith(note.html, $note);
expect(notes.setupNewNote).toHaveBeenCalledWith($newNote);
}); });
describe('while editing', () => { describe('while editing', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment