Skip to content

Commit

Permalink
Fix: Validation message fails to get displayed when user clicks on 'P…
Browse files Browse the repository at this point in the history 
…ost' without entering any comment while adding a Point Annotation. (#27)

* Chore: Indicate invalid input on annotation post
* Update: Add validation to CommentBox.js as well
* Update: consolidating focus methods
  • Loading branch information
@pramodsum
pramodsum committed Nov 14, 2017
1 parent 07fc4c1 commit d90e72c
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 10 deletions.
40 changes: 40 additions & 0 deletions src/AnnotationDialog.js
View file
Expand Up @@ -55,6 +55,8 @@ class AnnotationDialog extends EventEmitter {
this.canAnnotate = data.canAnnotate;
this.locale = data.locale;
this.isMobile = data.isMobile;

this.validateTextArea = this.validateTextArea.bind(this);
}

/**
Expand Down Expand Up @@ -240,6 +242,7 @@ class AnnotationDialog extends EventEmitter {
const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
const text = textInput || annotationTextEl.value;
if (text.trim() === '') {
annotationTextEl.classList.add(constants.CLASS_INVALID_INPUT);
return;
}

Expand Down Expand Up @@ -330,12 +333,38 @@ class AnnotationDialog extends EventEmitter {
this.element.addEventListener('mouseup', this.stopPropagation);
this.element.addEventListener('wheel', this.stopPropagation);

const replyTextEl = this.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);
if (replyTextEl) {
replyTextEl.addEventListener('focus', this.validateTextArea);
}

const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
if (annotationTextEl) {
annotationTextEl.addEventListener('focus', this.validateTextArea);
}

if (!this.isMobile) {
this.element.addEventListener('mouseenter', this.mouseenterHandler);
this.element.addEventListener('mouseleave', this.mouseleaveHandler);
}
}

/**
* Removes red border around textarea on focus
*
* @protected
* @param {Event} event Keyboard event
* @return {void}
*/
validateTextArea(event) {
const textEl = event.target;
if (textEl.type !== 'textarea' || textEl.value.trim() === '') {
return;
}

textEl.classList.remove(constants.CLASS_INVALID_INPUT);
}

