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

Merging Annotation header changes #240

Merged
merged 6 commits into from
Oct 16, 2018
Merged

Merging Annotation header changes #240

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
91cdce3
Update: Annotations with header (#233)
Oct 11, 2018
2dfc22e
Fix: Functional tests scripts
pramodsum Oct 11, 2018
6de2cf4
Chore: Update preview version to 1.54.0 in functional tests (#239)
Oct 12, 2018
6056334
Merge branch 'master' of github.com:box/box-annotations into react
pramodsum Oct 16, 2018
0d11ec0
Fix: Bugs
pramodsum Oct 16, 2018
8149e12
Fix: Tests
pramodsum Oct 16, 2018
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
9 changes: 9 additions & 0 deletions functional-tests/README.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
20 changes: 17 additions & 3 deletions src/Annotator.js
View file
Open in desktop
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
} from './constants';
import FileVersionAPI from './api/FileVersionAPI';

Expand Down Expand Up @@ -100,8 +101,20 @@ class Annotator extends EventEmitter {
init(initialScale = 1) {
// 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);
if (typeof this.container === 'string') {
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);
}

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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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