Remove unused components and one code smell - #4777
Open
VariableVince wants to merge 6 commits into
Open
Conversation
…e smell
### `google-ad` (`GoogleAdElement.ts`) — deleted
Direct Google AdSense integration that predated the Playwire RAMP migration. No code in the current codebase references `adsbygoogle`, `<google-ad>`, or `GoogleAdElement` — zero traces of AdSense remain. All ad serving goes through Playwire's `window.ramp` API (`spaAddAds`, `addUnits`).
### `matchmaking-button` (`Matchmaking.ts`) — deleted
An unused Lit component sitting alongside the active `matchmaking-modal` in the same file. The actual ranked flow is:
- "Ranked" button rendered in `GameModeSelector.ts:228-232` (desktop) and `:147-151` (mobile)
- Click calls `openRankedMenu()` (`:248`) → `window.showPage("page-ranked")` → opens `<ranked-modal>`
- Clicking 1v1/2v2 dispatches `open-matchmaking` → opens `<matchmaking-modal>` directly
`MatchmakingButton` was never imported or instantiated anywhere.
### `news-button` (`NewsModal.ts`) — deleted
An unused Lit component alongside the active `news-modal`. The actual News button is in `DesktopNavBar.ts:78-84` and `MobileNavBar.ts:101`, both with `data-page="page-news"` which routes to `<news-modal>` via the modal router. `NewsButton` was never imported, instantiated, or placed in the DOM.
### `main-radial-menu` (`MainRadialMenu.ts`) — fixed code smell
Extended `LitElement` with `@customElement("main-radial-menu")` but had no `render()` method — all rendering was delegated to a separate `RadialMenu` Controller. Never used as a DOM custom element (no template usage, never dynamically appended). Only ever instantiated via `new MainRadialMenu(...)` in `GameRenderer.ts:301` and used through the `Controller` interface. Removed the `LitElement`/`@customElement` baggage since they were misleading dead weight.
### Removals from `LangSelector.ts`
Both `"matchmaking-button"` and `"news-button"` were listed in the `components` array used for i18n re-rendering (`:287-293`). Since these elements never exist in the DOM, `querySelectorAll` returned empty for them — harmless but misleading. Removed.
…ration. No code in the current codebase references `adsbygoogle`, `<google-ad>`, or `GoogleAdElement` — zero traces of AdSense remain. All ad serving goes through Playwire's `window.ramp` API (`spaAddAds`, `addUnits`).
An unused Lit component sitting alongside the active `matchmaking-modal` in the same file. The actual ranked flow is:
- "Ranked" button rendered in `GameModeSelector.ts:228-232` (desktop) and `:147-151` (mobile)
- Click calls `openRankedMenu()` (`:248`) → `window.showPage("page-ranked")` → opens `<ranked-modal>`
- Clicking 1v1/2v2 dispatches `open-matchmaking` → opens `<matchmaking-modal>` directly
`MatchmakingButton` was never imported or instantiated anywhere.
An unused Lit component alongside the active `news-modal`. The actual News button is in `DesktopNavBar.ts:78-84` and `MobileNavBar.ts:101`, both with `data-page="page-news"` which routes to `<news-modal>` via the modal router. `NewsButton` was never imported, instantiated, or placed in the DOM.
Extended `LitElement` with `@customElement("main-radial-menu")` but had no `render()` method — all rendering was delegated to a separate `RadialMenu` Controller. Never used as a DOM custom element (no template usage, never dynamically appended). Only ever instantiated via `new MainRadialMenu(...)` in `GameRenderer.ts:301` and used through the `Controller` interface. Removed the `LitElement`/`@customElement` baggage since they were misleading dead weight.
Both `"matchmaking-button"` and `"news-button"` were listed in the `components` array used for i18n re-rendering (`:287-293`). Since these elements never exist in the DOM, `querySelectorAll` returned empty for them — harmless but misleading. Removed.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUnused Google ad, matchmaking button, and news button components are removed. ChangesClient component cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
VariableVince
changed the title
github-actions
Bot
added
the
auto-closed-needs-issue
|
Hi @VariableVince, thanks for the contribution. This PR was automatically closed because it doesn't fit our contribution workflow:
To contribute to OpenFront:
If you believe this was closed in error, please reach out on our Discord or comment below. See CONTRIBUTING.md for the full contribution process. — Automated PR gate. Source. |
github-project-automation
Bot
moved this from Triage
to Complete
in OpenFront Release Management
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
iiamlewis
added
approved
github-project-automation
Bot
moved this from Complete
to Triage
in OpenFront Release Management
Contributor
|
Hi @VariableVince, thanks for the contribution. This PR was automatically closed because it doesn't fit our contribution workflow:
To contribute to OpenFront:
If you believe this was closed in error, please reach out on our Discord or comment below. See CONTRIBUTING.md for the full contribution process. — Automated PR gate. Source. |
github-actions
Bot
added
the
auto-closed-needs-issue
github-project-automation
Bot
moved this from Triage
to Complete
in OpenFront Release Management
github-project-automation
Bot
moved this from Complete
to Triage
in OpenFront Release Management
iiamlewis
removed
the
auto-closed-needs-issue
Contributor
|
STOP FIGHTING ME GITBOT |
Contributor
Author
|
Will fix failing test asap (done) |
…n en.json. Moved the still used must_login to the matchmaking_modal category, and also changed in caller Matchmaking.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add approved & assigned issue number here:
Resolves #4776
Description:
Remove 3 unused components and their now-unused imports, fix 1 code smell
google-ad(GoogleAdElement.ts) — deletedDirect Google AdSense integration that predated the Playwire RAMP migration. No code in the current codebase references
adsbygoogle,<google-ad>, orGoogleAdElement— zero traces of AdSense remain. All ad serving goes through Playwire'swindow.rampAPI (spaAddAds,addUnits).matchmaking-button(Matchmaking.ts) — deletedAn unused Lit component sitting alongside the active
matchmaking-modalin the same file. The actual ranked flow is:GameModeSelector.ts:228-232(desktop) and:147-151(mobile)openRankedMenu()(:248) →window.showPage("page-ranked")→ opens<ranked-modal>open-matchmaking→ opens<matchmaking-modal>directlyMatchmakingButtonwas never imported or instantiated anywhere.news-button(NewsModal.ts) — deletedAn unused Lit component alongside the active
news-modal. The actual News button is inDesktopNavBar.ts:78-84andMobileNavBar.ts:101, both withdata-page="page-news"which routes to<news-modal>via the modal router.NewsButtonwas never imported, instantiated, or placed in the DOM.main-radial-menu(MainRadialMenu.ts) — fixed code smellExtended
LitElementwith@customElement("main-radial-menu")but had norender()method — all rendering was delegated to a separateRadialMenuController. Never used as a DOM custom element (no template usage, never dynamically appended). Only ever instantiated vianew MainRadialMenu(...)inGameRenderer.ts:301and used through theControllerinterface. Removed theLitElement/@customElementbaggage since they were misleading dead weight.Removals from
LangSelector.tsBoth
"matchmaking-button"and"news-button"were listed in thecomponentsarray used for i18n re-rendering (:287-293). Since these elements never exist in the DOM,querySelectorAllreturned empty for them — harmless but misleading. Removed.Removals and changes in
en.jsonandMatchmaking.tsAlso remove the already unused translations for "matchmaking-button". Move the only one still used to category "matchmaking-modal" in en.json and in its caller Matchmaking.ts.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
tryout33