/**
* Unbinds DOM event listeners.
*
Expand All @@ -348,6 +377,16 @@ class AnnotationDialog extends EventEmitter {
this.element.removeEventListener('mouseup', this.stopPropagation);
this.element.removeEventListener('wheel', this.stopPropagation);

const replyTextEl = this.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);
if (replyTextEl) {
replyTextEl.removeEventListener('focus', this.validateTextArea);
}

const annotationTextEl = this.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
if (annotationTextEl) {
annotationTextEl.removeEventListener('focus', this.validateTextArea);
}

if (!this.isMobile) {
this.element.removeEventListener('mouseenter', this.mouseenterHandler);
this.element.removeEventListener('mouseleave', this.mouseleaveHandler);
Expand Down Expand Up @@ -626,6 +665,7 @@ class AnnotationDialog extends EventEmitter {
const replyTextEl = this.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);
const text = replyTextEl.value;
if (text.trim() === '') {
replyTextEl.classList.add(constants.CLASS_INVALID_INPUT);
return;
}

Expand Down
4 changes: 4 additions & 0 deletions src/Annotator.scss
View file
Expand Up @@ -204,6 +204,10 @@ $tablet: 'max-width: 768px';
&.bp-is-active {
min-height: 68px;
}

&.bp-invalid-input {
border-color: fade-out($great-balls-of-fire, .5);
}
}

.annotation-comment {
Expand Down
28 changes: 24 additions & 4 deletions src/CommentBox.js
View file
Expand Up @@ -86,6 +86,7 @@ class CommentBox extends EventEmitter {
this.placeholderText = config.localized.addCommentPlaceholder;

// Explicit scope binding for event listeners
this.focus = this.focus.bind(this);
this.onCancel = this.onCancel.bind(this);
this.onPost = this.onPost.bind(this);
}
Expand All @@ -98,6 +99,7 @@ class CommentBox extends EventEmitter {
focus() {
if (this.textAreaEl) {
this.textAreaEl.focus();
this.textAreaEl.classList.remove(constants.CLASS_INVALID_INPUT);
}
}

Expand Down Expand Up @@ -161,12 +163,21 @@ class CommentBox extends EventEmitter {
this.containerEl.remove();
this.parentEl = null;
this.containerEl = null;
this.cancelEl.removeEventListener('click', this.onCancel);
this.postEl.removeEventListener('click', this.onPost);
if (this.hasTouch) {

if (this.cancelEl) {
this.cancelEl.removeEventListener('click', this.onCancel);
this.cancelEl.removeEventListener('touchstart', this.onCancel);
}

if (this.postEl) {
this.postEl.removeEventListener('click', this.onPost);
this.postEl.removeEventListener('touchstart', this.onPost);
}

if (this.textAreaEl) {
this.textAreaEl.removeEventListener('focus', this.focus);
this.textAreaEl.removeEventListener('keydown', this.focus);
}
}

//--------------------------------------------------------------------------
Expand Down Expand Up @@ -232,7 +243,14 @@ class CommentBox extends EventEmitter {
onPost(event) {
// stops touch propogating to a click event
this.preventDefaultAndPropagation(event);
this.emit(CommentBox.CommentEvents.post, this.textAreaEl.value);

const text = this.textAreaEl.value;
if (text.trim() === '') {
this.textAreaEl.classList.add(constants.CLASS_INVALID_INPUT);
return;
}

this.emit(CommentBox.CommentEvents.post, text);
this.clear();
}

Expand All @@ -252,9 +270,11 @@ class CommentBox extends EventEmitter {
this.postEl = containerEl.querySelector(constants.SELECTOR_ANNOTATION_BUTTON_POST);

// Add event listeners
this.textAreaEl.addEventListener('focus', this.focus);
this.cancelEl.addEventListener('click', this.onCancel);
this.postEl.addEventListener('click', this.onPost);
if (this.hasTouch) {
this.textAreaEl.addEventListener('keydown', this.focus);
containerEl.addEventListener('touchend', this.preventDefaultAndPropagation.bind(this));
this.cancelEl.addEventListener('touchend', this.onCancel);
this.postEl.addEventListener('touchend', this.onPost);
Expand Down
44 changes: 43 additions & 1 deletion src/__tests__/AnnotationDialog-test.js
View file
Expand Up @@ -13,6 +13,7 @@ const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
const CLASS_BUTTON_DELETE_COMMENT = 'delete-comment-btn';
const CLASS_COMMENTS_CONTAINER = 'annotation-comments';
const SELECTOR_DELETE_CONFIRMATION = '.delete-confirmation';
const CLASS_INVALID_INPUT = 'bp-invalid-input';

let dialog;
const sandbox = sinon.sandbox.create();
Expand Down Expand Up @@ -395,6 +396,10 @@ describe('AnnotationDialog', () => {
describe('bindDOMListeners()', () => {
it('should bind DOM listeners', () => {
stubs.add = sandbox.stub(dialog.element, 'addEventListener');
const replyTextEl = dialog.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);
sandbox.stub(replyTextEl, 'addEventListener');
const annotationTextEl = dialog.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
sandbox.stub(annotationTextEl, 'addEventListener');

dialog.bindDOMListeners();
expect(stubs.add).to.be.calledWith('keydown', sinon.match.func);
Expand All @@ -403,6 +408,8 @@ describe('AnnotationDialog', () => {
expect(stubs.add).to.be.calledWith('mouseenter', sinon.match.func);
expect(stubs.add).to.be.calledWith('mouseleave', sinon.match.func);
expect(stubs.add).to.be.calledWith('wheel', sinon.match.func);
expect(replyTextEl.addEventListener).to.be.calledWith('focus', sinon.match.func);
expect(annotationTextEl.addEventListener).to.be.calledWith('focus', sinon.match.func);
});

it('should not bind mouseenter/leave events for mobile browsers', () => {
Expand All @@ -416,12 +423,41 @@ describe('AnnotationDialog', () => {
expect(stubs.add).to.not.be.calledWith('mouseenter', sinon.match.func);
expect(stubs.add).to.not.be.calledWith('mouseleave', sinon.match.func);
expect(stubs.add).to.be.calledWith('wheel', sinon.match.func);
dialog.element = null;
});
});

describe('validateTextArea()', () => {
it('should do nothing if keyboard event was not in a textarea', () => {
stubs.textEl = document.createElement('div');
stubs.textEl.classList.add(constants.CLASS_INVALID_INPUT);
dialog.validateTextArea({ target: stubs.textEl });
expect(stubs.textEl).to.have.class(constants.CLASS_INVALID_INPUT);
});

it('should do nothing if textarea is blank', () => {
stubs.textEl = document.createElement('textarea');
stubs.textEl.classList.add(constants.CLASS_INVALID_INPUT);
dialog.validateTextArea({ target: stubs.textEl });
expect(stubs.textEl).to.have.class(constants.CLASS_INVALID_INPUT);
});

it('should remove red border around textarea', () => {
stubs.textEl = document.createElement('textarea');
stubs.textEl.classList.add(constants.CLASS_INVALID_INPUT);
stubs.textEl.value = 'words';
dialog.validateTextArea({ target: stubs.textEl });
expect(stubs.textEl).to.not.have.class(constants.CLASS_INVALID_INPUT);
});
});

describe('unbindDOMListeners()', () => {
it('should unbind DOM listeners', () => {
stubs.remove = sandbox.stub(dialog.element, 'removeEventListener');
const replyTextEl = dialog.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);
sandbox.stub(replyTextEl, 'removeEventListener');
const annotationTextEl = dialog.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
sandbox.stub(annotationTextEl, 'removeEventListener');

dialog.unbindDOMListeners();
expect(stubs.remove).to.be.calledWith('keydown', sinon.match.func);
Expand All @@ -430,6 +466,8 @@ describe('AnnotationDialog', () => {
expect(stubs.remove).to.be.calledWith('mouseenter', sinon.match.func);
expect(stubs.remove).to.be.calledWith('mouseleave', sinon.match.func);
expect(stubs.remove).to.be.calledWith('wheel', sinon.match.func);
expect(replyTextEl.removeEventListener).to.be.calledWith('focus', dialog.validateTextArea);
expect(annotationTextEl.removeEventListener).to.be.calledWith('focus', dialog.validateTextArea);
});

it('should not bind mouseenter/leave events for mobile browsers', () => {
Expand Down Expand Up @@ -750,10 +788,12 @@ describe('AnnotationDialog', () => {
});
});

describe('_postAannotation()', () => {
describe('postAnnotation()', () => {
it('should not post an annotation to the dialog if it has no text', () => {
const annotationTextEl = dialog.element.querySelector(constants.SELECTOR_ANNOTATION_TEXTAREA);
dialog.postAnnotation();
expect(stubs.emit).to.not.be.called;
expect(annotationTextEl).to.have.class(CLASS_INVALID_INPUT);
});

it('should post an annotation to the dialog if it has text', () => {
Expand Down Expand Up @@ -851,9 +891,11 @@ describe('AnnotationDialog', () => {
})
);
dialog.activateReply();
const replyTextEl = dialog.element.querySelector(`.${CLASS_REPLY_TEXTAREA}`);

dialog.postReply();
expect(stubs.emit).to.not.be.called;
expect(replyTextEl).to.have.class(CLASS_INVALID_INPUT);
});

it('should post a reply to the dialog if it has text', () => {
Expand Down
44 changes: 39 additions & 5 deletions src/__tests__/CommentBox-test.js
View file
Expand Up @@ -3,7 +3,8 @@ import CommentBox from '../CommentBox';
import {
CLASS_HIDDEN,
SELECTOR_ANNOTATION_BUTTON_CANCEL,
SELECTOR_ANNOTATION_BUTTON_POST
SELECTOR_ANNOTATION_BUTTON_POST,
CLASS_INVALID_INPUT
} from '../annotationConstants';

describe('CommentBox', () => {
Expand Down Expand Up @@ -172,6 +173,13 @@ describe('CommentBox', () => {
commentBox.destroy();
expect(remove).to.be.called;
});

it('should remove event listener from text area', () => {
commentBox.show();
const remove = sandbox.stub(commentBox.textAreaEl, 'removeEventListener');
commentBox.destroy();
expect(remove).to.be.called;
});
});

describe('createHTML()', () => {
Expand Down Expand Up @@ -218,15 +226,22 @@ describe('CommentBox', () => {

describe('onPost()', () => {
beforeEach(() => {
commentBox.textAreaEl = document.createElement('textarea');
sandbox.stub(commentBox, 'preventDefaultAndPropagation');
});

it('should invalidate textarea and do nothing if textarea is blank', () => {
const emit = sandbox.stub(commentBox, 'emit');
const text = '';
commentBox.onPost({ preventDefault: () => {} });
expect(emit).to.not.be.calledWith(CommentBox.CommentEvents.post, text);
expect(commentBox.textAreaEl).to.have.class(CLASS_INVALID_INPUT);
});

it('should emit a post event with the value of the text box', () => {
const emit = sandbox.stub(commentBox, 'emit');
const text = 'a comment';
commentBox.textAreaEl = {
value: text
};
commentBox.textAreaEl.value = text;
commentBox.onPost({ preventDefault: () => {} });
expect(emit).to.be.calledWith(CommentBox.CommentEvents.post, text);
});
Expand Down Expand Up @@ -259,17 +274,36 @@ describe('CommentBox', () => {
expect(commentBox.postEl).to.exist;
});

it('should add an event listener on the cancel and post buttons', () => {
it('should add an event listener on the textarea, cancel and post buttons', () => {
const uiElement = {
addEventListener: sandbox.stub()
};
const el = document.createElement('section');
sandbox.stub(el, 'querySelector').returns(uiElement);
sandbox.stub(commentBox, 'createHTML').returns(el);

commentBox.createCommentBox();
expect(uiElement.addEventListener).to.be.calledWith('click', commentBox.onCancel);
expect(uiElement.addEventListener).to.be.calledWith('click', commentBox.onPost);
expect(uiElement.addEventListener).to.be.calledWith('focus', commentBox.focus);
});

it('should add an event listener on the textarea, cancel and post buttons if the user is on a touch-enabled mobile device', () => {
const uiElement = {
addEventListener: sandbox.stub()
};
const el = document.createElement('section');
sandbox.stub(el, 'querySelector').returns(uiElement);
sandbox.stub(commentBox, 'createHTML').returns(el);
commentBox.hasTouch = true;

commentBox.createCommentBox();
expect(uiElement.addEventListener).to.be.calledWith('click', commentBox.onCancel);
expect(uiElement.addEventListener).to.be.calledWith('click', commentBox.onPost);
expect(uiElement.addEventListener).to.be.calledWith('focus', commentBox.focus);
expect(uiElement.addEventListener).to.be.calledWith('keydown', commentBox.focus);
expect(uiElement.addEventListener).to.be.calledWith('touchend', commentBox.onCancel);
expect(uiElement.addEventListener).to.be.calledWith('touchend', commentBox.onPost);
});
});
});
1 change: 1 addition & 0 deletions src/annotationConstants.js
View file
Expand Up @@ -45,6 +45,7 @@ export const CLASS_ANNOTATION_DRAWING_HEADER = 'bp-annotate-draw-header';
export const CLASS_ANNNOTATION_DRAWING_BACKGROUND = 'bp-annotate-draw-background';
export const CLASS_ADD_DRAWING_BTN = 'bp-btn-annotate-draw-add';
export const CLASS_DELETE_DRAWING_BTN = 'bp-btn-annotate-draw-delete';
export const CLASS_INVALID_INPUT = 'bp-invalid-input';

export const DATA_TYPE_ANNOTATION_DIALOG = 'annotation-dialog';
export const DATA_TYPE_ANNOTATION_INDICATOR = 'annotation-indicator';
Expand Down

0 comments on commit d90e72c

Please sign in to comment.