From 1dac7bd2e82ce935a2013f2adbc31817acd02499 Mon Sep 17 00:00:00 2001 From: FloPinguin <25036848+FloPinguin@users.noreply.github.com> Date: Wed, 28 Jan 2026 00:00:18 +0100 Subject: [PATCH] =?UTF-8?q?Confirm=20alliance=20break=20=E2=9A=A0=EF=B8=8F?= =?UTF-8?q?=20(#3033)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description: People accidentally clicked the betray button because it's at the same position as the ally button. So let's add a small confirmation step. https://github.com/user-attachments/assets/754f2d33-7419-42fc-a732-197c3107236e ## 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: FloPinguin --- resources/images/CheckmarkIconWhite.svg | 3 ++ src/client/graphics/layers/RadialMenu.ts | 5 ++- .../graphics/layers/RadialMenuElements.ts | 21 ++++++++++ ...nts.spec.ts => radialMenuElements.test.ts} | 42 ++++++++++++++++++- 4 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 resources/images/CheckmarkIconWhite.svg rename tests/{radialMenuElements.spec.ts => radialMenuElements.test.ts} (66%) diff --git a/resources/images/CheckmarkIconWhite.svg b/resources/images/CheckmarkIconWhite.svg new file mode 100644 index 000000000..ef1abfe12 --- /dev/null +++ b/resources/images/CheckmarkIconWhite.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/client/graphics/layers/RadialMenu.ts b/src/client/graphics/layers/RadialMenu.ts index 090df03c1..ab4d0198f 100644 --- a/src/client/graphics/layers/RadialMenu.ts +++ b/src/client/graphics/layers/RadialMenu.ts @@ -907,9 +907,12 @@ export class RadialMenu implements Layer { .select(".center-button-hitbox") .style("cursor", enabled ? "pointer" : "not-allowed"); + // Use default color for back button, otherwise use the current center button color + const buttonColor = + state === "back" ? this.defaultCenterButtonColor : this.centerButtonColor; centerButton .select(".center-button-visible") - .attr("fill", enabled ? this.centerButtonColor : "#999999"); + .attr("fill", enabled ? buttonColor : "#999999"); centerButton .select(".center-button-icon") diff --git a/src/client/graphics/layers/RadialMenuElements.ts b/src/client/graphics/layers/RadialMenuElements.ts index 0cf28cebb..4ee7e5924 100644 --- a/src/client/graphics/layers/RadialMenuElements.ts +++ b/src/client/graphics/layers/RadialMenuElements.ts @@ -17,6 +17,7 @@ import allianceIcon from "/images/AllianceIconWhite.svg?url"; import boatIcon from "/images/BoatIconWhite.svg?url"; import buildIcon from "/images/BuildIconWhite.svg?url"; import chatIcon from "/images/ChatIconWhite.svg?url"; +import checkmarkIcon from "/images/CheckmarkIconWhite.svg?url"; import donateGoldIcon from "/images/DonateGoldIconWhite.svg?url"; import donateTroopIcon from "/images/DonateTroopIconWhite.svg?url"; import emojiIcon from "/images/EmojiIconWhite.svg?url"; @@ -218,6 +219,15 @@ const allyBreakElement: MenuElement = { !!params.playerActions?.interaction?.canBreakAlliance, color: COLORS.breakAlly, icon: traitorIcon, + subMenu: () => [allyBreakCancelElement, allyBreakConfirmElement], +}; + +const allyBreakConfirmElement: MenuElement = { + id: "ally_break_confirm", + name: "confirm", + disabled: () => false, + color: COLORS.breakAlly, + icon: checkmarkIcon, action: (params: MenuElementParams) => { params.playerActionHandler.handleBreakAlliance( params.myPlayer, @@ -227,6 +237,17 @@ const allyBreakElement: MenuElement = { }, }; +const allyBreakCancelElement: MenuElement = { + id: "ally_break_cancel", + name: "cancel", + disabled: () => false, + color: COLORS.info, + icon: xIcon, + action: (params: MenuElementParams) => { + params.closeMenu(); + }, +}; + // eslint-disable-next-line @typescript-eslint/no-unused-vars const allyDonateGoldElement: MenuElement = { id: "ally_donate_gold", diff --git a/tests/radialMenuElements.spec.ts b/tests/radialMenuElements.test.ts similarity index 66% rename from tests/radialMenuElements.spec.ts rename to tests/radialMenuElements.test.ts index c24ea9227..a95e6be05 100644 --- a/tests/radialMenuElements.spec.ts +++ b/tests/radialMenuElements.test.ts @@ -75,6 +75,12 @@ const makeParams = (opts?: Partial): MenuElementParams => { const findAllyBreak = (items: any[]) => items.find((i) => i && i.id === "ally_break"); +const findAllyBreakConfirm = (items: any[]) => + items.find((i) => i && i.id === "ally_break_confirm"); + +const findAllyBreakCancel = (items: any[]) => + items.find((i) => i && i.id === "ally_break_cancel"); + describe("RadialMenuElements ally break", () => { test("shows break option with correct color when allied", () => { const params = makeParams(); @@ -85,12 +91,29 @@ describe("RadialMenuElements ally break", () => { expect(ally.color).toBe(COLORS.breakAlly); }); - test("action calls handleBreakAlliance and closes menu", () => { + test("break option opens confirmation submenu", () => { const params = makeParams(); const items = rootMenuElement.subMenu!(params); const ally = findAllyBreak(items)!; - ally.action!(params); + expect(ally.subMenu).toBeDefined(); + const subMenuItems = ally.subMenu!(params); + expect(subMenuItems.length).toBe(2); + + const confirmItem = findAllyBreakConfirm(subMenuItems); + const cancelItem = findAllyBreakCancel(subMenuItems); + expect(confirmItem).toBeTruthy(); + expect(cancelItem).toBeTruthy(); + }); + + test("confirm action calls handleBreakAlliance and closes menu", () => { + const params = makeParams(); + const items = rootMenuElement.subMenu!(params); + const ally = findAllyBreak(items)!; + const subMenuItems = ally.subMenu!(params); + const confirmItem = findAllyBreakConfirm(subMenuItems)!; + + confirmItem.action!(params); expect(params.playerActionHandler.handleBreakAlliance).toHaveBeenCalledWith( params.myPlayer, @@ -98,4 +121,19 @@ describe("RadialMenuElements ally break", () => { ); expect(params.closeMenu).toHaveBeenCalled(); }); + + test("cancel action closes menu without breaking alliance", () => { + const params = makeParams(); + const items = rootMenuElement.subMenu!(params); + const ally = findAllyBreak(items)!; + const subMenuItems = ally.subMenu!(params); + const cancelItem = findAllyBreakCancel(subMenuItems)!; + + cancelItem.action!(params); + + expect( + params.playerActionHandler.handleBreakAlliance, + ).not.toHaveBeenCalled(); + expect(params.closeMenu).toHaveBeenCalled(); + }); });