feat(header): Remove header buttons (#404)

* feat(header): Remove header buttons

* feat(header): Remove selectors

Author: Mingze

i18n/en-US.properties

@@ -1,21 +1,13 @@
-# Accessibility message for button that toggles drawing annotation mode
-ba.annotationDrawToggle = Drawing annotation mode
 # Accessibility message for button that toggles highlight annotation mode
 ba.annotationHighlightToggle = Highlight text
-# Accessibility message for button that toggles point annotation mode
-ba.annotationPointToggle = Point annotation mode
 # Error message when Authorizing
 ba.annotationsAuthError = Your session has expired. Please refresh the page
-# Label for the Cancel button
-ba.annotationsCancel = Cancel
 # Label for the close button
 ba.annotationsClose = Close
 # Error message when creating
 ba.annotationsCreateError = We’re sorry, the annotation could not be created.
 # Error message when deleting an annotation
 ba.annotationsDeleteError = We’re sorry, the annotation could not be deleted.
-# Label for the Done button
-ba.annotationsDone = Done
 # Error message when loading
 ba.annotationsLoadError = We’re sorry, annotations failed to load for this file
 # Label for the post button

src/constants.js

@@ -65,8 +65,6 @@ export const CLASS_ANNOTATION_DRAW_MODE = 'ba-draw-mode';
 export const SELECTOR_ANNOTATION_DRAW_MODE = `.${CLASS_ANNOTATION_DRAW_MODE}`;
 export const CLASS_ANNNOTATION_MODE_BACKGROUND = 'ba-annotate-mode-background';
 export const SELECTOR_ANNNOTATION_MODE_BACKGROUND = `.${CLASS_ANNNOTATION_MODE_BACKGROUND}`;
-export const CLASS_ANNOTATION_BUTTON_POINT_EXIT = 'ba-btn-annotate-point-exit';
-export const SELECTOR_ANNOTATION_BUTTON_POINT_EXIT = `.${CLASS_ANNOTATION_BUTTON_POINT_EXIT}`;
 
 export const CLASS_ANNOTATION_MODE_HEADER = 'ba-mode-header';
 export const SELECTOR_ANNOTATION_MODE_HEADER = `.${CLASS_ANNOTATION_MODE_HEADER}`;
@@ -93,14 +91,6 @@ export const SELECTOR_HIGHLIGHT_QUAD_CORNER = `.${CLASS_HIGHLIGHT_QUAD_CORNER}`;
 // Drawing CSS constants
 export const CLASS_ANNOTATION_DRAW = 'ba-annotation-draw';
 export const SELECTOR_ANNOTATION_DRAW = `.${CLASS_ANNOTATION_DRAW}`;
-export const CLASS_ANNOTATION_BUTTON_DRAW_UNDO = 'ba-btn-annotate-draw-undo';
-export const SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO = `.${CLASS_ANNOTATION_BUTTON_DRAW_UNDO}`;
-export const CLASS_ANNOTATION_BUTTON_DRAW_REDO = 'ba-btn-annotate-draw-redo';
-export const SELECTOR_ANNOTATION_BUTTON_DRAW_REDO = `.${CLASS_ANNOTATION_BUTTON_DRAW_REDO}`;
-export const CLASS_ANNOTATION_BUTTON_DRAW_POST = 'ba-btn-annotate-draw-post';
-export const SELECTOR_ANNOTATION_BUTTON_DRAW_POST = `.${CLASS_ANNOTATION_BUTTON_DRAW_POST}`;
-export const CLASS_ANNOTATION_BUTTON_DRAW_CANCEL = 'ba-btn-annotate-draw-cancel';
-export const SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL = `.${CLASS_ANNOTATION_BUTTON_DRAW_CANCEL}`;
 
 // Data types
 export const DATA_TYPE_ANNOTATION_INDICATOR = 'annotation-indicator';

src/controllers/AnnotationModeController.js

@@ -5,7 +5,6 @@ import noop from 'lodash/noop';
 
 import { insertTemplate, replaceHeader, hasValidBoundaryCoordinates } from '../util';
 import {
-    CLASS_HIDDEN,
     CLASS_ACTIVE,
     CLASS_ANNOTATION_MODE,
     CLASS_ANNNOTATION_MODE_BACKGROUND,
@@ -37,12 +36,6 @@ class AnnotationModeController extends EventEmitter {
     /** @property {HTMLElement} - Header HTML DOM element */
     headerElement: HTMLElement;
 
-    /** @property {HTMLElement} - Annotation mode button HTML DOM element */
-    buttonEl: HTMLElement;
-
-    /** @property {Object} - Annotation mode button selector and title */
-    modeButton: Object;
-
     /** @property {AnnotationType} - Mode for annotation controller */
     mode: AnnotationType;
 
@@ -112,11 +105,6 @@ class AnnotationModeController extends EventEmitter {
             permissions: this.permissions,
         });
         this.api.addListener(CONTROLLER_EVENT.error, this.handleAPIErrors);
-
-        if (data.modeButton && this.permissions.can_annotate) {
-            this.modeButton = data.modeButton;
-            this.showButton();
-        }
     }
 
     /**
@@ -151,44 +139,6 @@ class AnnotationModeController extends EventEmitter {
         return this.headerElement.querySelector(annotatorSelector);
     }
 
-    /**
-     * Shows the annotate button for the specified mode
-     *
-     * @return {void}
-     */
-    showButton(): void {
-        if (!this.permissions.can_annotate) {
-            return;
-        }
-
-        this.buttonEl = this.getButton(this.modeButton.selector);
-        // $FlowFixMe
-        if (this.buttonEl) {
-            this.buttonEl.classList.remove(CLASS_HIDDEN);
-
-            // $FlowFixMe
-            this.toggleMode = this.toggleMode.bind(this);
-            // $FlowFixMe
-            this.buttonEl.addEventListener('click', this.toggleMode);
-        }
-    }
-
-    /**
-     * Hides the annotate button for the specified mode
-     *
-     * @return {void}
-     */
-    hideButton() {
-        if (!this.permissions.can_annotate || !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.

src/controllers/DrawingModeController.js

@@ -7,18 +7,13 @@ import {
     TYPES,
     STATES,
     THREAD_EVENT,
-    SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL,
-    SELECTOR_ANNOTATION_BUTTON_DRAW_POST,
-    SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO,
-    SELECTOR_ANNOTATION_BUTTON_DRAW_REDO,
     SELECTOR_DRAW_MODE_HEADER,
     CLASS_ANNOTATION_LAYER_DRAW,
     CLASS_ANNOTATION_DRAW_MODE,
     CLASS_ANNOTATION_POINT_MARKER,
     ANNOTATOR_TYPE,
     CLASS_ANNOTATION_LAYER_DRAW_IN_PROGRESS,
 } from '../constants';
-import messages from '../messages';
 
 // $FlowFixMe
 import shell from './drawingShell.html';
@@ -27,18 +22,6 @@ class DrawingModeController extends AnnotationModeController {
     /** @property {AnnotationThread} - The currently selected DrawingThread */
     selectedThread: AnnotationThread;
 
-    /** @property {HTMLElement} - The button to cancel the pending drawing thread */
-    cancelButtonEl: HTMLElement;
-
-    /** @property {HTMLElement} - The button to commit the pending drawing thread */
-    postButtonEl: HTMLElement;
-
-    /** @property {HTMLElement} - The button to undo a stroke on the pending drawing thread */
-    undoButtonEl: HTMLElement;
-
-    /** @property {HTMLElement} - The button to redo a stroke on the pending drawing thread */
-    redoButtonEl: HTMLElement;
-
     /** @property {AnnotationThread} */
     currentThread: AnnotationThread;
 
@@ -58,28 +41,6 @@ class DrawingModeController extends AnnotationModeController {
         }
     }
 
-    showButton(): void {
-        super.showButton();
-
-        this.buttonEl.title = this.intl.formatMessage(messages.annotationDrawToggle);
-    }
-
-    /** @inheritdoc */
-    setupHeader(container: HTMLElement, header: HTMLElement): void {
-        super.setupHeader(container, header);
-
-        this.cancelButtonEl = this.getButton(SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL);
-        this.cancelButtonEl.textContent = this.intl.formatMessage(messages.annotationsCancel);
-
-        this.postButtonEl = this.getButton(SELECTOR_ANNOTATION_BUTTON_DRAW_POST);
-
-        // TODO(@spramod): Remove '||' string, once doneButton is properly localized within Preview
-        this.postButtonEl.textContent = this.intl.formatMessage(messages.annotationsDone);
-
-        this.undoButtonEl = this.getButton(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO);
-        this.redoButtonEl = this.getButton(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO);
-    }
-
     /**
      * Prevents click events from triggering other annotation types
      *
@@ -167,10 +128,6 @@ class DrawingModeController extends AnnotationModeController {
         this.drawingStartHandler = this.drawingStartHandler.bind(this);
 
         this.pushElementHandler(this.annotatedElement, 'click', this.stopPropagation, true);
-        this.pushElementHandler(this.cancelButtonEl, 'click', this.cancelDrawing);
-        this.pushElementHandler(this.postButtonEl, 'click', this.postDrawing);
-        this.pushElementHandler(this.undoButtonEl, 'click', this.undoDrawing);
-        this.pushElementHandler(this.redoButtonEl, 'click', this.redoDrawing);
 
         // Both click and touch listeners are bound for touch-enabled laptop edge cases
         this.pushElementHandler(this.annotatedElement, ['mousedown', 'touchstart'], this.drawingStartHandler, true);

src/controllers/PointModeController.js

@@ -8,20 +8,15 @@ import {
     THREAD_EVENT,
     CLASS_ACTIVE,
     SELECTOR_POINT_MODE_HEADER,
-    SELECTOR_ANNOTATION_BUTTON_POINT_EXIT,
     CLASS_ANNOTATION_POINT_MODE,
     ANNOTATOR_TYPE,
 } from '../constants';
-import messages from '../messages';
 import { replaceHeader, isInAnnotationOrMarker } from '../util';
 
 // $FlowFixMe
 import shell from './pointShell.html';
 
 class PointModeController extends AnnotationModeController {
-    /** @property {HTMLElement} - The button to exit point annotation mode */
-    exitButtonEl: HTMLElement;
-
     /** @inheritdoc */
     init(data: Object): void {
         super.init(data);
@@ -35,21 +30,6 @@ class PointModeController extends AnnotationModeController {
         }
     }
 
-    showButton(): void {
-        super.showButton();
-        this.buttonEl.title = this.intl.formatMessage(messages.annotationPointToggle);
-    }
-
-    /** @inheritdoc */
-    setupHeader(container: HTMLElement, header: HTMLElement): void {
-        super.setupHeader(container, header);
-
-        this.exitButtonEl = this.getButton(SELECTOR_ANNOTATION_BUTTON_POINT_EXIT);
-
-        // TODO(@spramod): Remove '||' string, once closeButton is properly localized
-        this.exitButtonEl.textContent = this.intl.formatMessage(messages.annotationsClose);
-    }
-
     /** @inheritdoc */
     setupHandlers(): void {
         // $FlowFixMe
@@ -59,7 +39,6 @@ class PointModeController extends AnnotationModeController {
 
         // Get handlers
         this.pushElementHandler(this.annotatedElement, ['click', 'touchstart'], this.pointClickHandler, true);
-        this.pushElementHandler(this.exitButtonEl, 'click', this.toggleMode);
     }
 
     /** @inheritdoc */

src/controllers/__tests__/AnnotationModeController-test.js

@@ -3,7 +3,6 @@ import rbush from 'rbush';
 import AnnotationModeController from '../AnnotationModeController';
 import * as util from '../../util';
 import {
-    CLASS_HIDDEN,
     CLASS_ACTIVE,
     CLASS_ANNOTATION_MODE,
     CLASS_ANNNOTATION_MODE_BACKGROUND,
@@ -67,7 +66,6 @@ describe('controllers/AnnotationModeController', () => {
                 permissions: { can_annotate: true },
                 intl: intlMock,
             });
-            expect(controller.showButton).toBeCalled();
         });
 
         it('should not show modeButton if none provided', () => {
@@ -129,95 +127,6 @@ describe('controllers/AnnotationModeController', () => {
             });
         });
 
-        describe('showButton()', () => {
-            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 = { can_annotate: true };
-                controller.getButton = jest.fn().mockReturnValue(buttonEl);
-            });
-
-            it('should do nothing if user cannot annotate', () => {
-                controller.permissions.can_annotate = false;
-                controller.showButton();
-                expect(buttonEl.classList).toContain(CLASS_HIDDEN);
-            });
-
-            it('should do nothing if the button is not in the container', () => {
-                controller.getButton = jest.fn();
-                controller.showButton();
-                expect(buttonEl.classList).toContain(CLASS_HIDDEN);
-            });
-
-            it('should set up and show an annotate button', () => {
-                controller.showButton();
-                expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
-                expect(buttonEl.addEventListener).toBeCalledWith('click', controller.toggleMode);
-            });
-            it('should set up and show an annotate button', () => {
-                controller.showButton();
-                expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
-                expect(buttonEl.addEventListener).toBeCalledWith('click', controller.toggleMode);
-            });
-        });
-
-        describe('hideButton()', () => {
-            let buttonEl;
-
-            beforeEach(() => {
-                controller.modeButton = {
-                    type: {
-                        title: 'Annotation Mode',
-                        selector: '.selector',
-                    },
-                };
-                buttonEl = document.createElement('button');
-                buttonEl.title = controller.modeButton.title;
-                buttonEl.classList.remove(CLASS_HIDDEN);
-                buttonEl.classList.add('selector');
-                buttonEl.addEventListener = jest.fn();
-
-                controller.permissions = { can_annotate: true };
-                controller.getButton = jest.fn().mockReturnValue(buttonEl);
-            });
-
-            it('should do nothing if user cannot annotate', () => {
-                controller.permissions.can_annotate = 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.can_annotate = false;
-                controller.hideButton();
-                expect(buttonEl.classList).not.toContain(CLASS_HIDDEN);
-            });
-        });
-
         describe('toggleMode()', () => {
             beforeEach(() => {
                 controller.emit = jest.fn();