Skip to content

Patch 2 fix: #648 Replace inline and direct event listeners with 100% delegated data-action pattern - #657

Draft
quantumchain-core wants to merge 35 commits into
NextCommunity:mainfrom
quantumchain-core:patch-2
Draft

quantumchain-core wants to merge 35 commits into
NextCommunity:mainfrom
quantumchain-core:patch-2

Conversation

@quantumchain-core

Copy link
Copy Markdown
Contributor

Fixes #648
Closes #648
Related to #627 and PR #649 (merged)

Context - What #648 Asked For

#648 = Replace all inline onclick="", onmouseenter="" and direct per-element addEventListener with centralized delegated listeners using data-action pattern.

Diagram reference:

  • Yellow layer (Site presentation: base.njk, index.njk, games.njk, bio.njk) = must be checked
  • Green layer (Browser experience: script.js, eggs.js, ui-bindings.js) = must be delegated
  • Blue layer (build.js, site.js, users.json) = build-time Node, out of scope for Replace inline onclick handlers with event listeners or delegated events #648
  • Pink layer (easter egg games) = already using addEventListener internally, out of scope

What Was Already Fixed in PR #649 (Merged)

PR #649 created src/assets/js/ui-bindings.js with centralized delegated pattern:

document.addEventListener('click', (e) => {
  const btn = e.target.closest('[data-action]');
  // handlers map
});
Verified after #649 merge:

1. `layouts/base.njk` - CLEAN, 0 onclick=
2. `index.njk` (Directory page) - CLEAN, only `data-action="duel-from-card"` which is correct
3. `games.njk` (Arcade page) - CLEAN, only `data-action="launch-invaders"` and `data-action="launch-breaker-arcade"`
4. `build.js` - OUT OF SCOPE, Node build file, runs `execSync("git rev-parse --short HEAD")` safely
5. `ui-bindings.js` itself - 90% CLEAN

Remaining violations after #649 were 2.

What Still Violated #648 Before This PR

Violation 1: `src/_includes/bio.njk` (Profile pages template)
<div class="... copy-email-btn" data-email="{{ email }}">
document.querySelector('.copy-email-btn')?.addEventListener('click', function() {
  copyToClipboard(this.getAttribute('data-email'), this);
});
Direct per-element listener = banned by #648. Should be delegated.

Violation 2: `src/assets/js/ui-bindings.js`
document.addEventListener('DOMContentLoaded', () => {
  document.querySelectorAll('.floating-xp-trigger').forEach((el) => {
    el.addEventListener('mouseenter', (ev) => createFloatingXP?.(ev));
  });
});
`querySelectorAll` + loop + per-element `addEventListener` = banned by #648. Should be 100% delegated on document.

What Changed In This PR - Full Fix For #648

File 1: `bio.njk` - FIXED
*Before:*
<div class="flex items-center gap-3 group cursor-pointer copy-email-btn" data-email="{{ email }}">
  <span>✉️</span>
  <span>{{ email }}</span>
  <span>Copy</span>
</div>
*After:*
<button data-action="copy-email" data-email="{{ email }}" class="flex items-center gap-3 group cursor-pointer w-full text-left">
  <span>✉️</span>
  <span>{{ email }}</span>
  <span>Copy</span>
</button>
Changes:
- `div` -> `button` for accessibility
- `copy-email-btn` class removed, replaced with `data-action="copy-email"`
- Deleted direct `addEventListener` from inline `<script>` block
- Kept `copyToClipboard()` helper function as global (called by ui-bindings)

File 2: `ui-bindings.js` - FIXED - Final version now 100% delegated
*Changes:*
1. Added `copyToClipboard` to global comment: `/_ global..., copyToClipboard _/`
2. Added delegated handler at top of click listener:
if (btn.dataset.action === 'copy-email') {
  const email = btn.dataset.email;
  if (email && typeof copyToClipboard === 'function') {
    copyToClipboard(email, btn);
  } else if (email && navigator.clipboard) {
    navigator.clipboard.writeText(email);
  }
  return;
}
3. Removed entire `DOMContentLoaded` block with `querySelectorAll` loop
4. Replaced with single delegated mouseover:
document.addEventListener('mouseover', (e) => {
  const el = e.target.closest('[data-action="floating-xp"],.floating-xp-trigger');
  if (el) createFloatingXP?.(e);
});
5. Now file has ONLY 2 listeners on document: `click` and `mouseover` -> passes #648

