Skip to content

Use custom attack ratio slider for consistent cross-browser rendering #3422

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.

Sign up for GitHub

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

Closed
hkio120 wants to merge 5 commits into from
Closed

Use custom attack ratio slider for consistent cross-browser rendering #3422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ae2b4fa
Use custom attack ratio slider for consistent cross-browser rendering
hkio120 Mar 13, 2026
5599737
Add keyboard accessibility to custom attack ratio slider
hkio120 Mar 14, 2026
3af40fc
Keep slider PR focused on visual consistency only
hkio120 Mar 14, 2026
5071d81
refactor: extract attack ratio slider into Lit element
hkio120 Mar 20, 2026
e900e81
refactor: reuse FluentSlider for attack ratio control
hkio120 Mar 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 53 additions & 18 deletions src/client/components/FluentSlider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { LitElement, html } from "lit";
import { html, LitElement } from "lit";
import { customElement, property, query, state } from "lit/decorators.js";
import { ifDefined } from "lit/directives/if-defined.js";
import { translateText } from "../Utils";

@customElement("fluent-slider")
Expand All @@ -16,6 +17,11 @@ export class FluentSlider extends LitElement {
@property({ type: String }) disabledKey = "";
@property({ type: Number }) defaultValue: number | undefined = undefined;
@property({ type: String }) defaultLabelKey = "";
@property({ type: Boolean }) compact = false;
@property({ type: Boolean }) inline = false;
@property({ type: Boolean }) live = false;
@property({ type: String, attribute: "aria-label" }) ariaLabel = "";
@property({ type: String, attribute: "aria-value-text" }) ariaValueText = "";

@state() private isEditing = false;

Expand All @@ -34,12 +40,17 @@ export class FluentSlider extends LitElement {
private handleSliderInput(e: Event) {
const target = e.target as HTMLInputElement;
this.value = target.valueAsNumber;
if (this.live) {
this.dispatchValueChange();
}
}

private handleSliderChange(e: Event) {
const target = e.target as HTMLInputElement;
this.value = target.valueAsNumber;
this.dispatchValueChange();
if (!this.live) {
this.dispatchValueChange();
}
}

private handleNumberInput(e: Event) {
Expand Down Expand Up @@ -71,30 +82,54 @@ export class FluentSlider extends LitElement {
this.updateComplete.then(() => this.numberInput?.focus());
}

render() {
private get sliderAriaLabel() {
if (this.ariaLabel) return this.ariaLabel;
if (this.labelKey) return translateText(this.labelKey);
return "Slider";
}

private renderSliderInput() {
const percentage =
this.max === this.min
? 0
: ((this.value - this.min) / (this.max - this.min)) * 100;
const trackHeight = this.inline ? (this.compact ? 6 : 8) : 6;

const sliderClass = this.inline
? this.compact
? "w-full h-4 p-0 m-0 bg-transparent appearance-none cursor-pointer focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-400/60 rounded-full [&::-webkit-slider-runnable-track]:w-full [&::-webkit-slider-runnable-track]:h-[6px] [&::-webkit-slider-runnable-track]:cursor-pointer [&::-webkit-slider-runnable-track]:bg-transparent [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:transition-colors [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:h-[12px] [&::-webkit-slider-thumb]:w-[12px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-blue-500 [&::-webkit-slider-thumb]:cursor-pointer [&::-webkit-slider-thumb]:-mt-[3px] [&::-webkit-slider-thumb]:shadow-[0_0_0_3px_rgba(59,130,246,0.18)] [&::-webkit-slider-thumb]:transition-all active:[&::-webkit-slider-thumb]:scale-110 [&::-moz-range-track]:w-full [&::-moz-range-track]:h-[6px] [&::-moz-range-track]:cursor-pointer [&::-moz-range-track]:bg-transparent [&::-moz-range-track]:rounded-full [&::-moz-range-thumb]:h-[12px] [&::-moz-range-thumb]:w-[12px] [&::-moz-range-thumb]:border-none [&::-moz-range-thumb]:rounded-full [&::-moz-range-thumb]:bg-blue-500 [&::-moz-range-thumb]:cursor-pointer [&::-moz-range-thumb]:shadow-[0_0_0_3px_rgba(59,130,246,0.18)]"
: "w-full h-5 p-0 m-0 bg-transparent appearance-none cursor-pointer focus:outline-none focus-visible:ring-2 focus-visible:ring-blue-400/60 rounded-full [&::-webkit-slider-runnable-track]:w-full [&::-webkit-slider-runnable-track]:h-[8px] [&::-webkit-slider-runnable-track]:cursor-pointer [&::-webkit-slider-runnable-track]:bg-transparent [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:transition-colors [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:h-[14px] [&::-webkit-slider-thumb]:w-[14px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-blue-500 [&::-webkit-slider-thumb]:cursor-pointer [&::-webkit-slider-thumb]:-mt-[3px] [&::-webkit-slider-thumb]:shadow-[0_0_0_3px_rgba(59,130,246,0.18)] [&::-webkit-slider-thumb]:transition-all active:[&::-webkit-slider-thumb]:scale-110 [&::-moz-range-track]:w-full [&::-moz-range-track]:h-[8px] [&::-moz-range-track]:cursor-pointer [&::-moz-range-track]:bg-transparent [&::-moz-range-track]:rounded-full [&::-moz-range-thumb]:h-[14px] [&::-moz-range-thumb]:w-[14px] [&::-moz-range-thumb]:border-none [&::-moz-range-thumb]:rounded-full [&::-moz-range-thumb]:bg-blue-500 [&::-moz-range-thumb]:cursor-pointer [&::-moz-range-thumb]:shadow-[0_0_0_3px_rgba(59,130,246,0.18)]"
: "w-full h-6 p-0 m-0 bg-transparent appearance-none cursor-pointer focus:outline-none [&::-webkit-slider-runnable-track]:w-full [&::-webkit-slider-runnable-track]:h-[6px] [&::-webkit-slider-runnable-track]:cursor-pointer [&::-webkit-slider-runnable-track]:bg-transparent [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:transition-colors [&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:h-[18px] [&::-webkit-slider-thumb]:w-[18px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-blue-500 [&::-webkit-slider-thumb]:cursor-pointer [&::-webkit-slider-thumb]:-mt-[6px] [&::-webkit-slider-thumb]:shadow-[0_0_0_4px_rgba(59,130,246,0.2)] [&::-webkit-slider-thumb]:transition-all active:[&::-webkit-slider-thumb]:scale-110 active:[&::-webkit-slider-thumb]:shadow-[0_0_0_6px_rgba(59,130,246,0.3)] [&::-moz-range-track]:w-full [&::-moz-range-track]:h-[6px] [&::-moz-range-track]:cursor-pointer [&::-moz-range-track]:bg-transparent [&::-moz-range-track]:rounded-full [&::-moz-range-track]:transition-colors [&::-moz-range-thumb]:h-[18px] [&::-moz-range-thumb]:w-[18px] [&::-moz-range-thumb]:border-none [&::-moz-range-thumb]:rounded-full [&::-moz-range-thumb]:bg-blue-500 [&::-moz-range-thumb]:cursor-pointer [&::-moz-range-thumb]:shadow-[0_0_0_4px_rgba(59,130,246,0.2)] [&::-moz-range-thumb]:transition-all active:[&::-moz-range-thumb]:scale-110 active:[&::-moz-range-thumb]:shadow-[0_0_0_6px_rgba(59,130,246,0.3)]";

return html`
<input
type="range"
.min=${this.min}
.max=${this.max}
.step=${this.step}
.valueAsNumber=${this.value}
aria-label=${this.sliderAriaLabel}
aria-valuetext=${ifDefined(this.ariaValueText || undefined)}
style="background: linear-gradient(to right, #3b82f6 0%, #3b82f6 ${percentage}%, rgba(255, 255, 255, 0.15) ${percentage}%, rgba(255, 255, 255, 0.15) 100%); background-size: 100% ${trackHeight}px; background-repeat: no-repeat; background-position: center; border-radius: 9999px;"
class=${sliderClass}
@input=${this.handleSliderInput}
@change=${this.handleSliderChange}
/>
`;
}

render() {
if (this.inline) {
return html`<div class="w-full min-w-0">
${this.renderSliderInput()}
</div>`;
}

return html`
<div
class="flex flex-col items-center justify-center gap-1 w-full text-center"
>
<input
type="range"
.min=${this.min}
.max=${this.max}
.step=${this.step}
.valueAsNumber=${this.value}
style="background: linear-gradient(to right, #3b82f6 0%, #3b82f6 ${percentage}%, rgba(255, 255, 255, 0.15) ${percentage}%, rgba(255, 255, 255, 0.15) 100%); background-size: 100% 6px; background-repeat: no-repeat; background-position: center; border-radius: 9999px;"
class="w-full h-6 p-0 m-0 bg-transparent appearance-none cursor-pointer focus:outline-none
[&::-webkit-slider-runnable-track]:w-full [&::-webkit-slider-runnable-track]:h-[6px] [&::-webkit-slider-runnable-track]:cursor-pointer [&::-webkit-slider-runnable-track]:bg-transparent [&::-webkit-slider-runnable-track]:rounded-full [&::-webkit-slider-runnable-track]:transition-colors
[&::-webkit-slider-thumb]:appearance-none [&::-webkit-slider-thumb]:h-[18px] [&::-webkit-slider-thumb]:w-[18px] [&::-webkit-slider-thumb]:rounded-full [&::-webkit-slider-thumb]:bg-blue-500 [&::-webkit-slider-thumb]:cursor-pointer [&::-webkit-slider-thumb]:-mt-[6px] [&::-webkit-slider-thumb]:shadow-[0_0_0_4px_rgba(59,130,246,0.2)] [&::-webkit-slider-thumb]:transition-all active:[&::-webkit-slider-thumb]:scale-110 active:[&::-webkit-slider-thumb]:shadow-[0_0_0_6px_rgba(59,130,246,0.3)]
[&::-moz-range-track]:w-full [&::-moz-range-track]:h-[6px] [&::-moz-range-track]:cursor-pointer [&::-moz-range-track]:bg-transparent [&::-moz-range-track]:rounded-full [&::-moz-range-track]:transition-colors
[&::-moz-range-thumb]:h-[18px] [&::-moz-range-thumb]:w-[18px] [&::-moz-range-thumb]:border-none [&::-moz-range-thumb]:rounded-full [&::-moz-range-thumb]:bg-blue-500 [&::-moz-range-thumb]:cursor-pointer [&::-moz-range-thumb]:shadow-[0_0_0_4px_rgba(59,130,246,0.2)] [&::-moz-range-thumb]:transition-all active:[&::-moz-range-thumb]:scale-110 active:[&::-moz-range-thumb]:shadow-[0_0_0_6px_rgba(59,130,246,0.3)]"
@input=${this.handleSliderInput}
@change=${this.handleSliderChange}
/>
${this.renderSliderInput()}
<div
class="text-xs uppercase font-bold tracking-wider text-center w-full leading-tight mb-1 flex flex-col items-center ${this
.value > 0
Expand Down
57 changes: 31 additions & 26 deletions src/client/graphics/layers/ControlPanel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { EventBus } from "../../../core/EventBus";
import { Gold } from "../../../core/game/Game";
import { GameView } from "../../../core/game/GameView";
import { ClientID } from "../../../core/Schemas";
import "../../components/FluentSlider";
import { AttackRatioEvent } from "../../InputHandler";
import { renderNumber, renderTroops } from "../../Utils";
import { UIState } from "../UIState";
Expand Down Expand Up @@ -69,8 +70,7 @@ export class ControlPanel extends LitElement implements Layer {
newAttackRatio = 0.1;
}

this.attackRatio = newAttackRatio;
this.onAttackRatioChange(this.attackRatio);
this.setAttackRatio(newAttackRatio);
});
}

Expand Down Expand Up @@ -124,15 +124,36 @@ export class ControlPanel extends LitElement implements Layer {
this.requestUpdate();
}

private handleRatioSliderInput(e: Event) {
const input = e.target as HTMLInputElement;
const value = Number(input.value);
this.attackRatio = value / 100;
private setAttackRatio(newRatio: number) {
const clamped = Math.min(1, Math.max(0.01, newRatio));
this.attackRatio = clamped;
localStorage.setItem("settings.attackRatio", String(this.attackRatio));
this.onAttackRatioChange(this.attackRatio);
this.requestUpdate();
}
Comment on lines +127 to +133
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard setAttackRatio against non-finite input and redundant writes.

Line 128 clamps range, but NaN still passes through as NaN, then Line 130 persists "NaN" and UI math can break. Also, unchanged values still trigger storage/event/update churn.

Suggested hardening 
  private setAttackRatio(newRatio: number) {
-    const clamped = Math.min(1, Math.max(0.01, newRatio));
-    this.attackRatio = clamped;
-    localStorage.setItem("settings.attackRatio", String(this.attackRatio));
-    this.onAttackRatioChange(this.attackRatio);
-    this.requestUpdate();
+    const safeRatio = Number.isFinite(newRatio) ? newRatio : 0.2;
+    const clamped = Math.min(1, Math.max(0.01, safeRatio));
+    if (clamped === this.attackRatio) return;
+    this.attackRatio = clamped;
+    localStorage.setItem("settings.attackRatio", String(clamped));
+    this.onAttackRatioChange(clamped);
+    this.requestUpdate();
  }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/client/graphics/layers/ControlPanel.ts` around lines 127 - 133, The
setAttackRatio method must reject non-finite inputs and avoid redundant side
effects: first check Number.isFinite(newRatio) and return early if not to
prevent persisting "NaN"; compute clamped = Math.min(1, Math.max(0.01,
newRatio)) and compare to this.attackRatio, and if unchanged return early to
avoid unnecessary localStorage.setItem("settings.attackRatio", ...),
this.onAttackRatioChange(...) and this.requestUpdate(); only when the clamped
value is finite and different from the current this.attackRatio, assign it,
persist the stringified value, call onAttackRatioChange(this.attackRatio) and
requestUpdate().


private handleRatioSliderPointerUp(e: Event) {
(e.target as HTMLInputElement).blur();
private handleAttackRatioSliderChange = (
e: CustomEvent<{ value: number }>,
) => {
this.setAttackRatio(e.detail.value);
};
private renderCustomAttackRatioSlider(compact: boolean = false) {
Copy link
Copy Markdown
Collaborator

@evanpelle evanpelle Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make the slider its own lit element? Then we can reuse it for other things

Copy link
Copy Markdown
Contributor Author

@hkio120 hkio120 Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done is it good ?

const ratioPercent = Math.max(1, Math.min(100, this.attackRatio * 100));

return html`
<fluent-slider
.value=${this.attackRatio}
.min=${0.01}
.max=${1}
.step=${0.01}
.compact=${compact}
.inline=${true}
.live=${true}
.ariaLabel=${"Attack ratio"}
.ariaValueText=${`${Math.round(ratioPercent)}%`}
@value-changed=${this.handleAttackRatioSliderChange}
></fluent-slider>
`;
}

private calculateTroopBar(): { greenPercent: number; orangePercent: number } {
Expand Down Expand Up @@ -319,15 +340,7 @@ export class ControlPanel extends LitElement implements Layer {
)})</span
>
</div>
<input
type="range"
min="1"
max="100"
.value=${String(Math.round(this.attackRatio * 100))}
@input=${(e: Event) => this.handleRatioSliderInput(e)}
@pointerup=${(e: Event) => this.handleRatioSliderPointerUp(e)}
class="flex-1 h-2 accent-blue-500 cursor-pointer"
/>
<div class="flex-1">${this.renderCustomAttackRatioSlider()}</div>
</div>
`;
}
Expand Down Expand Up @@ -366,15 +379,7 @@ export class ControlPanel extends LitElement implements Layer {
</div>
<!-- Attack ratio slider -->
<div class="flex-1" translate="no">
<input
type="range"
min="1"
max="100"
.value=${String(Math.round(this.attackRatio * 100))}
@input=${(e: Event) => this.handleRatioSliderInput(e)}
@pointerup=${(e: Event) => this.handleRatioSliderPointerUp(e)}
class="w-full h-1.5 accent-blue-500 cursor-pointer"
/>
${this.renderCustomAttackRatioSlider(true)}
</div>
</div>
`;
Expand Down
43 changes: 43 additions & 0 deletions tests/client/components/FluentSlider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,49 @@ describe("FluentSlider", () => {
});
});

describe("Inline and Live Modes", () => {
it("should dispatch value-changed on input when live mode is enabled", async () => {
slider.min = 0.01;
slider.max = 1;
slider.step = 0.01;
slider.value = 0.2;
slider.live = true;
await slider.updateComplete;

const eventSpy = vi.fn();
slider.addEventListener("value-changed", eventSpy);

const rangeInput = slider.querySelector(
'input[type="range"]',
) as HTMLInputElement;
rangeInput.valueAsNumber = 0.42;
rangeInput.dispatchEvent(new Event("input", { bubbles: true }));
await slider.updateComplete;

expect(eventSpy).toHaveBeenCalledTimes(1);
expect(
(eventSpy.mock.calls[0][0] as CustomEvent<{ value: number }>).detail
.value,
).toBe(0.42);
});

it("should render inline mode without the editable value button", async () => {
slider.inline = true;
slider.compact = true;
slider.ariaLabel = "Attack ratio";
slider.ariaValueText = "42%";
await slider.updateComplete;

expect(slider.querySelector('span[role="button"]')).toBeNull();

const rangeInput = slider.querySelector(
'input[type="range"]',
) as HTMLInputElement;
expect(rangeInput).toBeTruthy();
expect(rangeInput.getAttribute("aria-label")).toBe("Attack ratio");
expect(rangeInput.getAttribute("aria-valuetext")).toBe("42%");
});
});
describe("Edge Cases", () => {
it("should handle min equal to max without NaN in style", async () => {
slider.min = 100;
Expand Down