Skip to content

Commit

Permalink
Chore: Cleanup flow types (#274)
Browse files Browse the repository at this point in the history
  • Loading branch information
pramodsum authored Oct 31, 2018
1 parent d853b80 commit 33adb47
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 19 deletions.
7 changes: 4 additions & 3 deletions src/AnnotationThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class AnnotationThread extends EventEmitter {
fileVersionId: string;

/** @param {Location} */
location: Location;
location: ?Location;

/** @param {string} */
threadID: ?string;
Expand Down Expand Up @@ -163,6 +163,7 @@ class AnnotationThread extends EventEmitter {
getPopoverParent() {
return util.shouldDisplayMobileUI(this.container)
? this.container
// $FlowFixMe
: util.getPageEl(this.annotatedElement, this.location.page);
}

Expand All @@ -172,7 +173,7 @@ class AnnotationThread extends EventEmitter {
* @param {Event} event - Mouse event
* @return {void}
*/
renderAnnotationPopover(event: Event = null) {
renderAnnotationPopover(event: ?Event = null) {
if (event) {
event.stopPropagation();
event.preventDefault();
Expand Down Expand Up @@ -662,7 +663,7 @@ class AnnotationThread extends EventEmitter {
* @param {Object} eventData - Event data
* @return {void}
*/
emit(event: Event, eventData: Object) {
emit(event: Event, eventData: ?Object) {
const threadData = this.getThreadEventData();
super.emit(event, { data: threadData, eventData });
super.emit('threadevent', { event, data: threadData, eventData });
Expand Down
7 changes: 6 additions & 1 deletion src/controllers/AnnotationModeController.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,12 @@ class AnnotationModeController extends EventEmitter {
this.visibleThreadID = null;
break;
case THREAD_EVENT.render:
this.renderPage(eventData);
if (eventData) {
this.renderPage(eventData);
} else {
this.render();
}

break;
case THREAD_EVENT.delete:
// Thread should be cleaned up, unbind listeners - we
Expand Down
8 changes: 6 additions & 2 deletions src/controllers/__tests__/AnnotationModeController-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,7 @@ describe('controllers/AnnotationModeController', () => {
beforeEach(() => {
controller.emit = jest.fn();
controller.renderPage = jest.fn();
controller.render = jest.fn();
controller.registerThread = jest.fn();
controller.unregisterThread = jest.fn();
controller.localized = {
Expand All @@ -466,9 +467,12 @@ describe('controllers/AnnotationModeController', () => {
expect(controller.hadPendingThreads).toBeFalsy();
});

it('should re-render the page on render', () => {
controller.handleThreadEvents(thread, { event: THREAD_EVENT.render, data: {} });
it('should re-render the annotations on render', () => {
controller.handleThreadEvents(thread, { event: THREAD_EVENT.render, eventData: 1 });
expect(controller.renderPage).toBeCalled();

controller.handleThreadEvents(thread, { event: THREAD_EVENT.render });
expect(controller.render).toBeCalled();
});

it('should unregister thread on threadDelete', () => {
Expand Down
35 changes: 22 additions & 13 deletions src/doc/DocHighlightThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ import {
} from '../constants';

class DocHighlightThread extends AnnotationThread {
/**
* Cached page element for the document.
*
* @property {HTMLElement}
*/
pageEl;
/** @property {Location} */
location: ?Location;

/** @property {HTMLElement} */
pageEl: HTMLElement;

/**
* [constructor]
Expand Down Expand Up @@ -72,6 +71,8 @@ class DocHighlightThread extends AnnotationThread {
*/
destroy() {
this.threadID = null;

// $FlowFixMe
this.emit(THREAD_EVENT.render, this.location.page);
super.destroy();

Expand Down Expand Up @@ -108,9 +109,9 @@ class DocHighlightThread extends AnnotationThread {
* @param {string} text Text of annotation to save
* @return {void}
*/
save(type: AnnotationType, text: string) {
super.save(type, text);
save(type: AnnotationType, text: string): Promise<any> {
window.getSelection().removeAllRanges();
return super.save(type, text);
}

/**
Expand All @@ -120,14 +121,15 @@ class DocHighlightThread extends AnnotationThread {
* @param {boolean} [useServer] Whether or not to delete on server, default true
* @return {void}
*/
delete(annotation: Object, useServer: boolean = true) {
super.delete(annotation, useServer);
delete(annotation: Object, useServer: boolean = true): Promise<any> {
const promise = super.delete(annotation, useServer);

if (!this.threadID) {
return;
return promise;
}

this.renderAnnotationPopover();
return promise;
}

/**
Expand Down Expand Up @@ -284,6 +286,7 @@ class DocHighlightThread extends AnnotationThread {
scrollIntoView() {
this.scrollToPage();

// $FlowFixMe
const [yPos] = docUtil.getLowerRightCornerOfLastQuadPoint(this.location.quadPoints);

// Adjust scroll to highlight position
Expand Down Expand Up @@ -311,12 +314,14 @@ class DocHighlightThread extends AnnotationThread {
const pageHeight = pageDimensions.height - PAGE_PADDING_TOP - PAGE_PADDING_BOTTOM;
const zoomScale = util.getScale(this.annotatedElement);
const dimensionScale = util.getDimensionScale(
// $FlowFixMe
this.location.dimensions,
pageDimensions,
zoomScale,
PAGE_PADDING_TOP + PAGE_PADDING_BOTTOM
);

// $FlowFixMe
this.location.quadPoints.forEach((quadPoint) => {
// If needed, scale quad points comparing current dimensions with saved dimensions
let scaledQuadPoint = quadPoint;
Expand Down Expand Up @@ -367,6 +372,7 @@ class DocHighlightThread extends AnnotationThread {
const pageTop = pageDimensions.top + PAGE_PADDING_TOP;
const zoomScale = util.getScale(this.annotatedElement);
const dimensionScale = util.getDimensionScale(
// $FlowFixMe
this.location.dimensions,
pageDimensions,
zoomScale,
Expand All @@ -392,6 +398,7 @@ class DocHighlightThread extends AnnotationThread {

let eventOccurredInHighlight = false;

// $FlowFixMe
const points = this.location.quadPoints;
const { length } = points;

Expand Down Expand Up @@ -426,6 +433,7 @@ class DocHighlightThread extends AnnotationThread {
*/
getPageEl(): HTMLElement {
if (!this.pageEl) {
// $FlowFixMe
this.pageEl = util.getPageEl(this.annotatedElement, this.location.page);
}
return this.pageEl;
Expand All @@ -438,6 +446,7 @@ class DocHighlightThread extends AnnotationThread {
* @return {void}
*/
regenerateBoundary() {
// $FlowFixMe
if (!this.location || !this.location.quadPoints) {
return;
}
Expand Down Expand Up @@ -520,7 +529,7 @@ class DocHighlightThread extends AnnotationThread {
}

/** @inheritdoc */
cleanupAnnotationOnDelete(annotationIDToRemove: number) {
cleanupAnnotationOnDelete(annotationIDToRemove: string) {
// Delete matching comment from annotation
this.comments = this.comments.filter(({ id }) => id !== annotationIDToRemove);

Expand Down Expand Up @@ -557,7 +566,7 @@ class DocHighlightThread extends AnnotationThread {
}

/** @inheritdoc */
handleThreadSaveError(error: Error, tempAnnotationID: number) {
handleThreadSaveError(error: Error, tempAnnotationID: string) {
if (this.type === TYPES.highlight_comment && this.state === STATES.pending) {
this.type = TYPES.highlight;
this.reset();
Expand Down
1 change: 1 addition & 0 deletions src/doc/DocPointThread.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class DocPointThread extends AnnotationThread {
* @return {void}
*/
show() {
// $FlowFixMe
const pageEl = getPageEl(this.annotatedElement, this.location.page);
const [browserX, browserY] = getBrowserCoordinatesFromLocation(this.location, this.annotatedElement);

Expand Down

0 comments on commit 33adb47

Please sign in to comment.