Final Audit - All Files Clean For #648
File	onclick= count	on* inline	direct addEventListener	data-action pattern	Result
layouts/base.njk	0	0	0	N/A	CLEAN
http://index.njk	0	0	0	duel-from-card	CLEAN
http://bio.njk	0	0	0	copy-email, launch-codebreaker	FIXED in this PR
http://games.njk	0	0	0	launch-invaders, launch-breaker-arcade	CLEAN
http://ui-bindings.js	0	0	0 (only 2 delegated on document)	all handlers	FIXED in this PR
http://build.js / http://site.js / http://users.json	N/A	N/A	N/A	N/A	Out of scope
`grep -r "onclick" src/` -> 0 results
`grep -r "onmouse\|onload\|onerror" src/_includes/` -> 0 results

How to Test

1. `npm run build` should pass
2. Go to Directory page `/` -> hover cards -> Duel button appears -> click -> Dev Duel game launches (delegated)
3. Go to any profile page `/users/xxx/` -> click email -> should show Copied! -> check clipboard
4. Go to `/games/` -> click Launch Space Invaders and Play Code Breaker -> both work
5. Hover any element with `.floating-xp-trigger` -> XP popup appears (delegated mouseover)
6. Console -> no errors

Checklist

- No inline onclick= in any njk
- No per-element addEventListener in njk or http://ui-bindings.js
- All interactions via document-level delegation with data-action
- http://bio.njk copy email still works
- floating-xp hover still works
- Build passes
- Closes #648
- 

@deepsource-io

deepsource-io Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 2f0945a...1e63b17 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 Oct 10, 2026 8:01a.m. Review ↗
Secrets Oct 10, 2026 8:01a.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/ui-bindings.js Outdated

Copilot AI left a comment

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.

🟡 Changes recommended

Direct per-element listeners remain, and the attribute migration breaks existing dev-tool color selectors.

2 open findings
What changed in this PR

Centralizes browser UI interactions through delegated data-action handlers.

Changes:

  • Replaces inline event attributes across templates.
  • Adds centralized click and hover delegation.
  • Improves the email-copy control’s semantics.
File Description
src/​index.njk Delegates developer-duel launches.
src/​games.njk Delegates arcade game launches.
src/​assets/​js/​ui-bindings.js Implements centralized action dispatch.
src/​_includes/​system-override.njk Converts override controls to data actions.
src/​_includes/​system-log.njk Delegates console controls.
src/​_includes/​skills-list.njk Marks skill hover actions for delegation.
src/​_includes/​scripts.njk Loads the binding script last.
src/​_includes/​matrix-overlay.njk Delegates matrix closing.
src/​_includes/​game-stats.njk Delegates stats and theme controls.
src/​_includes/​footer-details.njk Delegates footer actions.
src/​_includes/​bio.njk Delegates email copying and game launch.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

// skipcq: JS-R1005 JS-0357
/* global scrollToRandomUser, handleLevelClick, toggleTheme, closeMatrix, reopenConsole, minimizeConsole, maximizeConsole, closeConsole, triggerSecretUnlock, triggerForceSurge, addExperience, XP_SPACE_INVADERS_WIN, _XP_CODE_BREAKER_WIN, _XP_DEV_DUEL_PLAY, playSound, triggerMagicXP, toggleScreenshotMode, jumpToLevel, startSelfDestruct, CodeBreaker, handleFooterDotClick, SpaceInvaders, startDuelFromCard, createFloatingXP, copyToClipboard */

// Fixes #627 + #648 - 100% delegated UI bindings
</button>
<button onclick="triggerForceSurge()"
class="relative overflow-hidden py-2 px-6 bg-blue-600/20 text-blue-300 border-2 border-blue-400 rounded-lg font-bold tracking-[0.2em] transition-all hover:bg-blue-500/40 hover:shadow-[0_0_30px_rgba(59,130,246,0.6)] animate-pulse uppercase">
<button data-action="secret-unlock" data-code="badge_click" class="override-btn py-1.5 bg-purple-500/5 hover:bg-purple-500/20 text-purple-400 text-[8px] border border-purple-500/20 rounded transition-all">BADGE_XP</button>
@jbampton
jbampton marked this pull request as draft October 10, 2026 23:43
@quantumchain-core
quantumchain-core marked this pull request as ready for review October 11, 2026 03:17
@quantumchain-core
quantumchain-core marked this pull request as draft October 11, 2026 03:29

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Review in progress

Development

Successfully merging this pull request may close these issues.

Replace inline onclick handlers with event listeners or delegated events

4 participants