Skip to content

Commit

Permalink
New: Mobile text highlight annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
Justin Holdstock committed Jul 19, 2017
1 parent 2a236fe commit 6d9d8c4
Show file tree
Hide file tree
Showing 16 changed files with 429 additions and 71 deletions.
27 changes: 14 additions & 13 deletions src/lib/annotations/AnnotationDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ import { CLASS_ACTIVE, CLASS_HIDDEN } from '../constants';
import { decodeKeydown } from '../util';
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 @@ -34,6 +33,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 @@ -92,8 +92,8 @@ 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);
if (this.highlightDialogEl && !this.element.querySelectorAll(`.${CLASS_COMMENT}`).length) {
this.element.classList.add(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT);

const headerEl = this.element.querySelector(constants.SELECTOR_MOBILE_DIALOG_HEADER);
headerEl.classList.add(CLASS_HIDDEN);
Expand All @@ -102,7 +102,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 @@ -157,14 +157,14 @@ class AnnotationDialog extends EventEmitter {
return;
}

this.element.classList.remove(CLASS_ANIMATE_DIALOG);
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 @@ -173,9 +173,7 @@ class AnnotationDialog extends EventEmitter {
this.unbindDOMListeners();

// Cancel any unsaved annotations
if (!this.hasAnnotations) {
this.cancelAnnotation();
}
this.cancelAnnotation();
}

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

if (this.isMobile) {
this.hideMobileDialog();
}
Expand Down Expand Up @@ -415,8 +417,7 @@ class AnnotationDialog extends EventEmitter {
// Clicking 'Cancel' button to cancel the annotation
case 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 @@ -489,7 +490,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: 2 additions & 0 deletions src/lib/annotations/Annotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
CLASS_ANNOTATION_DIALOG,
CLASS_MOBILE_DIALOG_HEADER,
CLASS_DIALOG_CLOSE,
ID_MOBILE_ANNOTATION_DIALOG,
TYPES
} from './annotationConstants';

Expand Down Expand Up @@ -112,6 +113,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
52 changes: 44 additions & 8 deletions src/lib/annotations/MobileAnnotator.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
$tablet: "(min-width: 768px)";
$three-c: #ccc;

@mixin border-top-bottom {
border-color: $three-c;
border-style: solid;
border-width: 1px 0;
}

.bp-mobile-annotation-dialog {
background: white;
Expand All @@ -9,6 +16,8 @@ $tablet: "(min-width: 768px)";
width: 100%;

&.bp-animate-show-dialog {
// padding: 0;

&:not(.bp-plain-highlight) {
animation: show-dialog-small;
animation-duration: .2s;
Expand All @@ -18,7 +27,7 @@ $tablet: "(min-width: 768px)";
animation: show-dialog-tablet;
animation-duration: .2s;
animation-fill-mode: forwards;
border-left: solid 1px #ccc;
border-left: solid 1px $three-c;
width: 450px;
}
}
Expand All @@ -31,6 +40,14 @@ $tablet: "(min-width: 768px)";
}
}

.bp-create-annotation-dialog.bp-mobile-annotation-dialog {
height: auto;

.bp-annotation-highlight-dialog {
bottom: 0;
}
}

@keyframes show-dialog-small {
0% {
top: 100%;
Expand All @@ -57,32 +74,32 @@ $tablet: "(min-width: 768px)";
}

100% {
top: 1px;
top: 0;
}
}

.bp-mobile-annotation-dialog.bp-annotation-dialog,
.bp-mobile-annotation-dialog.bp-temp-annotation-dialog {
.bp-mobile-annotation-dialog.bp-annotation-dialog {
.annotation-container {
background-color: $white;
border: 0;
border-radius: 4px;
height: 100%;
overflow-x: hidden;
overflow-y: auto;
padding: 15px 15px 60px;
padding: 15px;
position: absolute;
width: 100%;
}

.bp-annotation-mobile-header {
align-items: center;
background-color: $white;
border-bottom: 1px solid #ccc;
display: flex;
height: 48px;
justify-content: space-between;
padding: 0;

@include border-top-bottom;
}

.bp-annotation-dialog-close {
Expand Down Expand Up @@ -153,9 +170,23 @@ $tablet: "(min-width: 768px)";

/* Highlight dialog */
.bp-mobile-annotation-dialog.bp-plain-highlight {
border-bottom: 1px solid #ccc;
height: 47px; // includes mobile header & highlight dialog
top: auto;

@include border-top-bottom;
}

.bp-mobile-annotation-dialog .bp-annotation-highlight-btns,
.bp-mobile-annotation-dialog .bp-create-highlight-comment,
.bp-mobile-annotation-dialog .annotation-container section[data-section="create"] {
background: white;
bottom: 0;
left: 0;
padding: 15px;
position: fixed;
width: 100%;

@include border-top-bottom;
}

.bp-mobile-annotation-dialog .bp-annotation-highlight-dialog {
Expand All @@ -169,8 +200,13 @@ $tablet: "(min-width: 768px)";
text-align: center;
z-index: 9999;

.bp-annotations-highlight-btns button {
.bp-annotation-highlight-btns button {
float: left;
width: 50%;

&:first-child {
border-right: 1px solid $three-c;
}
}

&.cannot-annotate {
Expand Down
11 changes: 5 additions & 6 deletions src/lib/annotations/__tests__/AnnotationDialog-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as annotatorUtil from '../annotatorUtil';
import * as constants from '../annotationConstants';
import { CLASS_ACTIVE, CLASS_HIDDEN } from '../../constants';

const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight';
const CLASS_CANCEL_DELETE = 'cancel-delete-btn';
const CLASS_CANNOT_ANNOTATE = 'cannot-annotate';
const CLASS_REPLY_TEXTAREA = 'reply-textarea';
Expand Down Expand Up @@ -166,15 +165,15 @@ describe('lib/annotations/AnnotationDialog', () => {
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.true;
});

it('should hide the mobile header if a plain highlight', () => {
it('should reset the annotation dialog to be a plain highlight if no comments are present', () => {
dialog.isMobile = true;
dialog.highlightDialogEl = {};
dialog.hasComments = false;
sandbox.stub(dialog.element, 'querySelectorAll').withArgs('.annotation-comment').returns([]);
stubs.show = sandbox.stub(annotatorUtil, 'showElement');
stubs.bind = sandbox.stub(dialog, 'bindDOMListeners');

dialog.show();
expect(dialog.element).to.have.class(CLASS_ANNOTATION_PLAIN_HIGHLIGHT);

expect(dialog.element.classList.contains(constants.CLASS_ANNOTATION_PLAIN_HIGHLIGHT)).to.be.true;
});
});

Expand All @@ -196,7 +195,7 @@ describe('lib/annotations/AnnotationDialog', () => {
dialog.hideMobileDialog();
expect(stubs.hide).to.be.called;
expect(stubs.unbind).to.be.called;
expect(stubs.cancel).to.not.be.called;
expect(stubs.cancel).to.be.called;
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false;
});

Expand Down
2 changes: 1 addition & 1 deletion src/lib/annotations/__tests__/AnnotationThread-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ describe('lib/annotations/AnnotationThread', () => {
expect(thread.destroy).to.be.called;

// 'pending-active' state
thread.state = STATES.pending_ACTIVE;
thread.state = STATES.pending_active;
thread.cancelUnsavedAnnotation();
expect(thread.destroy).to.be.called;
});
Expand Down
4 changes: 4 additions & 0 deletions src/lib/annotations/annotationConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ export const CLASS_ANNOTATION_BUTTON_CANCEL = 'cancel-annotation-btn';
export const CLASS_ANNOTATION_BUTTON_POST = 'post-annotation-btn';
export const CLASS_ANNOTATION_DIALOG = 'bp-annotation-dialog';
export const CLASS_ANNOTATION_HIGHLIGHT_DIALOG = 'bp-annotation-highlight-dialog';
export const CLASS_ANNOTATION_PLAIN_HIGHLIGHT = 'bp-plain-highlight';
export const CLASS_ANNOTATION_POINT_BUTTON = 'bp-point-annotation-btn';
export const CLASS_ANNOTATION_POINT_MODE = 'bp-point-annotation-mode';
export const CLASS_ANNOTATION_CARET = 'bp-annotation-caret';
export const CLASS_ANNOTATION_TEXTAREA = 'annotation-textarea';
export const CLASS_BUTTON_CONTAINER = 'button-container';
export const CLASS_ANNOTATION_CONTAINER = 'annotation-container';
export const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';
export const CLASS_MOBILE_ANNOTATION_DIALOG = 'bp-mobile-annotation-dialog';
export const CLASS_MOBILE_DIALOG_HEADER = 'bp-annotation-mobile-header';
export const CLASS_DIALOG_CLOSE = 'bp-annotation-dialog-close';
Expand Down Expand Up @@ -61,3 +63,5 @@ export const HIGHLIGHT_FILL = {

export const PAGE_PADDING_TOP = 15;
export const PAGE_PADDING_BOTTOM = 15;

export const ID_MOBILE_ANNOTATION_DIALOG = 'mobile-annotation-dialog';
16 changes: 8 additions & 8 deletions src/lib/annotations/annotatorUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ export function findClosestElWithClass(element, className) {
}

/**
* Returns the page element and page number that the element is on.
*
* @param {HTMLElement} element - Element to find page and page number for
* @return {Object} Page element/page number if found or null/-1 if not
*/
* Returns the page element and page number that the element is on.
*
* @param {HTMLElement} element - Element to find page and page number for
* @return {Object} Page element/page number if found or null/-1 if not
*/
export function getPageInfo(element) {
const pageEl = findClosestElWithClass(element, 'page') || null;
let page = -1;
Expand Down Expand Up @@ -125,7 +125,7 @@ export function showInvisibleElement(elementOrSelector) {

/**
* Hides the specified element or element with specified selector. The element
* will still take up DOM space but not be visible in the UI
* will still take up DOM space but not be visible in the UI.
*
* @param {HTMLElement|string} elementOrSelector - Element or CSS selector
* @return {void}
Expand Down Expand Up @@ -167,8 +167,8 @@ export function resetTextarea(element, clearText) {
/**
* Checks whether element is fully in viewport.
*
* @param {HTMLElement} element - Element to check
* @return {boolean} Whether element is fully in viewport
* @param {HTMLElement} element - The element to check and see if it lies in the viewport
* @return {boolean} Whether the element is fully in viewport
*/
export function isElementInViewport(element) {
const dimensions = element.getBoundingClientRect();
Expand Down
Loading

0 comments on commit 6d9d8c4

Please sign in to comment.