Add assertions against stray classes

In previous iterations of the classPersister, we
found issues with the implementation meant classes
it should have added back to elements were also
added to other elements. This adds tests for this
scenario to ensure it doesn't happen again.

Also includes changes to fix a linting error with
the JS which complained about a function being
defined in a loop while referencing variables in
the outer scope.
This commit is contained in:
Tom Byers
2022-02-18 11:55:13 +00:00
parent 85e14d302c
commit 8521d1e45f
2 changed files with 14 additions and 9 deletions

View File

@@ -38,18 +38,19 @@
});
};
ClassesPersister.prototype.replace = function () {
var replaceClasses = (idx, el) => {
// Avoid updating elements that are no longer present.
// elements removed will still exist in memory but won't be attached to the DOM any more
if (global.document.body.contains(el)) {
$(el).addClass(className);
}
};
var className;
for (className in this._classesTo$ElsMap) {
this._classesTo$ElsMap[className].each((idx, el) => {
// Avoid updating elements that are no longer present.
// elements removed will still exist in memory but won't be attached to the DOM any more
if (global.document.body.contains(el)) {
$(el).addClass(className);
}
});
this._classesTo$ElsMap[className].each(replaceClasses);
}
// remove references to elements

View File

@@ -491,6 +491,10 @@ describe('Update content', () => {
expect(partialsInPage[0].querySelectorAll('.file-list h2')[0].classList.contains('js-child-has-focus')).toBe(true);
expect(partialsInPage[1].querySelectorAll('.file-list h2')[0].classList.contains('js-2nd-child-has-focus')).toBe(true);
// check each heading only has the classes assigned to it before updates occurred
expect(partialsInPage[0].querySelectorAll('.file-list h2')[0].classList.contains('js-2nd-child-has-focus')).toBe(false);
expect(partialsInPage[1].querySelectorAll('.file-list h2')[0].classList.contains('js-child-has-focus')).toBe(false);
});
});