Skip to content

Commit

Permalink
Merge branch 'master' of github.com:box/box-annotations into react
Browse files Browse the repository at this point in the history
  • Loading branch information
@pramodsum
pramodsum committed Oct 16, 2018
2 parents cdf2f06 + 6de2cf4 commit 6056334
Show file tree
Hide file tree
Showing 13 changed files with 453 additions and 46 deletions.
9 changes: 9 additions & 0 deletions functional-tests/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,12 @@
3) Start Selenium `selenium-standalone start`
4) In a separate terminal, build BoxAnnotations `yarn run selenium-build`
5) Run functional tests `FILE_ID="285568802145" FILE_VERSION_ID="300497533713" yarn run functional-tests`

### Running Specific Tests
1) Start Selenium `selenium-standalone start`
2) In a separate terminal, run `FILE_ID="<file-id>" FILE_VERSION_ID="<file-version-id>" ACCESS_TOKEN="<access-token>" CLIENT_ID="<client-id>" node functional-tests/app.js`
* You can find existing `file-id` and `file-version-id` in the .travis.yml file
3) In yet another separate terminal, run `FILE_ID="<file-id>" FILE_VERSION_ID="file-version-id>" ACCESS_TOKEN="<access-token>" CLIENT_ID="<client-id>" node ./node_modules/codeceptjs/bin/codecept.js run --grep "<regex>" --verbose`
* Make sure the `file-id` and `file-version-id` values are the same between these two terminals :)
* an example of `<regex>` would be "@doc" to run all the tests with "@doc" in the name
4) You should see a separate Chrome browser instance appear and run the tests
2 changes: 1 addition & 1 deletion functional-tests/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function gracefulShutdown() {
}

function errorCallback(err) {
console.log(err.response.body);
console.log(err);
gracefulShutdown();
}

Expand Down
4 changes: 2 additions & 2 deletions functional-tests/views/index.pug
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ html
<meta charset="UTF-8">
link(rel="stylesheet" href='annotations.css')
script(src='annotations.js')
link(rel="stylesheet" href='https://cdn01.boxcdn.net/platform/preview/1.37.0/en-US/preview.css')
script(src='https://cdn01.boxcdn.net/platform/preview/1.37.0/en-US/preview.js')
link(rel="stylesheet" href='https://cdn01.boxcdn.net/platform/preview/1.54.0/en-US/preview.css')
script(src='https://cdn01.boxcdn.net/platform/preview/1.54.0/en-US/preview.js')
style.
.preview-container {
border: 1px solid #eee;
Expand Down
7 changes: 7 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
"babel-preset-stage-1": "^6.24.1",
"box-locales": "^0.0.1",
"box-react-ui": "^26.1.1",
"bluebird": "^3.5.2",
"box-node-sdk": "^1.22.0",
"chai": "^4.1.2",
"chai-dom": "^1.8.0",
"circular-dependency-plugin": "^5.0.2",
Expand Down Expand Up @@ -91,6 +93,7 @@
"immutable": "^3.7.4",
"jest": "^23.5.0",
"jest-canvas-mock": "^1.1.0",
"jws": "^3.1.5",
"lint-staged": "^7.2.2",
"mini-css-extract-plugin": "^0.4.2",
"mocha": "^5.2.0",
Expand All @@ -104,6 +107,10 @@
"prop-types": "^15.6.1",
"properties-parser": "^0.3.1",
"raf": "^3.4.0",
"pug": "^2.0.3",
"pug-code-gen": "^2.0.1",
"pug-filters": "^3.1.0",
"pug-lexer": "^4.0.0",
"rangy": "^1.3.0",
"raw-loader": "^0.5.1",
"rbush": "^2.0.1",
Expand Down
18 changes: 16 additions & 2 deletions src/Annotator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
THREAD_EVENT,
ANNOTATOR_EVENT,
CONTROLLER_EVENT,
CLASS_ANNOTATIONS_LOADED
CLASS_ANNOTATIONS_LOADED,
SELECTOR_BOX_PREVIEW_HEADER_CONTAINER
} from './constants';
import FileVersionAPI from './api/FileVersionAPI';

