Skip to content

Commit

Permalink
Merge pull request #836 from JamesAPetts/feature/eventDispatchLocking…
Browse files Browse the repository at this point in the history
…Changes

Feature/event dispatch locking changes
  • Loading branch information
JamesAPetts authored Feb 15, 2019
2 parents 5e66375 + dfd6f8b commit 53bf96f
Show file tree
Hide file tree
Showing 14 changed files with 105 additions and 13 deletions.
12 changes: 11 additions & 1 deletion src/eventDispatchers/mouseEventHandlers/mouseDown.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import getToolsWithMoveableHandles from '../../store/getToolsWithMoveableHandles
import { findHandleDataNearImagePoint } from '../../util/findAndMoveHelpers.js';
import getInteractiveToolsForElement from './../../store/getInteractiveToolsForElement.js';
import getToolsWithDataForElement from './../../store/getToolsWithDataForElement.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

/**
* MouseDown is called before MouseDownActivate. If MouseDown
Expand Down Expand Up @@ -34,14 +35,18 @@ export default function(evt) {

// ACTIVE TOOL W/ PRE CALLBACK?
// Note: In theory, this should only ever be a single tool.
const activeTools = activeAndPassiveTools.filter(
let activeTools = activeAndPassiveTools.filter(
tool =>
tool.mode === 'active' &&
Array.isArray(tool.options.mouseButtonMask) &&
tool.options.mouseButtonMask.includes(eventData.buttons) &&
tool.options.isMouseActive
);

if (state.isMultiPartToolActive) {
activeTools = filterToolsUseableWithMultiPartTools(activeTools);
}

// If any tools are active, check if they have a special reason for dealing with the event.
if (activeTools.length > 0) {
// TODO: If length > 1, you could assess fitness and select the ideal tool
Expand All @@ -62,6 +67,11 @@ export default function(evt) {
}
}

if (state.isMultiPartToolActive) {
// Don't fire events to Annotation Tools during a multi part loop.
return;
}

// Annotation tool specific
const annotationTools = getToolsWithDataForElement(
element,
Expand Down
9 changes: 9 additions & 0 deletions src/eventDispatchers/mouseEventHandlers/mouseDownActivate.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import addNewMeasurement from './addNewMeasurement.js';
import { getters, state } from './../../store/index.js';
import getActiveToolsForElement from './../../store/getActiveToolsForElement.js';
import BaseAnnotationTool from './../../tools/base/BaseAnnotationTool.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

// Todo: We could simplify this if we only allow one active
// Tool per mouse button mask?
Expand All @@ -24,6 +25,10 @@ export default function(evt) {
tool.options.isMouseActive
);

if (state.isMultiPartToolActive) {
tools = filterToolsUseableWithMultiPartTools(tools);
}

if (tools.length === 0) {
return;
}
Expand All @@ -38,6 +43,10 @@ export default function(evt) {
}
}

if (state.isMultiPartToolActive) {
return;
}

// Note: custom `addNewMeasurement` will need to prevent event bubbling
if (activeTool.addNewMeasurement) {
activeTool.addNewMeasurement(evt, 'mouse');
Expand Down
5 changes: 5 additions & 0 deletions src/eventDispatchers/mouseEventHandlers/mouseDrag.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getters, state } from './../../store/index.js';
import getActiveToolsForElement from './../../store/getActiveToolsForElement.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

// Tools like wwwc. Non-annotation tools w/ specific
// Down + mouse behavior
Expand All @@ -25,6 +26,10 @@ export default function(evt) {
);
tools = tools.filter(tool => typeof tool.mouseDragCallback === 'function');

if (state.isMultiPartToolActive) {
tools = filterToolsUseableWithMultiPartTools(tools);
}

if (tools.length === 0) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion src/eventDispatchers/mouseEventHandlers/mouseMove.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import getToolsWithDataForElement from './../../store/getToolsWithDataForElement
* @param {*} evt
*/
export default function(evt) {
if (state.isToolLocked) {
if (state.isToolLocked || state.isMultiPartToolActive) {
return;
}

Expand Down
5 changes: 5 additions & 0 deletions src/eventDispatchers/shared/customCallbackHandler.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { state } from './../../store/index.js';
import getActiveToolsForElement from './../../store/getActiveToolsForElement.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

export default function(handlerType, customFunction, evt) {
if (state.isToolLocked) {
Expand All @@ -17,6 +18,10 @@ export default function(handlerType, customFunction, evt) {
// Tool has expected callback custom function
tools = tools.filter(tool => typeof tool[customFunction] === 'function');

if (state.isMultiPartToolActive) {
tools = filterToolsUseableWithMultiPartTools(tools);
}

if (tools.length === 0) {
return false;
}
Expand Down
5 changes: 5 additions & 0 deletions src/eventDispatchers/shared/customCallbackHandler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import getActiveToolsForElement from './../../store/getActiveToolsForElement.js'
jest.mock('./../../store/index.js', () => ({
state: {
isToolLocked: false,
isMultiPartToolActive: false,
tools: [
{
aDifferentCustomFunctionName: jest.fn(),
Expand All @@ -27,6 +28,9 @@ jest.mock('./../../store/index.js', () => ({
}));

jest.mock('./../../store/getActiveToolsForElement.js', () => jest.fn());
jest.mock('./../../store/filterToolsUsableWithMultiPartTools.js', () =>
jest.fn()
);

describe('eventDispatchers/customCallbackHandler.js', () => {
const firstToolWithCustomFunction = state.tools[1];
Expand All @@ -39,6 +43,7 @@ describe('eventDispatchers/customCallbackHandler.js', () => {

beforeEach(() => {
state.isToolLocked = false;
state.isMultiPartToolActive = false;

jest.resetAllMocks();
jest.clearAllMocks();
Expand Down
5 changes: 5 additions & 0 deletions src/eventDispatchers/touchEventHandlers/multiTouchDrag.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { state } from '../../store/index.js';
import getActiveToolsForElement from '../../store/getActiveToolsForElement.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

export default function(evt) {
if (state.isToolLocked) {
Expand All @@ -21,6 +22,10 @@ export default function(evt) {
numPointers === tool.configuration.touchPointers
);

if (state.isMultiPartToolActive) {
tools = filterToolsUseableWithMultiPartTools(tools);
}

if (tools.length === 0) {
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/eventDispatchers/touchEventHandlers/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { moveHandle, moveAllHandles } from '../../manipulators/index.js';
import deactivateAllToolInstances from './shared/deactivateAllToolInstances.js';

export default function(evt) {
if (state.isToolLocked) {
if (state.isToolLocked || state.isMultiPartToolActive) {
return;
}

Expand Down
8 changes: 6 additions & 2 deletions src/eventDispatchers/touchEventHandlers/touchStart.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// State
import { getters, state } from '../../store/index.js';
// Import anyHandlesOutsideImage from '../manipulators/anyHandlesOutsideImage.js';
import { findHandleDataNearImagePoint } from '../../util/findAndMoveHelpers.js';
import getToolsWithMoveableHandles from '../../store/getToolsWithMoveableHandles.js';
import { getToolState } from './../../stateManagement/toolState.js';
import getInteractiveToolsForElement from './../../store/getInteractiveToolsForElement.js';
import getToolsWithDataForElement from './../../store/getToolsWithDataForElement.js';
import filterToolsUseableWithMultiPartTools from './../../store/filterToolsUsableWithMultiPartTools.js';

export default function(evt) {
if (state.isToolLocked) {
Expand All @@ -21,10 +21,14 @@ export default function(evt) {
getters.touchTools()
);

const activeTools = activeAndPassiveTools.filter(
let activeTools = activeAndPassiveTools.filter(
tool => tool.mode === 'active' && tool.options.isTouchActive
);

if (state.isMultiPartToolActive) {
activeTools = filterToolsUseableWithMultiPartTools(activeTools);
}

// If any tools are active, check if they have a special reason for dealing with the event.
if (activeTools.length > 0) {
// TODO: If length > 1, you could assess fitness and select the ideal tool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import addNewMeasurement from './addNewMeasurement.js';
import BaseAnnotationTool from './../../tools/base/BaseAnnotationTool.js';

export default function(evt) {
if (state.isToolLocked) {
if (state.isToolLocked || state.isMultiPartToolActive) {
return;
}

Expand Down
21 changes: 21 additions & 0 deletions src/store/filterToolsUsableWithMultiPartTools.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import BaseAnnotationTool from '../tools/base/BaseAnnotationTool.js';
import BaseBrushTool from '../tools/base/BaseBrushTool.js';

/**
* Filters an array of tools, returning only tools which are active or passive.
* @export
* @public
* @method
* @name filterToolsUseableWithMultiPartTools
*
* @param {Object[]} tools The input tool array.
* @returns {Object[]} The filtered array.
*/
export default function(tools) {
return tools.filter(
tool =>
!tool.isMultiPartTool &&
!(tool instanceof BaseAnnotationTool) &&
!(tool instanceof BaseBrushTool)
);
}
1 change: 1 addition & 0 deletions src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const state = {
enabledElements: [],
tools: [],
isToolLocked: false,
activeMultiPartTool: null,
mousePositionImage: {},
// Settings
clickProximity: 6,
Expand Down
5 changes: 3 additions & 2 deletions src/tools/FreehandSculpterMouseTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export default class FreehandSculpterMouseTool extends BaseTool {
super(initialConfiguration);

this.hasCursor = true;
this.isMultiPartTool = true;
this.initialConfiguration = initialConfiguration;
this.referencedToolName = initialConfiguration.referencedToolName;

Expand Down Expand Up @@ -153,7 +154,7 @@ export default class FreehandSculpterMouseTool extends BaseTool {

this._active = false;

state.isToolLocked = false;
state.isMultiPartToolActive = false;

this._getMouseLocation(eventData);
this._invalidateToolData(eventData);
Expand Down Expand Up @@ -335,7 +336,7 @@ export default class FreehandSculpterMouseTool extends BaseTool {
this._active = true;

// Interupt event dispatcher
state.isToolLocked = true;
state.isMultiPartToolActive = true;

this._configureToolSize(eventData);
this._getMouseLocation(eventData);
Expand Down
36 changes: 31 additions & 5 deletions src/tools/annotation/FreehandMouseTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
super(initialConfiguration);

this.initialConfiguration = initialConfiguration;
this.isMultiPartTool = true;

this._drawing = false;
this._dragging = false;
Expand Down Expand Up @@ -569,7 +570,7 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
this._activateModify(element);

// Interupt eventDispatchers
state.isToolLocked = true;
state.isMultiPartToolActive = true;

preventPropagation(evt);
}
Expand Down Expand Up @@ -624,6 +625,11 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
*/
_drawingMouseDragCallback(evt) {
const eventData = evt.detail;

if (!this.options.mouseButtonMask.includes(eventData.buttons)) {
return;
}

const toolState = getToolState(eventData.element, this.name);

const config = this.configuration;
Expand All @@ -649,13 +655,18 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
* @returns {undefined}
*/
_drawingMouseUpCallback(evt) {
const eventData = evt.detail;

if (!this.options.mouseButtonMask.includes(eventData.buttons)) {
return;
}

if (!this._dragging) {
return;
}

this._dragging = false;

const eventData = evt.detail;
const element = eventData.element;

const config = this.configuration;
Expand Down Expand Up @@ -683,6 +694,11 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
*/
_drawingMouseDownCallback(evt) {
const eventData = evt.detail;

if (!this.options.mouseButtonMask.includes(eventData.buttons)) {
return;
}

const element = eventData.element;
const coords = eventData.currentPoints.canvas;

Expand Down Expand Up @@ -715,6 +731,11 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
*/
_editMouseDragCallback(evt) {
const eventData = evt.detail;

if (!this.options.mouseButtonMask.includes(eventData.buttons)) {
return;
}

const toolState = getToolState(eventData.element, this.name);

const config = this.configuration;
Expand Down Expand Up @@ -771,6 +792,11 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
*/
_editMouseUpCallback(evt) {
const eventData = evt.detail;

if (!this.options.mouseButtonMask.includes(eventData.buttons)) {
return;
}

const element = eventData.element;
const toolState = getToolState(eventData.element, this.name);

Expand All @@ -779,7 +805,7 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
this._dropHandle(eventData, toolState);
this._endDrawing(element);

state.isToolLocked = false;
state.isMultiPartToolActive = false;

external.cornerstone.updateImage(eventData.element);
}
Expand Down Expand Up @@ -837,7 +863,7 @@ export default class FreehandMouseTool extends BaseAnnotationTool {
const config = this.configuration;

// Block event distpatcher whilst drawing
state.isToolLocked = true;
state.isMultiPartToolActive = true;

this._referencedElement = element;

Expand Down Expand Up @@ -949,7 +975,7 @@ export default class FreehandMouseTool extends BaseAnnotationTool {

if (this._drawing) {
this._drawing = false;
state.isToolLocked = false;
state.isMultiPartToolActive = false;
this._deactivateDraw(element);
}

Expand Down

0 comments on commit 53bf96f

Please sign in to comment.