From c4b4a402dbb4f4a2bb5ac0092873eb3b69085e79 Mon Sep 17 00:00:00 2001 From: Mingze Date: Mon, 30 Mar 2020 16:18:08 -0700 Subject: [PATCH] feat(header): Remove header buttons (#404) * feat(header): Remove header buttons * feat(header): Remove selectors --- i18n/en-US.properties | 8 -- src/constants.js | 10 -- src/controllers/AnnotationModeController.js | 50 ---------- src/controllers/DrawingModeController.js | 43 --------- src/controllers/PointModeController.js | 21 ----- .../AnnotationModeController-test.js | 91 ------------------- .../__tests__/DrawingModeController-test.js | 38 -------- .../__tests__/PointModeController-test.js | 16 ---- src/controllers/drawingShell.html | 19 +--- src/controllers/pointShell.html | 6 +- src/messages.js | 20 ---- 11 files changed, 2 insertions(+), 320 deletions(-) diff --git a/i18n/en-US.properties b/i18n/en-US.properties index 959204ff9..432355612 100644 --- a/i18n/en-US.properties +++ b/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 diff --git a/src/constants.js b/src/constants.js index f43bad734..97017c0eb 100644 --- a/src/constants.js +++ b/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'; diff --git a/src/controllers/AnnotationModeController.js b/src/controllers/AnnotationModeController.js index 60a75cfc3..0c6c2dd8f 100644 --- a/src/controllers/AnnotationModeController.js +++ b/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. diff --git a/src/controllers/DrawingModeController.js b/src/controllers/DrawingModeController.js index 3f895db6c..117d02e76 100644 --- a/src/controllers/DrawingModeController.js +++ b/src/controllers/DrawingModeController.js @@ -7,10 +7,6 @@ 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, @@ -18,7 +14,6 @@ import { 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); diff --git a/src/controllers/PointModeController.js b/src/controllers/PointModeController.js index aa110093d..8991b2377 100644 --- a/src/controllers/PointModeController.js +++ b/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 */ diff --git a/src/controllers/__tests__/AnnotationModeController-test.js b/src/controllers/__tests__/AnnotationModeController-test.js index 021790ee3..273d071c9 100644 --- a/src/controllers/__tests__/AnnotationModeController-test.js +++ b/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(); diff --git a/src/controllers/__tests__/DrawingModeController-test.js b/src/controllers/__tests__/DrawingModeController-test.js index 718effb29..99d0b70da 100644 --- a/src/controllers/__tests__/DrawingModeController-test.js +++ b/src/controllers/__tests__/DrawingModeController-test.js @@ -5,10 +5,6 @@ import DrawingModeController from '../DrawingModeController'; import * as util from '../../util'; import { STATES, - 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_MODE, CLASS_ACTIVE, @@ -71,20 +67,6 @@ describe('controllers/DrawingModeController', () => { }); }); - describe('setupHeader', () => { - it('should setup header and get all the mode buttons', () => { - const blankDiv = document.createElement('div'); - util.insertTemplate = jest.fn(); - controller.getButton = jest.fn().mockReturnValue(blankDiv); - - controller.setupHeader(blankDiv, blankDiv); - expect(controller.getButton).toBeCalledWith(SELECTOR_ANNOTATION_BUTTON_DRAW_CANCEL); - expect(controller.getButton).toBeCalledWith(SELECTOR_ANNOTATION_BUTTON_DRAW_POST); - expect(controller.getButton).toBeCalledWith(SELECTOR_ANNOTATION_BUTTON_DRAW_UNDO); - expect(controller.getButton).toBeCalledWith(SELECTOR_ANNOTATION_BUTTON_DRAW_REDO); - }); - }); - describe('cancelDrawing()', () => { it('should exit drawing mode', () => { controller.exit = jest.fn(); @@ -150,26 +132,6 @@ describe('controllers/DrawingModeController', () => { expect.any(Function), true, ); - expect(controller.pushElementHandler).toBeCalledWith( - controller.postButtonEl, - 'click', - expect.any(Function), - ); - expect(controller.pushElementHandler).toBeCalledWith( - controller.undoButtonEl, - 'click', - expect.any(Function), - ); - expect(controller.pushElementHandler).toBeCalledWith( - controller.redoButtonEl, - 'click', - expect.any(Function), - ); - expect(controller.pushElementHandler).toBeCalledWith( - controller.cancelButtonEl, - 'click', - expect.any(Function), - ); expect(controller.pushElementHandler).toBeCalledWith( controller.annotatedElement, ['mousedown', 'touchstart'], diff --git a/src/controllers/__tests__/PointModeController-test.js b/src/controllers/__tests__/PointModeController-test.js index ad4650f12..12b8c6af8 100644 --- a/src/controllers/__tests__/PointModeController-test.js +++ b/src/controllers/__tests__/PointModeController-test.js @@ -7,7 +7,6 @@ import { CLASS_ANNOTATION_MODE, THREAD_EVENT, CONTROLLER_EVENT, - SELECTOR_ANNOTATION_BUTTON_POINT_EXIT, SELECTOR_POINT_MODE_HEADER, } from '../../constants'; import AnnotationThread from '../../AnnotationThread'; @@ -61,16 +60,6 @@ describe('controllers/PointModeController', () => { }); }); - describe('setupHeader', () => { - it('should setup header and get all the mode buttons', () => { - const blankDiv = document.createElement('div'); - util.insertTemplate = jest.fn(); - controller.getButton = jest.fn().mockReturnValue(blankDiv); - controller.setupHeader(blankDiv, blankDiv); - expect(controller.getButton).toBeCalledWith(SELECTOR_ANNOTATION_BUTTON_POINT_EXIT); - }); - }); - describe('setupHandlers()', () => { it('should successfully contain mode handlers', () => { controller.pushElementHandler = jest.fn(); @@ -83,11 +72,6 @@ describe('controllers/PointModeController', () => { controller.pointClickHandler, true, ); - expect(controller.pushElementHandler).toBeCalledWith( - controller.exitButtonEl, - 'click', - controller.toggleMode, - ); }); }); diff --git a/src/controllers/drawingShell.html b/src/controllers/drawingShell.html index d50b29922..da14faaa4 100644 --- a/src/controllers/drawingShell.html +++ b/src/controllers/drawingShell.html @@ -1,18 +1 @@ -
-
- - -
-
- - -
-
+
diff --git a/src/controllers/pointShell.html b/src/controllers/pointShell.html index 64912e4bb..5e2c53cc2 100644 --- a/src/controllers/pointShell.html +++ b/src/controllers/pointShell.html @@ -1,5 +1 @@ -
-
- -
-
+
diff --git a/src/messages.js b/src/messages.js index 502244eb5..14bd29ce6 100644 --- a/src/messages.js +++ b/src/messages.js @@ -6,21 +6,11 @@ export default defineMessages({ description: 'Place holder for the anonymous user', defaultMessage: 'Some User', }, - annotationsCancel: { - id: 'ba.annotationsCancel', - description: 'Label for the Cancel button', - defaultMessage: 'Cancel', - }, annotationsClose: { id: 'ba.annotationsClose', description: 'Label for the close button', defaultMessage: 'Close', }, - annotationsDone: { - id: 'ba.annotationsDone', - description: 'Label for the Done button', - defaultMessage: 'Done', - }, annotationsSave: { id: 'ba.annotationsSave', description: 'Label for the save button', @@ -31,21 +21,11 @@ export default defineMessages({ description: 'Label for the post button', defaultMessage: 'Post', }, - annotationDrawToggle: { - id: 'ba.annotationDrawToggle', - description: 'Accessibility message for button that toggles drawing annotation mode ', - defaultMessage: 'Drawing annotation mode', - }, annotationHighlightToggle: { id: 'ba.annotationHighlightToggle', description: 'Accessibility message for button that toggles highlight annotation mode ', defaultMessage: 'Highlight text', }, - annotationPointToggle: { - id: 'ba.annotationPointToggle', - description: 'Accessibility message for button that toggles point annotation mode ', - defaultMessage: 'Point annotation mode', - }, annotationsAuthError: { id: 'ba.annotationsAuthError', description: 'Error message when Authorizing',