Fix: Scroll to bottom of mobile dialogs (#70)

- Validate text areas when you cancel an annotations reply
box · Dec 12, 2017 · 7a33849 · 7a33849
1 parent e5a1749
commit 7a33849
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 25 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 = '';