Skip to content

Fix Panel C target reachability and compact power-up tray re-anchoring on fullscreen lifecycle changes #76

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

Merged
TeacherEvan merged 3 commits into from
Apr 19, 2026
Merged

Fix Panel C target reachability and compact power-up tray re-anchoring on fullscreen lifecycle changes #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7dfe768
Initial plan
Copilot Apr 18, 2026
b922bf3
fix: keep Panel C targets visible and re-anchor power-up tray on full…
Copilot Apr 18, 2026
aafa3b6
test: harden compact tray and panel-c catch-zone regressions
Copilot Apr 18, 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
1 change: 1 addition & 0 deletions src/scripts/symbol-rain.animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
const symbolObj = state.activeFallingSymbols[readIndex];
const isOffScreen = symbolObj.y > containerHeight + 50;
const isStuckAtBottom =
!state.isMobileMode &&
symbolObj.y > containerHeight - 100 &&
state.activeFallingSymbols.length > 30;
const isTouching = state.symbolsToRemove.has(symbolObj);
Expand Down
7 changes: 7 additions & 0 deletions src/scripts/worm-powerups.ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@
this._displayLayoutHandler = () => this.syncDisplayLayout();
window.addEventListener("resize", this._displayLayoutHandler);
window.addEventListener("orientationchange", this._displayLayoutHandler);
document.addEventListener("fullscreenchange", this._displayLayoutHandler);
const sharedResizeObserver =
window.__ensureSharedResizeObserver?.() || window.SharedResizeObserver;
sharedResizeObserver?.subscribe?.(this._displayLayoutHandler, {
Comment on lines +103 to +106

@sourcery-ai sourcery-ai Bot Apr 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Consider how the new fullscreen listener and resize observer subscription are cleaned up to avoid leaks.

These are long-lived subscriptions. Please ensure the UI’s teardown/dispose path removes the document fullscreenchange listener and unsubscribes from the shared resize observer (if it returns an unsubscribe handle); otherwise repeated teardown/recreate cycles may accumulate handlers.

immediate: true,
source: "worm-powerups-ui",
});
Comment on lines 101 to +109

Copilot AI Apr 19, 2026

Copy link

Choose a reason for hiding this comment

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

_bindUIEventHandlers now subscribes to the shared resize observer hub but still binds direct resize/orientationchange/fullscreenchange listeners. Because the hub already forwards those events, syncDisplayLayout() will be invoked multiple times per lifecycle event (and the hub subscription can’t be cleaned up because the unsubscribe function isn’t stored). Consider using either the hub (preferred) or the direct listeners as a fallback, and store the unsubscribe callback so it can be invoked if the system is ever torn down/reinitialized.

Suggested change
window.addEventListener("resize", this._displayLayoutHandler);
window.addEventListener("orientationchange", this._displayLayoutHandler);
document.addEventListener("fullscreenchange", this._displayLayoutHandler);
const sharedResizeObserver =
window.__ensureSharedResizeObserver?.() || window.SharedResizeObserver;
sharedResizeObserver?.subscribe?.(this._displayLayoutHandler, {
immediate: true,
source: "worm-powerups-ui",
});
}
if (!this._displayLayoutUnsubscribe) {
const sharedResizeObserver =
window.__ensureSharedResizeObserver?.() || window.SharedResizeObserver;
if (sharedResizeObserver?.subscribe) {
const unsubscribe = sharedResizeObserver.subscribe(
this._displayLayoutHandler,
{
immediate: true,
source: "worm-powerups-ui",
},
);
this._displayLayoutUnsubscribe =
typeof unsubscribe === "function" ? unsubscribe : null;
} else {
window.addEventListener("resize", this._displayLayoutHandler);
window.addEventListener("orientationchange", this._displayLayoutHandler);
document.addEventListener("fullscreenchange", this._displayLayoutHandler);
}

Copilot uses AI. Check for mistakes.
}
};

Expand Down
45 changes: 45 additions & 0 deletions tests/powerups.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
waitForGameplayInputReady,
} from "./utils/onboarding-runtime.js";

const COMPACT_TRAY_MAX_TOP = 80;

