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.

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

Modularize how events get turned into behavior #44

Open
nickswalker opened this issue May 25, 2022 · 0 comments
Open

Modularize how events get turned into behavior #44

nickswalker opened this issue May 25, 2022 · 0 comments
Labels
enhancement New feature or request long-term Not a high priority, noted down for remembering later

Comments

@nickswalker
Copy link
Member

nickswalker commented May 25, 2022

As motivating context, here is how a mousedown event is interpreted for the overhead display overlay:

overheadControl.addEventListener("mousedown", (event: MouseEvent) => {
let x = event.offsetX;
let y = event.offsetY;
mouseMoveX = x;
mouseMoveY = y;
// Remove old event handlers
overheadControl.removeEventListener('mousemove', updateAction);
overheadControl.removeEventListener('mousemove', drawTraj);
let namespace = 'nav'
let mode = this.refs.get("mode-toggle")!.querySelector("input[type=radio]:checked")!.value;
if (this.model.getSetting("displayMode", namespace) === "predictive-display" && mode === 'nav') {
if (this.model.getSetting("actionMode", namespace) === "control-continuous") {
// Update trajectory when mouse moves
overheadControl.addEventListener("mousemove", updateAction);
if (this.model.getSetting("startStopMode", namespace) === "press-release") {
// Execute trajectory as long as mouse is held down using last position of cursor
// Wait for predictive trajectory calculation to complete before creating mouseup
// event (handles repeated clicking)
const promise = new Promise((resolve, reject) => {
if (!this.activeVelocityAction) {
this.velocityExecutionHeartbeat = window.setInterval(() => {
overheadClickNavOverlay.removeTraj();
this.drawAndExecuteTraj(mouseMoveX, mouseMoveY, overheadClickNavOverlay)
}, 100)
resolve()
}
reject()
})
promise.then(() => document.body.addEventListener("mouseup", stopAction));
}
// Click-click mode: if action is being executed stop it
else if (this.activeVelocityAction) {
stopAction(event);
}
// Click-click mode: if no action start new action
else {
document.body.removeEventListener('mouseup', stopAction);
this.velocityExecutionHeartbeat = window.setInterval(() => {
overheadClickNavOverlay.removeTraj();
this.drawAndExecuteTraj(mouseMoveX, mouseMoveY, overheadClickNavOverlay)
}, 10)
}
} else {
// action mode is incremental/step actions
// execute trajectory once
this.drawAndExecuteTraj(x, y, overheadClickNavOverlay);
setTimeout(() => {
overheadClickNavOverlay.resetTraj(this.model.getSetting("colorblindMode"))
}, 1500);
overheadControl.addEventListener("mousemove", drawTraj)
}
}
});

Note how the behavior is different depending on how the user has configured the control. This event happens to only be needed if the control is configured to show the predictive navigation overlay, so all of the behavior is behind a guard on this condition. Further, the exact response to the event is governed by the "action-mode" the user has set. If it's press-release, then we need to listen for a release event so we can stop the action. If it's click-click, then we need to determine which click the event represents (the initiating or the terminating click), and then act appropriately.

The behavior for all "overlay modes" and all intersecting "action modes" are tangled here simply because we'd prefer to only register a single listener for the mouse down event. But his tangling creates significant potential for bugs (since code must be conditioned carefully to fire only in particular configurations), makes bugs harder to catch (since they're buried amongst a mass of code that doesn't or shouldn't fire). This makes it difficult to introduce new modes.

We should disentangle listening for an event from actually processing it. The simplest refactoring can be to isolate how the current handler would behave for each and every given mode permutation and create separate handler functions that represent each of them. Whenever the user changes their configuration, we can point the event listener at the appropriate handler function. These individual handlers will be easier to write and to maintain. There are places where a handler registers a second handler temporarily. These would need to be changed to a small class that is registered as the handler for both events, then internally determines when and how it should respond to either of them.

Many of these handlers share a common structure (e.g. they trigger a behavior on the first event, then disable it the second or whenever some other event is fired), so in the long term we'd want a parametric representation so we can compactly specify them. The state machine representation we've chatted about seems like the cleanest way here. We'd want to be able to write small, arbitrary state machine templates that could be instantiated for each UI component based on the user configuration. Once configured, each state machine declares what events it needs to consume and someone on the outside is responsible for feeding these to it. It governs how the UI component it owns is transformed in response to events as well as what commands get emitted.

@nickswalker nickswalker added enhancement New feature or request long-term Not a high priority, noted down for remembering later labels May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request long-term Not a high priority, noted down for remembering later
Projects
None yet
Development

No branches or pull requests

1 participant