Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Plain and Comment highlight annotations on mobile #276

Merged
merged 8 commits into from
Aug 14, 2017
Merged

New: Plain and Comment highlight annotations on mobile #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a60feab
Squashed commit of the following:
Aug 4, 2017
19ad58e
Chore: Moved point specific location lookup and added guard
Aug 4, 2017
fd2ff79
Chore: @SPramod PR feedback
Aug 4, 2017
f7baa6a
Chore: Using thread.reset() to remove code dup
Aug 8, 2017
238aa32
Merge branch 'master' of github.com:JustinHoldstock/box-content-previ…
Aug 8, 2017
b235c00
Chore: constant refs
Aug 8, 2017
5a10951
Chore: tests
Aug 11, 2017
c00162c
Merge branch 'master' of github.com:JustinHoldstock/box-content-previ…
Aug 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions src/lib/annotations/AnnotationDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ import * as annotatorUtil from './annotatorUtil';
import * as constants from './annotationConstants';
import { ICON_CLOSE, ICON_DELETE } from '../icons/icons';

const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight';
const CLASS_BUTTON_DELETE_COMMENT = 'delete-comment-btn';
const CLASS_CANCEL_DELETE = 'cancel-delete-btn';
const CLASS_CANNOT_ANNOTATE = 'cannot-annotate';
const CLASS_COMMENT = 'annotation-comment';
const CLASS_COMMENTS_CONTAINER = 'annotation-comments';
const CLASS_REPLY_CONTAINER = 'reply-container';
const CLASS_REPLY_TEXTAREA = 'reply-textarea';
const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
const CLASS_DELETE_CONFIRMATION = 'delete-confirmation';
const CLASS_BUTTON_DELETE_CONFIRM = 'confirm-delete-btn';

