From 7a33849d8d509e84c493cd5e1fb0d78c532a2971 Mon Sep 17 00:00:00 2001 From: Sumedha Pramod Date: Tue, 12 Dec 2017 11:22:22 -0800 Subject: [PATCH] Fix: Scroll to bottom of mobile dialogs (#70) - Validate text areas when you cancel an annotations reply --- src/AnnotationDialog.js | 4 +++- src/__tests__/AnnotationDialog-test.js | 8 ++++++-- src/__tests__/util-test.js | 26 +++++--------------------- src/util.js | 4 +++- 4 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/AnnotationDialog.js b/src/AnnotationDialog.js index 039275b1b..8923f4a7f 100644 --- a/src/AnnotationDialog.js +++ b/src/AnnotationDialog.js @@ -102,6 +102,7 @@ class AnnotationDialog extends EventEmitter { // Don't re-position if reply textarea is already active const textareaIsActive = textAreaEl.classList.contains(constants.CLASS_ACTIVE); if (textareaIsActive && this.element.parentNode) { + this.scrollToLastComment(); return; } } @@ -126,7 +127,7 @@ class AnnotationDialog extends EventEmitter { * @return {void} */ scrollToLastComment() { - const annotationsEl = this.dialogEl.querySelector(constants.SELECTOR_ANNOTATION_CONTAINER); + const annotationsEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_CONTAINER); if (annotationsEl) { const isDialogFlipped = this.dialogEl.classList.contains(CLASS_FLIPPED_DIALOG); const clientHeight = isDialogFlipped ? 0 : annotationsEl.clientHeight; @@ -753,6 +754,7 @@ class AnnotationDialog extends EventEmitter { // Don't activate if reply textarea is already active const isActive = replyTextEl.classList.contains(constants.CLASS_ACTIVE); + replyTextEl.classList.remove(constants.CLASS_INVALID_INPUT); if (isActive) { return; } diff --git a/src/__tests__/AnnotationDialog-test.js b/src/__tests__/AnnotationDialog-test.js index 130572007..43826ce28 100644 --- a/src/__tests__/AnnotationDialog-test.js +++ b/src/__tests__/AnnotationDialog-test.js @@ -74,6 +74,7 @@ describe('AnnotationDialog', () => { beforeEach(() => { stubs.position = sandbox.stub(dialog, 'position'); stubs.focus = sandbox.stub(dialog, 'focusTextArea'); + stubs.scroll = sandbox.stub(dialog, 'scrollToLastComment'); dialog.canAnnotate = true; }); @@ -82,6 +83,7 @@ describe('AnnotationDialog', () => { dialog.activateReply(); dialog.show(); + expect(stubs.scroll).to.be.called; expect(stubs.position).to.not.be.called; }); @@ -106,6 +108,7 @@ describe('AnnotationDialog', () => { dialog.show(); expect(stubs.position).to.be.called; expect(stubs.focus).to.be.called; + expect(stubs.scroll).to.be.called; }); it('should position the dialog', () => { @@ -117,6 +120,7 @@ describe('AnnotationDialog', () => { dialog.show(); expect(stubs.position).to.be.called; expect(stubs.focus).to.be.called; + expect(stubs.scroll).to.be.called; }); }); @@ -158,7 +162,7 @@ describe('AnnotationDialog', () => { clientHeight: 300, scrollTop: 0 }; - sandbox.stub(dialog.dialogEl, 'querySelector').returns(annotationEl); + sandbox.stub(dialog.element, 'querySelector').returns(annotationEl); dialog.scrollToLastComment(); expect(annotationEl.scrollTop).equals(200); @@ -171,7 +175,7 @@ describe('AnnotationDialog', () => { scrollTop: 0 }; dialog.dialogEl.classList.add('bp-annotation-dialog-flipped'); - sandbox.stub(dialog.dialogEl, 'querySelector').returns(annotationEl); + sandbox.stub(dialog.element, 'querySelector').returns(annotationEl); dialog.scrollToLastComment(); expect(annotationEl.scrollTop).equals(500); diff --git a/src/__tests__/util-test.js b/src/__tests__/util-test.js index 059e635c3..18d8f11dc 100644 --- a/src/__tests__/util-test.js +++ b/src/__tests__/util-test.js @@ -207,27 +207,11 @@ describe('util', () => { resetTextarea(textAreaEl); - assert.ok(!textAreaEl.classList.contains('bp-is-active'), 'Should be inactive'); - assert.equal(textAreaEl.value, 'test', 'Value should NOT be reset'); - assert.equal(textAreaEl.style.width, '', 'Width should be reset'); - assert.equal(textAreaEl.style.height, '', 'Height should be reset'); - }); - - it('should reset text area', () => { - const textAreaEl = document.querySelector('.textarea'); - - // Fake making text area 'active' - textAreaEl.classList.add('bp-is-active'); - textAreaEl.value = 'test'; - textAreaEl.style.width = '10px'; - textAreaEl.style.height = '10px'; - - resetTextarea(textAreaEl, true); - - assert.ok(!textAreaEl.classList.contains('bp-is-active'), 'Should be inactive'); - assert.equal(textAreaEl.value, '', 'Value should be reset'); - assert.equal(textAreaEl.style.width, '', 'Width should be reset'); - assert.equal(textAreaEl.style.height, '', 'Height should be reset'); + expect(textAreaEl).to.not.have.class('bp-is-active'); + expect(textAreaEl).to.not.have.class('bp-invalid-input'); + expect(textAreaEl.value).equals('test'); + expect(textAreaEl.style.width).equals(''); + expect(textAreaEl.style.height).equals(''); }); }); diff --git a/src/util.js b/src/util.js index 190b43187..74ff03092 100644 --- a/src/util.js +++ b/src/util.js @@ -9,7 +9,8 @@ import { CLASS_ACTIVE, CLASS_HIDDEN, CLASS_INVISIBLE, - CLASS_DISABLED + CLASS_DISABLED, + CLASS_INVALID_INPUT } from './constants'; const HEADER_CLIENT_NAME = 'X-Box-Client-Name'; @@ -198,6 +199,7 @@ export function resetTextarea(element, clearText) { textareaEl.style.width = ''; textareaEl.style.height = ''; textareaEl.classList.remove(CLASS_ACTIVE); + textareaEl.classList.remove(CLASS_INVALID_INPUT); if (clearText) { textareaEl.value = '';