Skip to content

Comments

refactor: move event handlers from simple-modals to respective files (@biplavbarua)#7333

Merged
Miodec merged 13 commits intomonkeytypegame:masterfrom
biplavbarua:refactor/move-event-handlers
Jan 15, 2026
Merged

refactor: move event handlers from simple-modals to respective files (@biplavbarua)#7333
Miodec merged 13 commits intomonkeytypegame:masterfrom
biplavbarua:refactor/move-event-handlers

Conversation

@biplavbarua
Copy link
Contributor

@biplavbarua biplavbarua commented Jan 9, 2026

This PR addresses the TODO in simple-modals.ts by moving event handlers to their respective page controller files (account-settings.ts and settings.ts).

Key Changes:

  1. Moved .pageAccountSettings handlers to account-settings.ts.
  2. Moved .pageSettings handlers to settings.ts.
  3. Extracted PopupKey, list, and showPopup to simple-modals-base.ts to resolve circular dependencies introduced by importing page controllers in simple-modals.ts (which now import showPopup from base).

Testing:

  • Verified no circular dependencies with madge.
  • Verified type safety with tsc.
  • Verified build success.

@monkeytypegeorge monkeytypegeorge added the frontend User interface or web stuff label Jan 9, 2026
@biplavbarua biplavbarua changed the title Refactor: Move event handlers from simple-modals to respective files refactor: move event handlers from simple-modals to respective files (@biplavbarua) Jan 9, 2026
@Miodec Miodec requested a review from Copilot January 14, 2026 20:09
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Jan 14, 2026
Copilot AI reviewed Jan 14, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the codebase by moving event handlers from simple-modals.ts to their respective page controller files (account-settings.ts and settings.ts) and extracts shared utilities to a new simple-modals-base.ts file to prevent circular dependencies.

Changes:

  • Created simple-modals-base.ts to export PopupKey, list, and showPopup function
  • Moved event handlers for account-related buttons from simple-modals.ts to account-settings.ts
  • Moved event handlers for settings-related buttons from simple-modals.ts to settings.ts

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
frontend/src/ts/modals/simple-modals-base.ts New file containing extracted PopupKey type, list object, and showPopup function to prevent circular dependencies
frontend/src/ts/modals/simple-modals.ts Removed event handlers and showPopup function; now imports and re-exports from simple-modals-base.ts
frontend/src/ts/pages/account-settings.ts Added event handlers for account-related buttons and imported showPopup from simple-modals-base.ts
frontend/src/ts/pages/settings.ts Added event handlers for settings-related buttons (custom themes and fonts) and imported showPopup from simple-modals-base.ts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Miodec Miodec added waiting for update Pull requests or issues that require changes/comments before continuing and removed waiting for review Pull requests that require a review before continuing labels Jan 14, 2026
@biplavbarua
Copy link
Contributor Author

biplavbarua commented Jan 15, 2026

Refactored event handlers based on Copilot feedback:\n- Moved #resetSettingsButton handler to settings.ts.\n- Moved #bannerCenter handler to simple-modals.ts.

@github-actions github-actions bot removed the waiting for update Pull requests or issues that require changes/comments before continuing label Jan 15, 2026
@Miodec Miodec requested a review from Copilot January 15, 2026 14:17
Copilot AI reviewed Jan 15, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Jan 15, 2026
@Miodec Miodec requested a review from Copilot January 15, 2026 15:06
Copilot AI reviewed Jan 15, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Miodec
Miodec approved these changes Jan 15, 2026
View reviewed changes
Copy link
Member

@Miodec Miodec left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions github-actions bot removed the waiting for review Pull requests that require a review before continuing label Jan 15, 2026
@Miodec Miodec merged commit 9896c18 into monkeytypegame:master Jan 15, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@Miodec Miodec Miodec approved these changes

Assignees

No one assigned

Labels

frontend User interface or web stuff

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@biplavbarua @Miodec @monkeytypegeorge