Skip to content

Remove inline button handlers and centralize UI click binding - #626

Closed
jbampton with Copilot wants to merge 3 commits into
mainfrom
copilot/remove-inline-js-event-listeners
Closed

jbampton with Copilot wants to merge 3 commits into
mainfrom
copilot/remove-inline-js-event-listeners

Conversation

Copilot AI commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

This change removes inline onclick usage from interactive controls and moves button behavior into script-managed event listeners. The update keeps the existing UI actions intact while eliminating inline JavaScript from the affected templates.

  • Template changes

    • Replaced inline button handlers with stable id and data-* hooks across dev tools, console controls, game launchers, footer actions, and profile interactions.
    • Kept the markup intent explicit by encoding action metadata in attributes instead of executable HTML.
  • Client-side event wiring

    • Added centralized listener registration in src/assets/js/script.js via initButtonHandlers().
    • Mapped declarative hooks to existing behaviors such as secret unlocks, XP awards, console controls, game launches, theme toggle, screenshot mode, and level jump.
    • Hardened the duel launcher binding to resolve the nearest .user-card[data-name] at click time before invoking startDuelFromCard().
  • Selector alignment

    • Updated dev-tool styling rules in src/assets/css/style.css to target the new data-* attributes instead of [onclick*="..."] selectors.
<button type="button" data-action="launch-space-invaders">
  🚀 Launch
</button>
document
  .querySelectorAll("button[data-action='launch-space-invaders']")
  .forEach((button) => {
    button.addEventListener("click", () => {
      SpaceInvaders.launch();
    });
  });

Copilot AI and others added 3 commits September 26, 2026 16:00
Co-authored-by: jbampton <418747+jbampton@users.noreply.github.com>
Co-authored-by: jbampton <418747+jbampton@users.noreply.github.com>
Co-authored-by: jbampton <418747+jbampton@users.noreply.github.com>
@deepsource-io

deepsource-io Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 1839f0e...29676bd on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Sep 26, 2026 4:08p.m. Review ↗
Secrets Sep 26, 2026 4:08p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

Comment thread src/assets/js/script.js
Comment on lines +1363 to +1371
function resolveExperienceValue(value) {
const experienceMap = {
XP_SPACE_INVADERS_WIN,
_XP_CODE_BREAKER_WIN,
_XP_DEV_DUEL_PLAY,
};

return experienceMap[value] ?? Number(value);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable


It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

Comment thread src/assets/js/script.js
Comment on lines +1373 to +1379
function bindClickHandler(id, handler) {
const element = document.getElementById(id);

if (element) {
element.addEventListener("click", handler);
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable


It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

Comment thread src/assets/js/script.js
Comment on lines +1381 to +1472
function initButtonHandlers() {
document.querySelectorAll("button[data-secret-unlock]").forEach((button) => {
button.addEventListener("click", () => {
triggerSecretUnlock(button.dataset.secretUnlock);
});
});

document.querySelectorAll("button[data-add-experience]").forEach((button) => {
button.addEventListener("click", () => {
const amount = resolveExperienceValue(button.dataset.addExperience);

if (Number.isFinite(amount)) {
addExperience(amount);
}

if (button.dataset.sound) {
playSound(button.dataset.sound);
}
});
});

document.querySelectorAll("button[data-action='trigger-force-surge']").forEach((button) => {
button.addEventListener("click", triggerForceSurge);
});

document.querySelectorAll("button[data-action='trigger-magic-xp']").forEach((button) => {
button.addEventListener("click", () => {
triggerMagicXP();
if (button.dataset.sound) {
playSound(button.dataset.sound);
}
});
});

document
.querySelectorAll("button[data-action='toggle-screenshot-mode']")
.forEach((button) => {
button.addEventListener("click", window.toggleScreenshotMode);
});

document
.querySelectorAll("button[data-action='launch-profile-code-breaker']")
.forEach((button) => {
button.addEventListener("click", () => {
CodeBreaker.launch(window.PROFILE_SKILLS, window.PROFILE_NAME);
});
});

document
.querySelectorAll("button[data-action='launch-space-invaders']")
.forEach((button) => {
button.addEventListener("click", () => {
SpaceInvaders.launch();
});
});

document
.querySelectorAll("button[data-action='launch-arcade-code-breaker']")
.forEach((button) => {
button.addEventListener("click", () => {
CodeBreaker.launch(null, "Arcade Mode");
});
});

document
.querySelectorAll("button[data-action='start-duel-from-card']")
.forEach((button) => {
button.addEventListener("click", (event) => {
const card = event.currentTarget.closest(".user-card[data-name]");

if (card) {
startDuelFromCard(card);
}
});
});

bindClickHandler("close-matrix-btn", closeMatrix);
bindClickHandler("footer-surge-button", handleFooterDotClick);
bindClickHandler("reopen-console-btn", reopenConsole);
bindClickHandler("minimize-console-btn", minimizeConsole);
bindClickHandler("maximize-console-btn", maximizeConsole);
bindClickHandler("close-console-btn", closeConsole);
bindClickHandler("surprise-me-btn", scrollToRandomUser);
bindClickHandler("level-badge", handleLevelClick);
bindClickHandler("theme-icon", toggleTheme);
bindClickHandler("jump-to-level-btn", jumpToLevel);
bindClickHandler("self-destruct-btn", window.startSelfDestruct);
bindClickHandler("reset-local-storage-btn", () => {
localStorage.clear();
location.reload();
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable


It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

Comment thread src/assets/js/script.js
.querySelectorAll("button[data-action='launch-profile-code-breaker']")
.forEach((button) => {
button.addEventListener("click", () => {
CodeBreaker.launch(window.PROFILE_SKILLS, window.PROFILE_NAME);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'CodeBreaker' is not defined


Variables that aren't defined, but accessed may throw reference errors at runtime.

NOTE: In browser applications, DeepSource recommends the use of ESModules over regular text/javascript scripts.
Using variables that are injected by scripts included in an HTML file is currently not supported.

Comment thread src/assets/js/script.js
.querySelectorAll("button[data-action='launch-space-invaders']")
.forEach((button) => {
button.addEventListener("click", () => {
SpaceInvaders.launch();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'SpaceInvaders' is not defined


Variables that aren't defined, but accessed may throw reference errors at runtime.

NOTE: In browser applications, DeepSource recommends the use of ESModules over regular text/javascript scripts.
Using variables that are injected by scripts included in an HTML file is currently not supported.

Comment thread src/assets/js/script.js
.querySelectorAll("button[data-action='launch-arcade-code-breaker']")
.forEach((button) => {
button.addEventListener("click", () => {
CodeBreaker.launch(null, "Arcade Mode");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'CodeBreaker' is not defined


Variables that aren't defined, but accessed may throw reference errors at runtime.

NOTE: In browser applications, DeepSource recommends the use of ESModules over regular text/javascript scripts.
Using variables that are injected by scripts included in an HTML file is currently not supported.

Comment thread src/assets/js/script.js
const card = event.currentTarget.closest(".user-card[data-name]");

if (card) {
startDuelFromCard(card);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'startDuelFromCard' is not defined


Variables that aren't defined, but accessed may throw reference errors at runtime.

NOTE: In browser applications, DeepSource recommends the use of ESModules over regular text/javascript scripts.
Using variables that are injected by scripts included in an HTML file is currently not supported.

@jbampton jbampton closed this Sep 26, 2026
@github-project-automation github-project-automation Bot moved this to Done in Next Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants