Skip to content

Commit

Permalink
Chore: PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Justin Holdstock committed Jul 25, 2017
1 parent 3e7c3a7 commit 242ad81
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 41 deletions.
49 changes: 9 additions & 40 deletions src/lib/annotations/doc/CreateHighlightDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,63 +38,31 @@ export const CreateEvents = {
};

class CreateHighlightDialog extends EventEmitter {
/**
* Container element for the dialog.
*
* @property {HTMLElement}
*/
/** @property {HTMLElement} - Container element for the dialog. */
containerEl;

/**
* The clickable element for creating plain highlights.
*
* @property {HTMLElement}
*/
/** @property {HTMLElement} - The clickable element for creating plain highlights. */
highlightCreateEl;

/**
* The clickable element got creating comment highlights.
*
* @property {HTMLElement}
*/
/** @property {HTMLElement} - The clickable element got creating comment highlights. */
commentCreateEl;

/**
* The parent container to nest the dialog element in.
*
* @property {HTMLElement}
*/
/** @property {HTMLElement} - The parent container to nest the dialog element in. */
parentEl;

/**
* The element containing the buttons that can creaet highlights.
*
* @property {HTMLElement}
*/
/** @property {HTMLElement} - The element containing the buttons that can creaet highlights. */
buttonsEl;

/**
* The comment box instance. Contains area for text input and post/cancel buttons.
*
* @property {CommentBox}
*/
/** @property {CommentBox} - The comment box instance. Contains area for text input and post/cancel buttons. */
commentBox;

/**
* Position, on the DOM, to align the dialog to the end of a highlight.
*
* @property {Object}
*/
/** @property {Object} - Position, on the DOM, to align the dialog to the end of a highlight. */
position = {
x: 0,
y: 0
};

/**
* Whether or not we're on a mobile device.
*
* @property {boolean}
*/
/** @property {boolean} - Whether or not we're on a mobile device. */
isMobile;

/**
Expand Down Expand Up @@ -323,6 +291,7 @@ class CreateHighlightDialog extends EventEmitter {
const highlightDialogEl = document.createElement('div');
highlightDialogEl.classList.add(CLASS_CREATE_DIALOG);
highlightDialogEl.innerHTML = CREATE_HIGHLIGHT_DIALOG_TEMPLATE;

// Get rid of the caret
if (this.isMobile) {
highlightDialogEl.classList.add('bp-mobile-annotation-dialog');
Expand Down
5 changes: 4 additions & 1 deletion src/lib/annotations/doc/DocAnnotator.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class DocAnnotator extends Annotator {
this.highlightCreateHandler = this.highlightCreateHandler.bind(this);
this.onTouchStart = this.onTouchStart.bind(this);

this.createHighlightDialog = new CreateHighlightDialog(undefined, this.isMobile);
this.createHighlightDialog = new CreateHighlightDialog(this.container, this.isMobile);
this.createHighlightDialog.addListener(CreateEvents.plain, this.createPlainHighlight);

this.createHighlightDialog.addListener(CreateEvents.comment, this.highlightCurrentSelection);
Expand Down Expand Up @@ -674,6 +674,7 @@ class DocAnnotator extends Annotator {
if (this.highlighter) {
this.highlighter.removeAllHighlights();
}

this.createHighlightDialog.hide();
// Creating highlights is disabled on mobile for now since the
// event we would listen to, selectionchange, fires continuously and
Expand All @@ -684,6 +685,7 @@ class DocAnnotator extends Annotator {
} else {
this.highlightClickHandler(event);
}

this.isCreatingHighlight = false;
}

Expand All @@ -696,6 +698,7 @@ class DocAnnotator extends Annotator {
if (this.highlighter) {
this.highlighter.removeAllHighlights();
}

this.createHighlightDialog.hide();
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/annotations/doc/DocHighlightDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class DocHighlightDialog extends AnnotationDialog {
if (text.trim() === '') {
return;
}

// Convert from plain highlight to comment
const headerEl = this.isMobile ? this.element.querySelector('.bp-annotation-mobile-header') : null;
if (headerEl) {
Expand Down

0 comments on commit 242ad81

Please sign in to comment.