From bbde134f0a7b73a266fc50931e4baad3c3576bd9 Mon Sep 17 00:00:00 2001 From: Alec Gibson Date: Thu, 2 Jan 2020 11:22:18 +0000 Subject: [PATCH] Update toolbar on format This change fixes the following bug: 1. Start with plain text "foo bar" in the editor 2. Highlight "bar" (NB: it's important to select text that's not at the start) 3. Hit Ctrl+B to apply bold formatting 4. Notice that the bold button in the toolbar doesn't receive the `ql-active` class, when it should This appears to happen because the listener on `SCROLL_OPTIMIZE` gets an incorrect range. Instead of listening on optimize, we listen for all `EDITOR_CHANGE` events, which _do_ get the correct range. --- modules/toolbar.js | 7 +------ test/unit/modules/toolbar.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/modules/toolbar.js b/modules/toolbar.js index a16923ec0a..e203cd3b13 100644 --- a/modules/toolbar.js +++ b/modules/toolbar.js @@ -33,12 +33,7 @@ class Toolbar extends Module { this.attach(input); }, ); - this.quill.on(Quill.events.EDITOR_CHANGE, (type, range) => { - if (type === Quill.events.SELECTION_CHANGE) { - this.update(range); - } - }); - this.quill.on(Quill.events.SCROLL_OPTIMIZE, () => { + this.quill.on(Quill.events.EDITOR_CHANGE, () => { const [range] = this.quill.selection.getRange(); // quill.getSelection triggers update this.update(range); }); diff --git a/test/unit/modules/toolbar.js b/test/unit/modules/toolbar.js index 7e04fc911d..f316bcfe62 100644 --- a/test/unit/modules/toolbar.js +++ b/test/unit/modules/toolbar.js @@ -181,5 +181,15 @@ describe('Toolbar', function() { expect(centerButton.classList.contains('ql-active')).toBe(false); expect(leftButton.classList.contains('ql-active')).toBe(false); }); + + it('update on format', function() { + const boldButton = this.container.parentNode.querySelector( + 'button.ql-bold', + ); + this.quill.setSelection(1, 2); + expect(boldButton.classList.contains('ql-active')).toBe(false); + this.quill.format('bold', true, 'user'); + expect(boldButton.classList.contains('ql-active')).toBe(true); + }); }); });