Expand All @@ -23,6 +22,7 @@ class AnnotationDialog extends EventEmitter {

/**
* The data object for constructing a dialog.
*
* @typedef {Object} AnnotationDialogData
* @property {HTMLElement} annotatedElement HTML element being annotated on
* @property {Annotation[]} annotations Annotations in dialog, can be an
Expand Down Expand Up @@ -81,8 +81,9 @@ class AnnotationDialog extends EventEmitter {
annotatorUtil.showElement(this.element);
this.element.appendChild(this.dialogEl);

if (this.highlightDialogEl && !this.hasComments) {
this.element.classList.add(CLASS_ANNOTATION_PLAIN_HIGHLIGHT);
const commentEls = this.element.querySelectorAll(`.${CLASS_COMMENT}`);
if (this.highlightDialogEl && !commentEls.length) {
this.element.classList.add(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT);

const headerEl = this.element.querySelector(constants.SELECTOR_MOBILE_DIALOG_HEADER);
headerEl.classList.add(constants.CLASS_HIDDEN);
Expand All @@ -91,7 +92,7 @@ class AnnotationDialog extends EventEmitter {
const dialogCloseButtonEl = this.element.querySelector(constants.SELECTOR_DIALOG_CLOSE);
dialogCloseButtonEl.addEventListener('click', this.hideMobileDialog);

this.element.classList.add(CLASS_ANIMATE_DIALOG);
this.element.classList.add(constants.CLASS_ANIMATE_DIALOG);

this.bindDOMListeners();
}
Expand Down Expand Up @@ -146,14 +147,18 @@ class AnnotationDialog extends EventEmitter {
return;
}

this.element.classList.remove(CLASS_ANIMATE_DIALOG);
if (this.dialogEl && this.dialogEl.parentNode) {
this.dialogEl.parentNode.removeChild(this.dialogEl);
}

this.element.classList.remove(constants.CLASS_ANIMATE_DIALOG);

// Clear annotations from dialog
this.element.innerHTML = `
<div class="${constants.CLASS_MOBILE_DIALOG_HEADER}">
<button class="${constants.CLASS_DIALOG_CLOSE}">${ICON_CLOSE}</button>
</div>`.trim();
this.element.classList.remove(CLASS_ANNOTATION_PLAIN_HIGHLIGHT);
this.element.classList.remove(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT);

const dialogCloseButtonEl = this.element.querySelector(constants.SELECTOR_DIALOG_CLOSE);
dialogCloseButtonEl.removeEventListener('click', this.hideMobileDialog);
Expand All @@ -162,9 +167,7 @@ class AnnotationDialog extends EventEmitter {
this.unbindDOMListeners();

// Cancel any unsaved annotations
if (!this.hasAnnotations) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check that this is ok to move on all other annotation types. I feel like there was a reason I added that check just forgot to add a comment explaining why

this.cancelAnnotation();
}
this.cancelAnnotation();
}

/**
Expand All @@ -173,6 +176,10 @@ class AnnotationDialog extends EventEmitter {
* @return {void}
*/
hide() {
if (this.element && this.element.classList.contains(constants.CLASS_HIDDEN)) {
return;
}

if (this.isMobile) {
this.hideMobileDialog();
}
Expand Down Expand Up @@ -404,8 +411,7 @@ class AnnotationDialog extends EventEmitter {
// Clicking 'Cancel' button to cancel the annotation
case constants.DATA_TYPE_CANCEL:
if (this.isMobile) {
// Hide mobile dialog without destroying the thread
this.hideMobileDialog();
this.hide();
} else {
// Cancels + destroys the annotation thread
this.cancelAnnotation();
Expand Down Expand Up @@ -478,7 +484,7 @@ class AnnotationDialog extends EventEmitter {
const text = annotatorUtil.htmlEscape(annotation.text);

const annotationEl = document.createElement('div');
annotationEl.classList.add('annotation-comment');
annotationEl.classList.add(CLASS_COMMENT);
annotationEl.setAttribute('data-annotation-id', annotation.annotationID);
annotationEl.innerHTML = `
<div class="profile-image-container">${avatarHtml}</div>
Expand Down
2 changes: 1 addition & 1 deletion src/lib/annotations/AnnotationThread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ class AnnotationThread extends EventEmitter {
* @return {void}
*/
cancelUnsavedAnnotation() {
if (!this.isMobile && !annotatorUtil.isPending(this.state)) {
if (!annotatorUtil.isPending(this.state)) {
return;
}
this.destroy();
Expand Down
3 changes: 3 additions & 0 deletions src/lib/annotations/Annotator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
CLASS_ANNOTATION_MODE,
CLASS_MOBILE_DIALOG_HEADER,
CLASS_DIALOG_CLOSE,
ID_MOBILE_ANNOTATION_DIALOG,
SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL,
SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER,
SELECTOR_ANNOTATION_BUTTON_DRAW_POST,
Expand Down Expand Up @@ -56,6 +57,7 @@ class Annotator extends EventEmitter {
this.locale = data.locale;
this.validationErrorEmitted = false;
this.isMobile = data.isMobile;
this.hasTouch = data.hasTouch;
this.previewUI = data.previewUI;
this.modeButtons = data.modeButtons;
this.annotationModeHandlers = [];
Expand Down Expand Up @@ -195,6 +197,7 @@ class Annotator extends EventEmitter {
mobileDialogEl.classList.add(CLASS_MOBILE_ANNOTATION_DIALOG);
mobileDialogEl.classList.add(CLASS_ANNOTATION_DIALOG);
mobileDialogEl.classList.add(CLASS_HIDDEN);
mobileDialogEl.id = ID_MOBILE_ANNOTATION_DIALOG;

mobileDialogEl.innerHTML = `
<div class="${CLASS_MOBILE_DIALOG_HEADER}">
Expand Down
61 changes: 48 additions & 13 deletions src/lib/annotations/CommentBox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ class CommentBox extends EventEmitter {
*/
parentEl;

/** Whether or not we should use touch events */
hasTouch;

/* Events that the comment box can emit. */
static CommentEvents = {
cancel: 'comment_cancel',
Expand All @@ -87,6 +90,7 @@ class CommentBox extends EventEmitter {
this.cancelText = config.cancel || this.cancelText;
this.postText = config.post || this.postText;
this.placeholderText = config.placeholder || this.placeholderText;
this.hasTouch = config.hasTouch;

// Explicit scope binding for event listeners
this.onCancel = this.onCancel.bind(this);
Expand All @@ -99,11 +103,20 @@ class CommentBox extends EventEmitter {
* @return {void}
*/
focus() {
if (!this.containerEl) {
return;
if (this.textAreaEl) {
this.textAreaEl.focus();
}
}

this.textAreaEl.focus();
/**
* Unfocus the text box.
*
* @return {void}
*/
blur() {
if (document.activeElement) {
document.activeElement.blur();
}
}

/**
Expand All @@ -112,11 +125,9 @@ class CommentBox extends EventEmitter {
* @return {void}
*/
clear() {
if (!this.containerEl) {
return;
if (this.textAreaEl) {
this.textAreaEl.value = '';
}

this.textAreaEl.value = '';
}

/**
Expand All @@ -125,11 +136,9 @@ class CommentBox extends EventEmitter {
* @return {void}
*/
hide() {
if (!this.containerEl) {
return;
if (this.containerEl) {
hideElement(this.containerEl);
}

hideElement(this.containerEl);
}

/**
Expand Down Expand Up @@ -161,6 +170,10 @@ class CommentBox extends EventEmitter {
this.containerEl = null;
this.cancelEl.removeEventListener('click', this.onCancel);
this.postEl.removeEventListener('click', this.onPost);
if (this.hasTouch) {
this.cancelEl.removeEventListener('touchstart', this.onCancel);
this.postEl.removeEventListener('touchstart', this.onPost);
}
}

//--------------------------------------------------------------------------
Expand Down Expand Up @@ -191,13 +204,27 @@ class CommentBox extends EventEmitter {
return containerEl;
}

/**
* Stop default behaviour of an element.
*
* @param {Event} event Event created by an input event.
* @return {void}
*/
preventDefaultAndPropagation(event) {
event.preventDefault();
event.stopPropagation();
}

/**
* Clear the current text in the textarea element and notify listeners.
*
* @private
* @param {Event} event Event created by input event
* @return {void}
*/
onCancel() {
onCancel(event) {
// stops touch propogating to a click event
this.preventDefaultAndPropagation(event);
this.clear();
this.emit(CommentBox.CommentEvents.cancel);
}
Expand All @@ -206,9 +233,12 @@ class CommentBox extends EventEmitter {
* Notify listeners of submit event and then clear textarea element.
*
* @private
* @param {Event} event Event created by input event
* @return {void}
*/
onPost() {
onPost(event) {
// stops touch propogating to a click event
this.preventDefaultAndPropagation(event);
this.emit(CommentBox.CommentEvents.post, this.textAreaEl.value);
this.clear();
}
Expand All @@ -231,6 +261,11 @@ class CommentBox extends EventEmitter {
// Add event listeners
this.cancelEl.addEventListener('click', this.onCancel);
this.postEl.addEventListener('click', this.onPost);
if (this.hasTouch) {
containerEl.addEventListener('touchend', this.preventDefaultAndPropagation.bind(this));
this.cancelEl.addEventListener('touchend', this.onCancel);
this.postEl.addEventListener('touchend', this.onPost);
}

return containerEl;
}
Expand Down
Loading