mirror of
https://github.com/openfrontio/OpenFrontIO.git
synced 2026-06-21 07:50:45 +00:00
Fix Ctrl+Click for Macs, and alphabetize gitignore (#2720)
Resolves #2719 - [X] Ran `npm run format` - [X] Ran `npm run lint:fix` - [X] Ran `npm test` - [X] Manually tested fixes ## Description: ### What problem(s) was I solving? On Mac, `Ctrl+Click` is commonly used as a substitute for right-click to open context menus. However, in OpenFront, `Ctrl+Click` was triggering both the context menu, AND causing an attack. This effectively prevented Mac users from being able to ally with bots/nations, as trying to ally would cause you to attack them. ### What changes did I make? - `Ctrl+Click` on Mac no longer triggers an attack, allowing players to properly interact with the alliance system ### How I implemented it 1. Added an `isMac()` method to `InputHandler` which encapsulates the "is mac" logic, and lets it be mocked during tests 2. In the `onPointerUp` handler, added a check: if on Mac and `ControlLeft` is held, emit a `ContextMenuEvent`, and then return (instead of continuing to also create a `MouseUpEvent`) 3. Extracted magic numbers for mouse button checks into descriptive helper methods (`isMiddleMouseButton`, `isNonLeftMouseButton`) for improved code clarity 4. Added clarifying comments throughout the pointer event handlers Last, alphabetized the `.gitignore` file and organized it into "Folders" and "Files" sections to make it easier to read. ### How to verify it #### Manual Testing - [X] On a Mac, hold `Ctrl` and left-click on another nation - verify the context menu opens (not an attack) - [X] On a Mac, right-click should still open the context menu as expected - [X] On Windows/Linux, `Ctrl+Click` continue to work as before (modifier key for build menu if configured) - [X] Regular left-click still triggers attacks/interactions as expected #### Automated Testing - [x] New unit test added: `Mac Ctrl+Click Context Menu` - verifies that `Ctrl+Click` on Mac emits `ContextMenuEvent` instead of `MouseUpEvent` ### Description for the changelog Fixed `Ctrl+Click` on Mac to properly open the context menu instead of triggering an attack, restoring the ability for Mac users to form alliances with other nations. ## Please complete the following: - [X] I have added screenshots for all UI updates - [X] I process any text displayed to the user through translateText() and I've added it to the en.json file - [X] I have added relevant tests to the test directory - [X] I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced ## Please put your Discord username so you can be contacted if a bug or regression is found: Terekhov
This commit is contained in:
+16
-10
@@ -1,15 +1,21 @@
|
||||
# Folders
|
||||
.claude/
|
||||
.clinic/
|
||||
.idea/
|
||||
build/
|
||||
coverage/
|
||||
node_modules/
|
||||
out/
|
||||
static/
|
||||
coverage/
|
||||
TODO.txt
|
||||
resources/images/.DS_Store
|
||||
resources/.DS_Store
|
||||
.env*
|
||||
.DS_Store
|
||||
.clinic/
|
||||
CLAUDE.md
|
||||
.idea/
|
||||
|
||||
# this is autogenerated by script
|
||||
src/assets/
|
||||
|
||||
static/
|
||||
|
||||
# Files
|
||||
.DS_Store
|
||||
.env*
|
||||
CLAUDE.md
|
||||
resources/images/.DS_Store
|
||||
resources/.DS_Store
|
||||
TODO.txt
|
||||
|
||||
+40
-10
@@ -153,6 +153,12 @@ export class InputHandler {
|
||||
private activeKeys = new Set<string>();
|
||||
private keybinds: Record<string, string> = {};
|
||||
|
||||
private readonly isMacReal = /Mac/.test(navigator.userAgent);
|
||||
isMac(): boolean {
|
||||
// Method exists so we can mock during tests
|
||||
return this.isMacReal;
|
||||
}
|
||||
|
||||
private readonly PAN_SPEED = 5;
|
||||
private readonly ZOOM_SPEED = 10;
|
||||
|
||||
@@ -186,9 +192,6 @@ export class InputHandler {
|
||||
console.warn("Invalid keybinds JSON:", e);
|
||||
}
|
||||
|
||||
// Mac users might have different keybinds
|
||||
const isMac = /Mac/.test(navigator.userAgent);
|
||||
|
||||
this.keybinds = {
|
||||
toggleView: "Space",
|
||||
centerCamera: "KeyC",
|
||||
@@ -202,8 +205,9 @@ export class InputHandler {
|
||||
attackRatioUp: "KeyY",
|
||||
boatAttack: "KeyB",
|
||||
groundAttack: "KeyG",
|
||||
// Mac users might have different keybinds
|
||||
modifierKey: this.isMac() ? "MetaLeft" : "ControlLeft",
|
||||
swapDirection: "KeyU",
|
||||
modifierKey: isMac ? "MetaLeft" : "ControlLeft",
|
||||
altKey: "AltLeft",
|
||||
buildCity: "Digit1",
|
||||
buildFactory: "Digit2",
|
||||
@@ -448,13 +452,15 @@ export class InputHandler {
|
||||
}
|
||||
|
||||
private onPointerDown(event: PointerEvent) {
|
||||
if (event.button === 1) {
|
||||
// Handle middle mouse button (wheel click) for auto-upgrade
|
||||
if (this.isMiddleMouseButton(event.button)) {
|
||||
event.preventDefault();
|
||||
this.eventBus.emit(new AutoUpgradeEvent(event.clientX, event.clientY));
|
||||
return;
|
||||
}
|
||||
|
||||
if (event.button > 0) {
|
||||
// Ignore right mouse button and other non-left buttons
|
||||
if (!this.isLeftMouseButton(event.button)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -475,12 +481,14 @@ export class InputHandler {
|
||||
}
|
||||
|
||||
onPointerUp(event: PointerEvent) {
|
||||
if (event.button === 1) {
|
||||
// Prevent default behavior for middle mouse button, but don't process further
|
||||
if (this.isMiddleMouseButton(event.button)) {
|
||||
event.preventDefault();
|
||||
return;
|
||||
}
|
||||
|
||||
if (event.button > 0) {
|
||||
// Ignore right mouse button and other non-left buttons
|
||||
if (!this.isLeftMouseButton(event.button)) {
|
||||
return;
|
||||
}
|
||||
this.pointerDown = false;
|
||||
@@ -490,15 +498,27 @@ export class InputHandler {
|
||||
this.eventBus.emit(new ShowBuildMenuEvent(event.clientX, event.clientY));
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.isAltKeyPressed(event)) {
|
||||
this.eventBus.emit(new ShowEmojiMenuEvent(event.clientX, event.clientY));
|
||||
return;
|
||||
}
|
||||
|
||||
// If Ctrl is held down (for example, on a Mac which doesn't have a right-click)
|
||||
// then Ctrl+Click is used to open context menu.
|
||||
//
|
||||
// Without this conditional, Ctrl+click causes the player to attack someone
|
||||
// if they are not allied, effectively removing ability to ally with bots/nations.
|
||||
if (this.isMac() && this.activeKeys.has("ControlLeft")) {
|
||||
this.eventBus.emit(new ContextMenuEvent(event.clientX, event.clientY));
|
||||
return;
|
||||
}
|
||||
|
||||
const dist =
|
||||
Math.abs(event.x - this.lastPointerDownX) +
|
||||
Math.abs(event.y - this.lastPointerDownY);
|
||||
if (dist < 10) {
|
||||
// Handle touch events separately (prevents both touch and click events from firing)
|
||||
if (event.pointerType === "touch") {
|
||||
this.eventBus.emit(new TouchEvent(event.x, event.y));
|
||||
event.preventDefault();
|
||||
@@ -532,17 +552,18 @@ export class InputHandler {
|
||||
}
|
||||
|
||||
private onPointerMove(event: PointerEvent) {
|
||||
if (event.button === 1) {
|
||||
if (this.isMiddleMouseButton(event.button)) {
|
||||
event.preventDefault();
|
||||
return;
|
||||
}
|
||||
|
||||
if (event.button > 0) {
|
||||
if (!this.isLeftMouseButton(event.button)) {
|
||||
return;
|
||||
}
|
||||
|
||||
this.pointers.set(event.pointerId, event);
|
||||
|
||||
// When not dragging, just track mouse position for hover effects
|
||||
if (!this.pointerDown) {
|
||||
this.eventBus.emit(new MouseOverEvent(event.clientX, event.clientY));
|
||||
return;
|
||||
@@ -572,6 +593,7 @@ export class InputHandler {
|
||||
|
||||
private onContextMenu(event: MouseEvent) {
|
||||
event.preventDefault();
|
||||
// If placing a structure, right-click cancels instead of opening context menu
|
||||
if (this.uiState.ghostStructure !== null) {
|
||||
this.setGhostStructure(null);
|
||||
return;
|
||||
@@ -639,4 +661,12 @@ export class InputHandler {
|
||||
(this.keybinds.altKey === "MetaLeft" && event.metaKey)
|
||||
);
|
||||
}
|
||||
|
||||
private isMiddleMouseButton(button: number): boolean {
|
||||
return button === 1;
|
||||
}
|
||||
|
||||
private isLeftMouseButton(button: number): boolean {
|
||||
return button === 0;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
import { AutoUpgradeEvent, InputHandler } from "../src/client/InputHandler";
|
||||
import {
|
||||
AutoUpgradeEvent,
|
||||
ContextMenuEvent,
|
||||
InputHandler,
|
||||
MouseUpEvent,
|
||||
} from "../src/client/InputHandler";
|
||||
import { EventBus } from "../src/core/EventBus";
|
||||
|
||||
class MockPointerEvent {
|
||||
@@ -452,4 +457,46 @@ describe("InputHandler AutoUpgrade", () => {
|
||||
spy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("Mac Ctrl+Click Context Menu", () => {
|
||||
test("should create context menu with Ctrl+Click on Mac, but not attack", () => {
|
||||
// Mock isMac() to return true
|
||||
vi.spyOn(inputHandler as any, "isMac").mockReturnValue(true);
|
||||
const mockEmit = vi.spyOn(eventBus, "emit");
|
||||
|
||||
// Simulate ControlLeft being held
|
||||
inputHandler["activeKeys"].add("ControlLeft");
|
||||
|
||||
// Simulate a pointer down first (to set pointerDown state)
|
||||
const pointerDownEvent = new PointerEvent("pointerdown", {
|
||||
button: 0,
|
||||
clientX: 100,
|
||||
clientY: 200,
|
||||
pointerId: 1,
|
||||
});
|
||||
inputHandler["onPointerDown"](pointerDownEvent);
|
||||
mockEmit.mockClear();
|
||||
|
||||
// Now trigger pointer up
|
||||
const pointerUpEvent = new PointerEvent("pointerup", {
|
||||
button: 0,
|
||||
clientX: 100,
|
||||
clientY: 200,
|
||||
pointerId: 1,
|
||||
});
|
||||
inputHandler["onPointerUp"](pointerUpEvent);
|
||||
|
||||
// Verify ContextMenuEvent was emitted with correct coordinates
|
||||
expect(mockEmit).toHaveBeenCalledTimes(1);
|
||||
// If MouseUp is fired, that would cause an attack - which we do not want.
|
||||
expect(mockEmit).not.toHaveBeenCalledWith(expect.any(MouseUpEvent));
|
||||
expect(mockEmit).toHaveBeenCalledWith(expect.any(ContextMenuEvent));
|
||||
expect(mockEmit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
x: 100,
|
||||
y: 200,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user