From 29f92887106d5cf6d8938d0b748c9c0def5af93a Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Tue, 21 Apr 2020 15:17:50 -0500 Subject: [PATCH 1/8] Ignore .DS_Store from git --- .DS_Store | Bin 6148 -> 0 bytes .gitignore | 1 + 2 files changed, 1 insertion(+) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 5008ddfcf53c02e82d7eee2e57c38e5672ef89f6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeH~Jr2S!425mzP>H1@V-^m;4Wg<&0T*E43hX&L&p$$qDprKhvt+--jT7}7np#A3 zem<@ulZcFPQ@L2!n>{z**++&mCkOWA81W14cNZlEfg7;MkzE(HCqgga^y>{tEnwC%0;vJ&^%eQ zLs35+`xjp>T0 Date: Wed, 22 Apr 2020 16:11:33 -0500 Subject: [PATCH 2/8] Add tests for mutation observers --- spec/JustNotSorrySpec.js | 102 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/spec/JustNotSorrySpec.js b/spec/JustNotSorrySpec.js index bf55b92..a4c4dbf 100644 --- a/spec/JustNotSorrySpec.js +++ b/spec/JustNotSorrySpec.js @@ -25,14 +25,28 @@ describe('JustNotSorry', function () { }); describe('#addObserver', function() { - it('adds an observer on focus of an element', function () { + it('adds an observer that listens for structural changes to the content editable div', function () { let target = document.getElementById('div-1'); spyOn(observer, 'observe'); target.addEventListener('focus', addObserver); expect(observer.observe).not.toHaveBeenCalled(); dispatchEventOnElement(target, 'focus'); - expect(observer.observe).toHaveBeenCalled(); + expect(observer.observe).toHaveBeenCalledWith(target, jasmine.objectContaining({childList: true})); + }); + + xit('adds an input event listener that checks for warnings', function() { + + }); + + it('adds warnings to the content editable div', function() { + let target = document.getElementById('div-1'); + spyOn(warningChecker, 'addWarnings'); + + target.addEventListener('focus', addObserver); + dispatchEventOnElement(target, 'focus'); + + expect(warningChecker.addWarnings).toHaveBeenCalledWith(target.parentNode); }); describe('when a global id variable is set', function() { @@ -49,7 +63,7 @@ describe('JustNotSorry', function () { target.addEventListener('focus', addObserver); dispatchEventOnElement(target, 'focus'); - target.innerHTML = '
'; + target.appendChild(document.createElement('BR')); setTimeout(function () { expect(id).toEqual('test value'); @@ -60,7 +74,24 @@ describe('JustNotSorry', function () { }); describe('#removeObserver', function() { - it('disconnects the observer on blur of an element', function () { + it('removes any existing warnings', function() { + let target = document.getElementById('div-2'); + spyOn(warningChecker, 'removeWarnings'); + + target.addEventListener('focus', addObserver); + dispatchEventOnElement(target, 'focus'); + + target.addEventListener('blur', removeObserver); + dispatchEventOnElement(target, 'blur'); + + expect(warningChecker.removeWarnings).toHaveBeenCalledWith(target.parentNode); + }); + + xit('removes the input event listener that checks for warnings', function() { + + }); + + it('disconnects the observer', function () { let target = document.getElementById('div-2'); spyOn(observer, 'observe'); spyOn(observer, 'disconnect'); @@ -91,4 +122,67 @@ describe('JustNotSorry', function () { expect(callCount).toEqual(3); }); }); + + describe('contentEditable observer', function () { + var newDivId = 'div-5'; + + afterEach(function () { + var targetDiv = document.getElementById(newDivId); + targetDiv.parentNode.removeChild(targetDiv); + }); + + it('dispatches an input event when nodes are added or removed from a content editable div', function (done) { + generateEditableDiv(newDivId); + const target = document.getElementById(newDivId); + + spyOn(warningChecker, 'addWarnings'); + spyOn(warningChecker, 'removeWarnings'); + + // trigger documentObserver to register this content editable div + target.appendChild(document.createElement('BR')); + + setTimeout(function () { + dispatchEventOnElement(target, 'focus'); + + setTimeout(function () { + let element = document.createElement('SPAN'); + element.textContent = 'Hello'; + target.appendChild(element); + + setTimeout(function () { + expect(warningChecker.removeWarnings).toHaveBeenCalledWith(target.parentNode); + expect(warningChecker.addWarnings).toHaveBeenCalledWith(target.parentNode); + done(); + }, 100); + }); + }); + }); + }); + + describe('documentObserver', function () { + var newDivId = 'div-4'; + + beforeEach(function () { + generateEditableDiv(newDivId); + }); + afterEach(function () { + var targetDiv = document.getElementById(newDivId); + targetDiv.parentNode.removeChild(targetDiv); + }); + + it('sets up event listeners when a new content editable div is added', function (done) { + spyOn(observer, 'observe'); + var targetDiv = document.getElementById(newDivId); + + // trigger documentObserver to register this content editable div + targetDiv.appendChild(document.createElement('BR')); + + setTimeout(function () { + dispatchEventOnElement(targetDiv, 'focus'); + expect(observer.observe).toHaveBeenCalledTimes(1); + expect(observer.observe).toHaveBeenCalledWith(targetDiv, jasmine.objectContaining({subtree: true, childList: true})); + done(); + }); + }); + }); }); \ No newline at end of file From 59ec77eafa930179126c7f6ba17d8b997be0e3d4 Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Thu, 23 Apr 2020 09:30:36 -0500 Subject: [PATCH 3/8] Improve performance by reducing the number of warning checker calls per key stroke --- spec/JustNotSorrySpec.js | 63 ++++++++++++++++++++++++++++++++++++++-- src/JustNotSorry.js | 8 ++--- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/spec/JustNotSorrySpec.js b/spec/JustNotSorrySpec.js index a4c4dbf..5573646 100644 --- a/spec/JustNotSorrySpec.js +++ b/spec/JustNotSorrySpec.js @@ -35,8 +35,19 @@ describe('JustNotSorry', function () { expect(observer.observe).toHaveBeenCalledWith(target, jasmine.objectContaining({childList: true})); }); - xit('adds an input event listener that checks for warnings', function() { + it('starts checking for warnings', function(done) { + spyOn(window, 'checkForWarnings').and.callThrough(); + let target = document.getElementById('div-1'); + + target.addEventListener('focus', addObserver); + dispatchEventOnElement(target, 'focus'); + + dispatchEventOnElement(target, 'input'); + setTimeout(function () { + expect(window.checkForWarnings).toHaveBeenCalled(); + done(); + }); }); it('adds warnings to the content editable div', function() { @@ -87,8 +98,22 @@ describe('JustNotSorry', function () { expect(warningChecker.removeWarnings).toHaveBeenCalledWith(target.parentNode); }); - xit('removes the input event listener that checks for warnings', function() { + it('no longer checks for warnings on input events', function(done) { + spyOn(window, 'checkForWarnings').and.callThrough(); + let target = document.getElementById('div-2'); + target.addEventListener('focus', addObserver); + dispatchEventOnElement(target, 'focus'); + + target.addEventListener('blur', removeObserver); + dispatchEventOnElement(target, 'blur'); + + dispatchEventOnElement(target, 'input'); + + setTimeout(function () { + expect(window.checkForWarnings).not.toHaveBeenCalled(); + done(); + }); }); it('disconnects the observer', function () { @@ -131,7 +156,7 @@ describe('JustNotSorry', function () { targetDiv.parentNode.removeChild(targetDiv); }); - it('dispatches an input event when nodes are added or removed from a content editable div', function (done) { + it('dispatches an input event when nodes are added to a content editable div', function (done) { generateEditableDiv(newDivId); const target = document.getElementById(newDivId); @@ -143,6 +168,8 @@ describe('JustNotSorry', function () { setTimeout(function () { dispatchEventOnElement(target, 'focus'); + expect(warningChecker.addWarnings).toHaveBeenCalledTimes(1); + warningChecker.addWarnings.calls.reset(); setTimeout(function () { let element = document.createElement('SPAN'); @@ -150,13 +177,43 @@ describe('JustNotSorry', function () { target.appendChild(element); setTimeout(function () { + expect(warningChecker.removeWarnings).toHaveBeenCalledTimes(1); expect(warningChecker.removeWarnings).toHaveBeenCalledWith(target.parentNode); + expect(warningChecker.addWarnings).toHaveBeenCalledTimes(1); expect(warningChecker.addWarnings).toHaveBeenCalledWith(target.parentNode); done(); }, 100); }); }); }); + + it('does not dispatch an input event when a class changes on a content editable div', function (done) { + generateEditableDiv(newDivId); + const target = document.getElementById(newDivId); + + spyOn(warningChecker, 'addWarnings'); + spyOn(warningChecker, 'removeWarnings'); + + // trigger documentObserver to register this content editable div + target.appendChild(document.createElement('BR')); + + setTimeout(function () { + dispatchEventOnElement(target, 'focus'); + expect(warningChecker.addWarnings).toHaveBeenCalledTimes(1); + warningChecker.addWarnings.calls.reset(); + + setTimeout(function () { + + target.className = 'test'; + + setTimeout(function () { + expect(warningChecker.removeWarnings).not.toHaveBeenCalled(); + expect(warningChecker.addWarnings).not.toHaveBeenCalled(); + done(); + }); + }); + }); + }); }); describe('documentObserver', function () { diff --git a/src/JustNotSorry.js b/src/JustNotSorry.js index 73a1eec..8ab62df 100644 --- a/src/JustNotSorry.js +++ b/src/JustNotSorry.js @@ -23,12 +23,14 @@ var observer = new MutationObserver(function(mutations) { }); var addObserver = function() { + this.addEventListener('input', checkForWarnings); warningChecker.addWarnings(this.parentNode); - observer.observe(this, {characterData: true, subtree: true, childList: true, attributes: true}); + observer.observe(this, {characterData: false, subtree: true, childList: true, attributes: false}); }; var removeObserver = function() { warningChecker.removeWarnings(this.parentNode); + this.removeEventListener('input', checkForWarnings); observer.disconnect(); }; @@ -40,7 +42,6 @@ var checkForWarnings = function() { var applyEventListeners = function(id) { var targetDiv = document.getElementById(id); targetDiv.addEventListener('focus', addObserver); - targetDiv.addEventListener('input', checkForWarnings); targetDiv.addEventListener('blur', removeObserver); }; @@ -48,11 +49,10 @@ var documentObserver = new MutationObserver(function(mutations) { var divCount = getEditableDivs().length; if (divCount !== editableDivCount) { editableDivCount = divCount; - var id; if (mutations[0]) { mutations.forEach(function(mutation) { if (mutation.type === 'childList' && mutation.target.hasAttribute('contentEditable')) { - id = mutation.target.id; + var id = mutation.target.id; if (id) { applyEventListeners(id); } From 40fc73598f337a4b7a2707cd725e0c0ff6738b36 Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Fri, 24 Apr 2020 11:07:21 -0500 Subject: [PATCH 4/8] Reduce the number of DOM updates by using a debounce function --- spec/JustNotSorrySpec.js | 2 +- src/JustNotSorry.js | 24 ++++++++++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/spec/JustNotSorrySpec.js b/spec/JustNotSorrySpec.js index 5573646..8c825c8 100644 --- a/spec/JustNotSorrySpec.js +++ b/spec/JustNotSorrySpec.js @@ -182,7 +182,7 @@ describe('JustNotSorry', function () { expect(warningChecker.addWarnings).toHaveBeenCalledTimes(1); expect(warningChecker.addWarnings).toHaveBeenCalledWith(target.parentNode); done(); - }, 100); + }, 510); }); }); }); diff --git a/src/JustNotSorry.js b/src/JustNotSorry.js index 8ab62df..54aefc9 100644 --- a/src/JustNotSorry.js +++ b/src/JustNotSorry.js @@ -3,6 +3,26 @@ var warningChecker = new WarningChecker(WARNINGS); var editableDivCount = 0; +// from underscore.js +// Returns a function, that, as long as it continues to be invoked, will not +// be triggered. The function will be called after it stops being called for +// N milliseconds. If `immediate` is passed, trigger the function on the +// leading edge, instead of the trailing. +function debounce(func, wait, immediate) { + var timeout; + return function() { + var context = this, args = arguments; + var later = function() { + timeout = null; + if (!immediate) func.apply(context, args); + }; + var callNow = immediate && !timeout; + clearTimeout(timeout); + timeout = setTimeout(later, wait); + if (callNow) func.apply(context, args); + }; +} + var observer = new MutationObserver(function(mutations) { if (mutations[0]) { mutations.forEach(function(mutation) { @@ -34,10 +54,10 @@ var removeObserver = function() { observer.disconnect(); }; -var checkForWarnings = function() { +var checkForWarnings = debounce(function() { warningChecker.removeWarnings(this.parentNode); warningChecker.addWarnings(this.parentNode); -}; +}, 500); var applyEventListeners = function(id) { var targetDiv = document.getElementById(id); From bd4dc44e365ea127caf4e8709ac730890d9e79c6 Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Fri, 24 Apr 2020 11:07:49 -0500 Subject: [PATCH 5/8] Reduce layout thrashing to improve performance by using fastdom to batch DOM measurement and mutation tasks. --- gulpfile.js | 2 + lib/fastdom-promised.js | 77 +++++++++++ lib/fastdom.js | 244 +++++++++++++++++++++++++++++++++ manifest.json | 2 + spec/HighlightGeneratorSpec.js | 19 +-- spec/WarningCheckerSpec.js | 108 +++++++++------ src/HighlightGenerator.js | 27 ++-- src/ScriptLoader.js | 2 + src/WarningChecker.js | 23 +++- 9 files changed, 438 insertions(+), 66 deletions(-) create mode 100644 lib/fastdom-promised.js create mode 100644 lib/fastdom.js diff --git a/gulpfile.js b/gulpfile.js index b3faaa7..f049d2c 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -4,6 +4,8 @@ const watch = require('gulp-watch'); const open = require('gulp-open'); const filesForTest = [ + 'lib/fastdom.js', + 'lib/fastdom-promised.js', 'src/Warnings.js', 'src/WarningChecker.js', 'src/HighlightGenerator.js', diff --git a/lib/fastdom-promised.js b/lib/fastdom-promised.js new file mode 100644 index 0000000..45857a9 --- /dev/null +++ b/lib/fastdom-promised.js @@ -0,0 +1,77 @@ +!(function() { + + /** + * Wraps fastdom in a Promise API + * for improved control-flow. + * + * @example + * + * // returning a result + * fastdom.measure(() => el.clientWidth) + * .then(result => ...); + * + * // returning promises from tasks + * fastdom.measure(() => { + * var w = el1.clientWidth; + * return fastdom.mutate(() => el2.style.width = w + 'px'); + * }).then(() => console.log('all done')); + * + * // clearing pending tasks + * var promise = fastdom.measure(...) + * fastdom.clear(promise); + * + * @type {Object} + */ + var exports = { + initialize: function() { + this._tasks = new Map(); + }, + + mutate: function(fn, ctx) { + return create(this, 'mutate', fn, ctx); + }, + + measure: function(fn, ctx) { + return create(this, 'measure', fn, ctx); + }, + + clear: function(promise) { + var tasks = this._tasks; + var task = tasks.get(promise); + this.fastdom.clear(task); + tasks.delete(promise); + } + }; + + /** + * Create a fastdom task wrapped in + * a 'cancellable' Promise. + * + * @param {FastDom} fastdom + * @param {String} type - 'measure'|'muatate' + * @param {Function} fn + * @return {Promise} + */ + function create(promised, type, fn, ctx) { + var tasks = promised._tasks; + var fastdom = promised.fastdom; + var task; + + var promise = new Promise(function(resolve, reject) { + task = fastdom[type](function() { + tasks.delete(promise); + try { resolve(ctx ? fn.call(ctx) : fn()); } + catch (e) { reject(e); } + }, ctx); + }); + + tasks.set(promise, task); + return promise; + } + +// Expose to CJS, AMD or global + if ((typeof define)[0] == 'f') define(function() { return exports; }); + else if ((typeof module)[0] == 'o') module.exports = exports; + else window.fastdomPromised = exports; + +})(); diff --git a/lib/fastdom.js b/lib/fastdom.js new file mode 100644 index 0000000..70bf7d7 --- /dev/null +++ b/lib/fastdom.js @@ -0,0 +1,244 @@ +!(function(win) { + +/** + * FastDom + * + * Eliminates layout thrashing + * by batching DOM read/write + * interactions. + * + * @author Wilson Page + * @author Kornel Lesinski + */ + +'use strict'; + +/** + * Mini logger + * + * @return {Function} + */ +var debug = 0 ? console.log.bind(console, '[fastdom]') : function() {}; + +/** + * Normalized rAF + * + * @type {Function} + */ +var raf = win.requestAnimationFrame + || win.webkitRequestAnimationFrame + || win.mozRequestAnimationFrame + || win.msRequestAnimationFrame + || function(cb) { return setTimeout(cb, 16); }; + +/** + * Initialize a `FastDom`. + * + * @constructor + */ +function FastDom() { + var self = this; + self.reads = []; + self.writes = []; + self.raf = raf.bind(win); // test hook + debug('initialized', self); +} + +FastDom.prototype = { + constructor: FastDom, + + /** + * We run this inside a try catch + * so that if any jobs error, we + * are able to recover and continue + * to flush the batch until it's empty. + * + * @param {Array} tasks + */ + runTasks: function(tasks) { + debug('run tasks'); + var task; while (task = tasks.shift()) task(); + }, + + /** + * Adds a job to the read batch and + * schedules a new frame if need be. + * + * @param {Function} fn + * @param {Object} ctx the context to be bound to `fn` (optional). + * @public + */ + measure: function(fn, ctx) { + debug('measure'); + var task = !ctx ? fn : fn.bind(ctx); + this.reads.push(task); + scheduleFlush(this); + return task; + }, + + /** + * Adds a job to the + * write batch and schedules + * a new frame if need be. + * + * @param {Function} fn + * @param {Object} ctx the context to be bound to `fn` (optional). + * @public + */ + mutate: function(fn, ctx) { + debug('mutate'); + var task = !ctx ? fn : fn.bind(ctx); + this.writes.push(task); + scheduleFlush(this); + return task; + }, + + /** + * Clears a scheduled 'read' or 'write' task. + * + * @param {Object} task + * @return {Boolean} success + * @public + */ + clear: function(task) { + debug('clear', task); + return remove(this.reads, task) || remove(this.writes, task); + }, + + /** + * Extend this FastDom with some + * custom functionality. + * + * Because fastdom must *always* be a + * singleton, we're actually extending + * the fastdom instance. This means tasks + * scheduled by an extension still enter + * fastdom's global task queue. + * + * The 'super' instance can be accessed + * from `this.fastdom`. + * + * @example + * + * var myFastdom = fastdom.extend({ + * initialize: function() { + * // runs on creation + * }, + * + * // override a method + * measure: function(fn) { + * // do extra stuff ... + * + * // then call the original + * return this.fastdom.measure(fn); + * }, + * + * ... + * }); + * + * @param {Object} props properties to mixin + * @return {FastDom} + */ + extend: function(props) { + debug('extend', props); + if (typeof props != 'object') throw new Error('expected object'); + + var child = Object.create(this); + mixin(child, props); + child.fastdom = this; + + // run optional creation hook + if (child.initialize) child.initialize(); + + return child; + }, + + // override this with a function + // to prevent Errors in console + // when tasks throw + catch: null +}; + +/** + * Schedules a new read/write + * batch if one isn't pending. + * + * @private + */ +function scheduleFlush(fastdom) { + if (!fastdom.scheduled) { + fastdom.scheduled = true; + fastdom.raf(flush.bind(null, fastdom)); + debug('flush scheduled'); + } +} + +/** + * Runs queued `read` and `write` tasks. + * + * Errors are caught and thrown by default. + * If a `.catch` function has been defined + * it is called instead. + * + * @private + */ +function flush(fastdom) { + debug('flush'); + + var writes = fastdom.writes; + var reads = fastdom.reads; + var error; + + try { + debug('flushing reads', reads.length); + fastdom.runTasks(reads); + debug('flushing writes', writes.length); + fastdom.runTasks(writes); + } catch (e) { error = e; } + + fastdom.scheduled = false; + + // If the batch errored we may still have tasks queued + if (reads.length || writes.length) scheduleFlush(fastdom); + + if (error) { + debug('task errored', error.message); + if (fastdom.catch) fastdom.catch(error); + else throw error; + } +} + +/** + * Remove an item from an Array. + * + * @param {Array} array + * @param {*} item + * @return {Boolean} + */ +function remove(array, item) { + var index = array.indexOf(item); + return !!~index && !!array.splice(index, 1); +} + +/** + * Mixin own properties of source + * object into the target. + * + * @param {Object} target + * @param {Object} source + */ +function mixin(target, source) { + for (var key in source) { + if (source.hasOwnProperty(key)) target[key] = source[key]; + } +} + +// There should never be more than +// one instance of `FastDom` in an app +var exports = win.fastdom = (win.fastdom || new FastDom()); // jshint ignore:line + +// Expose to CJS & AMD +if ((typeof define) == 'function') define(function() { return exports; }); +else if ((typeof module) == 'object') module.exports = exports; + +})( typeof window !== 'undefined' ? window : this); diff --git a/manifest.json b/manifest.json index abc7ba9..c839ccd 100644 --- a/manifest.json +++ b/manifest.json @@ -35,6 +35,8 @@ }, "web_accessible_resources": [ "lib/dom-regexp-match-1.1.0.js", + "lib/fastdom.js", + "lib/fastdom-promised.js", "src/HighlightGenerator.js", "src/WarningChecker.js", "src/Warnings.js", diff --git a/spec/HighlightGeneratorSpec.js b/spec/HighlightGeneratorSpec.js index 10fe122..50b4aad 100644 --- a/spec/HighlightGeneratorSpec.js +++ b/spec/HighlightGeneratorSpec.js @@ -17,19 +17,22 @@ describe('HighlightGenerator', function () { range.getClientRects.and.returnValue(rects); }); - it('appends one highlight node to the parent for each client rect in the range', function () { - HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); - expect(parentNodeSpy.appendChild).toHaveBeenCalled(); - expect(parentNodeSpy.appendChild.calls.count()).toEqual(rects.length); + it('appends one highlight node to the parent for each client rect in the range', async function (done) { + await HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); + requestAnimationFrame(function () { + expect(parentNodeSpy.appendChild).toHaveBeenCalled(); + expect(parentNodeSpy.appendChild.calls.count()).toEqual(rects.length); + done(); + }) }); - it('sets the same message on all highlight nodes', function () { - HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); + it('sets the same message on all highlight nodes', async function () { + await HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); expect(mockNode.title).toEqual(message); }); - it('sets the warning class on the highlight nodes', function() { - HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); + it('sets the warning class on the highlight nodes', async function() { + await HighlightGenerator.highlightMatches(message, warningClass).call(parentNodeSpy, currMatch, range); expect(mockNode.className).toEqual(warningClass); }); }); diff --git a/spec/WarningCheckerSpec.js b/spec/WarningCheckerSpec.js index 3650bf2..2ff29b1 100644 --- a/spec/WarningCheckerSpec.js +++ b/spec/WarningCheckerSpec.js @@ -6,70 +6,82 @@ describe('WarningChecker', function() { checker = new WarningChecker({}); }); - it('delegates to domRegexpMatch', function() { + it('delegates to domRegexpMatch', async function() { var matcherSpy = spyOn(window, 'domRegexpMatch'); var content = 'test just test'; var $fixture = setFixtures(content); - checker.addWarning($fixture, 'just', 'warning message'); + await checker.addWarning($fixture, 'just', 'warning message'); expect(matcherSpy).toHaveBeenCalled(); }); - it('passes the message and warningClass to the highlight generator callback', function() { - var matcherSpy = spyOn(window, 'domRegexpMatch'); - var generatorSpy = spyOn(HighlightGenerator, 'highlightMatches'); + it('adds a warning for a single keyword', async function(done) { var content = 'test just test'; var $fixture = setFixtures(content); - checker.addWarning($fixture, 'just', 'warning message'); - expect(generatorSpy).toHaveBeenCalledWith('warning message', 'jns-warning'); - }); - - it('adds a warning for a single keyword', function() { - var content = 'test just test'; - var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'just', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(1); + await checker.addWarning($fixture[0], 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(1); + done(); + }); }); - it('does not add warnings for partial matches', function() { + it('does not add warnings for partial matches', async function(done) { var content = 'test justify test'; var $fixture = setFixtures(content); - checker.addWarning($fixture, 'just', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(0); + await checker.addWarning($fixture, 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(0); + done(); + }); }); - it('adds multiple warnings when keyword is matched multiple times', function() { + it('adds multiple warnings when keyword is matched multiple times', async function(done) { var content = 'test just test just test'; var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'just', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(2); + await checker.addWarning($fixture[0], 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(2); + done(); + }); }); - it('adds a title element to provide a message in a tooltip', function() { + it('adds a title element to provide a message in a tooltip', async function(done) { var content = 'test just test sorry test'; var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'just', 'warning message'); - expect($fixture.find('div.jns-warning')[0].title).toEqual('warning message'); + await checker.addWarning($fixture[0], 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning')[0].title).toEqual('warning message'); + done(); + }); }); - it('matches case insensitive', function() { + it('matches case insensitive', async function(done) { var content = 'jUsT kidding'; var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'just', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(1); + await checker.addWarning($fixture[0], 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(1); + done(); + }); }); - it('catches keywords with punctuation', function() { + it('catches keywords with punctuation', async function(done) { var content = 'just. test'; var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'just', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(1); + await checker.addWarning($fixture[0], 'just', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(1); + done(); + }); }); - it('matches phrases', function() { + it('matches phrases', async function(done) { var content = 'my cat is so sorry because of you'; var $fixture = setFixtures(content); - checker.addWarning($fixture[0], 'so sorry', 'warning message'); - expect($fixture.find('div.jns-warning').length).toEqual(1); + await checker.addWarning($fixture[0], 'so sorry', 'warning message'); + requestAnimationFrame(function () { + expect($fixture.find('div.jns-warning').length).toEqual(1); + done(); + }); }); }); @@ -90,32 +102,42 @@ describe('WarningChecker', function() { }); describe('.addWarnings', function() { - it('does nothing when given an empty string', function() { + it('does nothing when given an empty string', async function(done) { var content = ''; var $fixture = setFixtures(content); - checker.addWarnings($fixture[0]); - expect($fixture.find('div.warning1').length).toEqual(0); + await checker.addWarnings($fixture[0]); + requestAnimationFrame(function () { + expect($fixture.find('div.warning1').length).toEqual(0); + done(); + }); }); - it('adds warnings to all keywords', function() { + it('adds warnings to all keywords', async function(done) { var content = 'I am just so sorry. Yes, just.'; var $fixture = setFixtures(content); - checker.addWarnings($fixture[0]); - expect($fixture.find('div.warning1').length).toEqual(3); - expect($fixture.find('div.warning1[title="test"]').length).toEqual(2); - expect($fixture.find('div.warning1[title="test 2"]').length).toEqual(1); + await checker.addWarnings($fixture[0]); + requestAnimationFrame(function () { + expect($fixture.find('div.warning1').length).toEqual(3); + expect($fixture.find('div.warning1[title="test"]').length).toEqual(2); + expect($fixture.find('div.warning1[title="test 2"]').length).toEqual(1); + done(); + }); }); }); describe('.removeWarnings', function() { - it('removes all warnings', function() { + it('removes all warnings', async function(done) { var content = 'I am so sorry'; var $fixture = setFixtures(content); $fixture.append($('
')); expect($fixture.find('div.warning1').length).toEqual(1); - checker.removeWarnings($fixture[0]); - expect($fixture.find('div.warning1').length).toEqual(0); + await checker.removeWarnings($fixture[0]); + + requestAnimationFrame(function () { + expect($fixture.find('div.warning1').length).toEqual(0); + done(); + }) }); }); }); diff --git a/src/HighlightGenerator.js b/src/HighlightGenerator.js index e4a375f..a42f5a4 100644 --- a/src/HighlightGenerator.js +++ b/src/HighlightGenerator.js @@ -1,17 +1,28 @@ var HighlightGenerator = window.HighlightGenerator = {}; const HIGHLIGHT_YPOS_ADJUSTMENT = 3; +const myFastdom = fastdom.extend(fastdomPromised); HighlightGenerator.highlightMatches = function highlightMatches(message, warningClass) { return function (currMatch, rangeToHighlight) { var parentNode = this; - var parentRect = parentNode.getBoundingClientRect(); - var rectsToHighlight = rangeToHighlight.getClientRects(); - for (var i = 0; i < rectsToHighlight.length; i++) { - var highlightNode = HighlightGenerator.highlightMatch(rectsToHighlight[i], parentRect); - highlightNode.title = message; - highlightNode.className = warningClass; - parentNode.appendChild(highlightNode); - } + return myFastdom.measure(function () { + var parentRect = parentNode.getBoundingClientRect(); + var rectsToHighlight = rangeToHighlight.getClientRects(); + var highlightNodes = []; + for (var i = 0; i < rectsToHighlight.length; i++) { + var highlightNode = HighlightGenerator.highlightMatch(rectsToHighlight[i], parentRect); + highlightNode.title = message; + highlightNode.className = warningClass; + highlightNodes.push(highlightNode); + } + return highlightNodes; + }).then(function (highlightNodes) { + myFastdom.mutate(function () { + highlightNodes.forEach(function (highlightNode) { + parentNode.appendChild(highlightNode); + }); + }) + }); } }; diff --git a/src/ScriptLoader.js b/src/ScriptLoader.js index 78740f2..bb9a620 100644 --- a/src/ScriptLoader.js +++ b/src/ScriptLoader.js @@ -1,5 +1,7 @@ var scriptsToLoad = [ chrome.extension.getURL('lib/dom-regexp-match-1.1.0.js'), + chrome.extension.getURL('lib/fastdom.js'), + chrome.extension.getURL('lib/fastdom-promised.js'), chrome.extension.getURL('src/HighlightGenerator.js'), chrome.extension.getURL('src/WarningChecker.js'), chrome.extension.getURL('src/Warnings.js'), diff --git a/src/WarningChecker.js b/src/WarningChecker.js index c79d493..d0a528f 100644 --- a/src/WarningChecker.js +++ b/src/WarningChecker.js @@ -7,21 +7,30 @@ function WarningChecker(options) { WarningChecker.prototype.addWarning = function addWarning(node, keyword, message) { 'use strict'; var pattern = new RegExp('\\b(' + keyword + ')\\b', 'ig'); - domRegexpMatch(node, pattern, HighlightGenerator.highlightMatches(message, this.warningClass)); + var promises = []; + var warningClass = this.warningClass; + var promisifiedMatchCallback = function(match, range) { + var matchPromise = HighlightGenerator.highlightMatches(message, warningClass).call(node, match, range); + promises.push(matchPromise); + } + domRegexpMatch(node, pattern, promisifiedMatchCallback); + return Promise.all(promises); }; WarningChecker.prototype.addWarnings = function addWarnings(node) { 'use strict'; var _this = this; - this.warnings.forEach(function(warning) { - _this.addWarning(node, warning.keyword, warning.message); - }); + return Promise.all(this.warnings.map(function(warning) { + return _this.addWarning(node, warning.keyword, warning.message); + })); }; WarningChecker.prototype.removeWarnings = function removeWarnings(node) { 'use strict'; var elementsToRemove = document.getElementsByClassName(this.warningClass); - for (var i = elementsToRemove.length; i--;) { - elementsToRemove[i].remove(); - } + return myFastdom.mutate(function () { + for (var i = elementsToRemove.length; i--;) { + node.removeChild(elementsToRemove[i]); + } + }); }; From 7ae4d89019a75df32c9ca3d0a05abb7c69cef9cf Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Fri, 24 Apr 2020 11:12:06 -0500 Subject: [PATCH 6/8] Bump version to prepare for release --- manifest.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifest.json b/manifest.json index c839ccd..19164d1 100644 --- a/manifest.json +++ b/manifest.json @@ -3,7 +3,7 @@ "name": "Just Not Sorry -- the Gmail Plug-in", "short_name": "JustNotSorry", "author": "Steve Brudz, Manish Kakwani, Tami Reiss, and Eric Tillberg of Def Method", - "version": "1.6.2", + "version": "1.6.3", "description": "A Gmail Plug-in that warns you when you write emails using words which undermine your message", "icons": { "16": "img/JustNotSorry-16.png", "48": "img/JustNotSorry-48.png", diff --git a/package.json b/package.json index 6721c1f..9f2a1fb 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "just-not-sorry", "description": "Gmail Plug-in that warns you when you write emails using words which undermine your message", - "version": "1.6.2", + "version": "1.6.3", "author": "Steve Brudz, Manish Kakwani, Tami Reiss, and Eric Tillberg of Def Method", "license": "MIT", "repository": "https://github.com/defmethodinc/just-not-sorry", From e6a292c3f998f5a60028728870eff09f9fc26519 Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Fri, 24 Apr 2020 11:14:58 -0500 Subject: [PATCH 7/8] Add MIT license to fastdom files --- lib/fastdom-promised.js | 9 +++++++++ lib/fastdom.js | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/fastdom-promised.js b/lib/fastdom-promised.js index 45857a9..77f7328 100644 --- a/lib/fastdom-promised.js +++ b/lib/fastdom-promised.js @@ -21,6 +21,15 @@ * fastdom.clear(promise); * * @type {Object} + * + * (The MIT License) + * Copyright (c) 2016 Wilson Page wilsonpage@me.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the 'Software'), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ var exports = { initialize: function() { diff --git a/lib/fastdom.js b/lib/fastdom.js index 70bf7d7..a8562ac 100644 --- a/lib/fastdom.js +++ b/lib/fastdom.js @@ -9,6 +9,15 @@ * * @author Wilson Page * @author Kornel Lesinski + * + * (The MIT License) + * Copyright (c) 2016 Wilson Page wilsonpage@me.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the 'Software'), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ 'use strict'; From 50b009a61e3d3f7bd3eefd74dfffe7448f42fbda Mon Sep 17 00:00:00 2001 From: Steve Brudz Date: Fri, 24 Apr 2020 11:22:12 -0500 Subject: [PATCH 8/8] Extract debounce time into a constant --- spec/JustNotSorrySpec.js | 2 +- src/JustNotSorry.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/JustNotSorrySpec.js b/spec/JustNotSorrySpec.js index 8c825c8..7d16145 100644 --- a/spec/JustNotSorrySpec.js +++ b/spec/JustNotSorrySpec.js @@ -182,7 +182,7 @@ describe('JustNotSorry', function () { expect(warningChecker.addWarnings).toHaveBeenCalledTimes(1); expect(warningChecker.addWarnings).toHaveBeenCalledWith(target.parentNode); done(); - }, 510); + }, WAIT_TIME_BEFORE_RECALC_WARNINGS + 10); }); }); }); diff --git a/src/JustNotSorry.js b/src/JustNotSorry.js index 54aefc9..88d637e 100644 --- a/src/JustNotSorry.js +++ b/src/JustNotSorry.js @@ -2,6 +2,7 @@ var warningChecker = new WarningChecker(WARNINGS); var editableDivCount = 0; +var WAIT_TIME_BEFORE_RECALC_WARNINGS = 500; // from underscore.js // Returns a function, that, as long as it continues to be invoked, will not @@ -57,7 +58,7 @@ var removeObserver = function() { var checkForWarnings = debounce(function() { warningChecker.removeWarnings(this.parentNode); warningChecker.addWarnings(this.parentNode); -}, 500); +}, WAIT_TIME_BEFORE_RECALC_WARNINGS); var applyEventListeners = function(id) { var targetDiv = document.getElementById(id);