@sourcery-ai sourcery-ai Bot Apr 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: Avoid hard-coding a magic top threshold for the compact tray to reduce layout brittleness.

Using a fixed COMPACT_TRAY_MAX_TOP = 80 couples this test to the current pixel layout and makes it brittle to spacing/typography changes. Please derive this bound from the actual compact controls layout (e.g. from the controls’ bounding rect plus a margin), or at minimum add a clear comment explaining why 80 is valid and when it should be updated, so layout tweaks don’t cause spurious test failures.

Suggested change
const COMPACT_TRAY_MAX_TOP = 80;
/**
* Upper bound (in CSS px) for the compact power-up tray’s top position.
*
* This value is derived from the current compact layout:
* - ~56px: tray height + internal padding
* - ~16px: top HUD spacing
* - ~8px: safety margin so minor typography/spacing tweaks don’t break the test
*
* If the compact controls are visibly moved lower in the viewport (e.g. layout
* redesign, font/spacing changes), update this threshold to be slightly larger
* than the tray’s new expected top position. The intent is to assert that the
* tray remains “near the top” of the screen, not to lock the exact pixel value.
*/
const COMPACT_TRAY_MAX_TOP = 80;


async function pressPowerUp(page, type) {
await page
.locator(`[data-testid="powerup-${type}"]`)
Expand Down Expand Up @@ -463,6 +465,49 @@ test.describe("Power-Up Compact Layout", () => {
expect(layout.computedTop).not.toBe("auto");
expect(layout.distanceToTop).toBeLessThan(layout.distanceToBottom);
});

test("re-anchors the power-up tray after fullscreen lifecycle changes", async ({
page,
}) => {
await page.evaluate(() => {
const display = document.getElementById("power-up-display");
if (!display) {
return;
}

display.dataset.dragged = "false";
display.style.left = "8px";
display.style.top = `${Math.round(window.innerHeight * 0.6)}px`;
document.dispatchEvent(new Event("fullscreenchange"));
});

await expect
.poll(
async () =>
page.evaluate((maxTop) => {
const display = document.getElementById("power-up-display");
const panelB = document.getElementById("panel-b");
const controls = document.querySelector(".panel-b-controls");

if (!display || !panelB || !controls) {
return false;
}

const rect = display.getBoundingClientRect();
const panelRect = panelB.getBoundingClientRect();
const controlsRect = controls.getBoundingClientRect();

return (
rect.left >= panelRect.left &&
rect.right <= panelRect.right &&
rect.bottom <= controlsRect.top &&
rect.top < maxTop
);
}, COMPACT_TRAY_MAX_TOP),
{ timeout: 5000 },
)
.toBe(true);
});
});

test.describe("Power-Up Portrait Rotation Contract", () => {
Expand Down
31 changes: 31 additions & 0 deletions tests/symbol-rain.mobile.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
stopEvanHelpIfActive,
} from "./utils/onboarding-runtime.js";

const PANEL_C_CATCH_ZONE_THRESHOLD = 0.7;

test.use({
...devices["Pixel 7"],
viewport: { width: 915, height: 412 },
Expand Down Expand Up @@ -392,6 +394,35 @@ test.describe("Symbol rain mobile interactions", () => {
timeout: 10000,
});

await expect
.poll(
async () =>
page.evaluate((catchZoneThreshold) => {
const panel = document.getElementById("panel-c");
const panelRect = panel?.getBoundingClientRect();
if (!panelRect) {
return 0;
}

return Array.from(
document.querySelectorAll("#panel-c .falling-symbol:not(.clicked)"),
).filter((element) => {
const rect = element.getBoundingClientRect();
const intersectsPanel =
rect.bottom > panelRect.top &&
rect.top < panelRect.bottom &&
rect.right > panelRect.left &&
rect.left < panelRect.right;
const reachesCatchZone =
rect.top >=
panelRect.top + panelRect.height * catchZoneThreshold;
return intersectsPanel && reachesCatchZone;
}).length;
}, PANEL_C_CATCH_ZONE_THRESHOLD),
{ timeout: 8000 },
)
.toBeGreaterThan(0);

await context.close();
});

Expand Down