Expand Down Expand Up @@ -101,7 +102,19 @@ class Annotator extends EventEmitter {
// Get the container dom element if selector was passed, in tests
this.container = this.options.container;
if (typeof this.options.container === 'string') {
this.container = document.querySelector(this.options.container);
this.container = document.querySelector(this.container);
}

// Get the header dom element if selector was passed, in tests
this.headerElement = this.options.headerElement;
if (typeof this.headerElement === 'string') {
this.headerElement = document.querySelector(this.headerElement);
}

// If using box content preview header and no external header element was specified,
// fallback to the container element
if (this.options.header !== 'none' && !this.headerElement) {
this.headerElement = this.container.querySelector(SELECTOR_BOX_PREVIEW_HEADER_CONTAINER);
}

if (!this.container) {
Expand Down Expand Up @@ -233,6 +246,7 @@ class Annotator extends EventEmitter {
const controller = this.modeControllers[type];
controller.init({
container: this.container,
headerElement: this.headerElement,
annotatedElement: this.annotatedElement,
mode: type,
modeButton: this.modeButtons[type],
Expand Down
27 changes: 18 additions & 9 deletions src/__tests__/Annotator-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,23 @@ import {
ANNOTATOR_EVENT,
THREAD_EVENT,
CONTROLLER_EVENT,
SELECTOR_ANNOTATED_ELEMENT
SELECTOR_ANNOTATED_ELEMENT,
SELECTOR_BOX_PREVIEW_HEADER_CONTAINER
} from '../constants';

const api = {
formatAnnotation: jest.fn()
};
let annotator;
let controller;
let thread;
const html = `<div class="bp-header-container"></div>
<button class="bp-btn-annotate"></button>
<div class="annotated-element"></div>`;

describe('Annotator', () => {
let rootElement;
let annotator;
let controller;
let thread;
const html = `<button class="bp-btn-annotate"></button>
<div class="annotated-element"></div>`;

const api = {
formatAnnotation: jest.fn()
};

beforeEach(() => {
rootElement = document.createElement('div');
Expand Down Expand Up @@ -115,6 +118,12 @@ describe('Annotator', () => {
expect(annotator.loadAnnotations).toBeCalled();
});

it('should set the headerElement to the container as a fallback', () => {
annotator.options.header = 'light';
annotator.init(5);
expect(annotator.headerElement).toEqual(document.querySelector(SELECTOR_BOX_PREVIEW_HEADER_CONTAINER));
});

it('should setup mobile dialog for mobile browsers', () => {
annotator.isMobile = true;
annotator.init();
Expand Down
2 changes: 2 additions & 0 deletions src/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export const CLASS_ANNOTATION_BUTTON_DRAW = 'bp-btn-annotate-draw';
export const SELECTOR_ANNOTATION_BUTTON_DRAW = `.${CLASS_ANNOTATION_BUTTON_DRAW}`;
export const CLASS_ANNOTATION_BUTTON_DRAW_ENTER = 'bp-btn-annotate-draw-enter';
export const SELECTOR_ANNOTATION_BUTTON_DRAW_ENTER = `.${CLASS_ANNOTATION_BUTTON_DRAW_ENTER}`;
export const CLASS_BOX_PREVIEW_HEADER_CONTAINER = 'bp-header-container';
export const SELECTOR_BOX_PREVIEW_HEADER_CONTAINER = `.${CLASS_BOX_PREVIEW_HEADER_CONTAINER}`;

export const CLASS_BOX_PREVIEW = 'bp';
export const SELECTOR_BOX_PREVIEW = `.${CLASS_BOX_PREVIEW}`;
Expand Down
31 changes: 28 additions & 3 deletions src/controllers/AnnotationModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class AnnotationModeController extends EventEmitter {
*/
init(data: Object): void {
this.container = data.container;
this.headerElement = data.headerElement;
this.annotatedElement = data.annotatedElement;
this.mode = data.mode;
this.fileVersionId = data.fileVersionId;
Expand Down Expand Up @@ -130,6 +131,10 @@ class AnnotationModeController extends EventEmitter {
this.buttonEl.removeEventListener('click', this.toggleMode);
}
this.api.removeListener(CONTROLLER_EVENT.error, this.handleAPIErrors);

if (this.modeButton) {
this.hideButton();
}
}

/**
Expand All @@ -139,7 +144,11 @@ class AnnotationModeController extends EventEmitter {
* @return {HTMLElement|null} Annotate button element or null if the selector did not find an element.
*/
getButton(annotatorSelector: string): HTMLElement {
return this.container.querySelector(annotatorSelector) || this.container;
if (!this.headerElement) {
return null;
}

return this.headerElement.querySelector(annotatorSelector);
}

/**
Expand All @@ -148,7 +157,7 @@ class AnnotationModeController extends EventEmitter {
* @return {void}
*/
showButton(): void {
if (!this.permissions.can_annotate) {
if (!this.permissions.canAnnotate || !this.modeButton) {
return;
}

Expand All @@ -162,6 +171,22 @@ class AnnotationModeController extends EventEmitter {
}
}

/**
* Hides the annotate button for the specified mode
*
* @return {void}
*/
hideButton() {
if (!this.permissions.canAnnotate || !this.modeButton) {
return;
}

this.buttonEl = this.getButton(this.modeButton.selector);
if (this.buttonEl) {
this.buttonEl.classList.add(CLASS_HIDDEN);
}
}

/**
* Toggles annotation modes on and off. When an annotation mode is
* on, annotation threads will be created at that location.
Expand All @@ -187,7 +212,7 @@ class AnnotationModeController extends EventEmitter {
*/
exit(): void {
this.emit(CONTROLLER_EVENT.exit, { mode: this.mode });
replaceHeader(this.container, SELECTOR_BOX_PREVIEW_BASE_HEADER);
replaceHeader(this.headerElement, SELECTOR_BOX_PREVIEW_BASE_HEADER);

this.destroyPendingThreads();

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/DrawingModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class DrawingModeController extends AnnotationModeController {
// light, dark, or no value given), then we want to use our draw
// header. Otherwise we expect header UI to be handled by Preview’s
// consumer
if (data.options.header !== 'none') {
this.setupHeader(this.container, shell);
if (data.options.header !== 'none' || this.headerElement) {
this.setupHeader(this.headerElement, shell);
}
}

Expand Down Expand Up @@ -268,7 +268,7 @@ class DrawingModeController extends AnnotationModeController {
*/
enter(): void {
super.enter();
replaceHeader(this.container, SELECTOR_DRAW_MODE_HEADER);
replaceHeader(this.headerElement, SELECTOR_DRAW_MODE_HEADER);
this.annotatedElement.classList.add(CLASS_ANNOTATION_DRAW_MODE);
}

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/PointModeController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ class PointModeController extends AnnotationModeController {
// light, dark, or no value given), then we want to use our draw
// header. Otherwise we expect header UI to be handled by Preview’s
// consumer
if (data.options.header !== 'none') {
this.setupHeader(this.container, shell);
if (data.options.header !== 'none' || this.headerElement) {
this.setupHeader(this.headerElement, shell);
}
}

Expand Down Expand Up @@ -117,7 +117,7 @@ class PointModeController extends AnnotationModeController {
/** @inheritdoc */
enter(): void {
super.enter();
replaceHeader(this.container, SELECTOR_POINT_MODE_HEADER);
replaceHeader(this.headerElement, SELECTOR_POINT_MODE_HEADER);
this.annotatedElement.classList.add(CLASS_ANNOTATION_POINT_MODE);

if (this.buttonEl) {
Expand Down
82 changes: 74 additions & 8 deletions src/controllers/__tests__/AnnotationModeController-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,23 +109,37 @@ describe('controllers/AnnotationModeController', () => {
expect(controller.buttonEl).toBeUndefined();
});

it('should remove listener from button', () => {
controller.buttonEl = {
removeEventListener: jest.fn()
};
it('should hide the button if modeButton exists', () => {
controller.modeButton = {};
controller.destroy();
expect(controller.hideButton).toBeCalled();
});

it('should not hide the button if modeButton does not exist', () => {
controller.modeButton = undefined;
controller.destroy();
expect(controller.buttonEl.removeEventListener).toBeCalled();
expect(controller.hideButton).not.toBeCalled();
});
});

describe('getButton', () => {
it('should return the annotation mode button', () => {
const buttonEl = document.createElement('button');
buttonEl.classList.add('class');
controller.container = document.createElement('div');
controller.container.appendChild(buttonEl);
controller.headerElement = document.createElement('div');
controller.headerElement.appendChild(buttonEl);

expect(controller.getButton('.class')).not.toBeNull();
it('should remove listener from button', () => {
controller.buttonEl = {
removeEventListener: jest.fn()
};
controller.destroy();
expect(controller.buttonEl.removeEventListener).toBeCalled();
});
});

it('should return null if no headerElement', () => {
expect(controller.getButton('.class')).toBeNull();
});
});

Expand Down Expand Up @@ -166,6 +180,58 @@ describe('controllers/AnnotationModeController', () => {
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
expect(buttonEl.addEventListener).toBeCalledWith('click', controller.toggleMode);
});

it('should do nothing if no modeButton', () => {
controller.modeButton = undefined;
controller.permissions.canAnnotate = false;
controller.showButton();
expect(buttonEl.classList).toContain(CLASS_HIDDEN);
});
});

describe('hideButton()', () => {
let buttonEl;

beforeEach(() => {
controller.modeButton = {
type: {
title: 'Annotation Mode',
selector: '.selector'
}
};
buttonEl = document.createElement('button');
buttonEl.title = controller.modeButton.title;
// buttonEl.classList.add(CLASS_HIDDEN);
buttonEl.classList.add('selector');
buttonEl.addEventListener = jest.fn();

controller.permissions = { canAnnotate: true };
controller.getButton = jest.fn().mockReturnValue(buttonEl);
});

it('should do nothing if user cannot annotate', () => {
controller.permissions.canAnnotate = false;
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});

it('should do nothing if button is not found', () => {
controller.getButton = jest.fn();
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});

it('should add the bp-is-hidden class to the button', () => {
controller.hideButton();
expect(buttonEl.classList).toContain(CLASS_HIDDEN);
});

it('should do nothing if no modeButton', () => {
controller.modeButton = undefined;
controller.permissions.canAnnotate = false;
controller.hideButton();
expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
});
});

describe('toggleMode()', () => {
Expand Down
Loading

0 comments on commit 6056334

Please sign in to comment.