fix: kick screen would allow going back with lockConnect parameter

[Phoenix616]

User description

Fix that the kick screen would still contain a button to go back to the main menu even if the connection was locked.

This hides that button (similar to the main menu) and resetting takes you back to the connection screen.

---

PR Type

Bug fix

---

Description

• Fixes the issue where the "Back" button was visible on the kick screen despite the lockConnect parameter.

• Introduces a check for the lockConnect parameter to conditionally hide the "Back" button.

• Ensures consistent behavior with the locked connection state.

---

Changes walkthrough 📝

	Relevant files
Bug fix	AppStatus.tsx Conditional rendering of "Back" button based on `lockConnect` src/react/AppStatus.tsx Added a check for the lockConnect parameter from URL query string. Modified the rendering logic to hide the "Back" button when lockConnect is true. +3/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review URL Parameter Access Direct access to URL parameters without validation or sanitization. Consider adding error handling for malformed URLs or missing parameters. const qsParams = new URLSearchParams(window.location.search) const lockConnect = qsParams?.get('lockConnect') === 'true'

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add null safety checks Add null check for window.location.search to prevent runtime errors in environments where it might be undefined src/react/AppStatus.tsx [18] -const qsParams = new URLSearchParams(window.location.search) +const qsParams = new URLSearchParams(window?.location?.search || '') Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding null safety checks for window.location.search is important for preventing runtime errors in environments where window might be undefined (like SSR), making the code more robust.	7
General	Remove unnecessary optional chaining Remove optional chaining on qsParams as URLSearchParams is guaranteed to be defined after instantiation src/react/AppStatus.tsx [19] -const lockConnect = qsParams?.get('lockConnect') === 'true' +const lockConnect = qsParams.get('lockConnect') === 'true' Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion correctly points out that URLSearchParams instance is guaranteed to exist after instantiation, making the optional chaining unnecessary and potentially misleading.	5

[zardoy]

can we refactor to have some centralized typed QS params getters. eg we can them from one place

[Phoenix616]

Yeah that would probably be good, I can look into that tomorrow.

[zardoy]

I think typed proxy object should be used so anywhere in the app we can just use

// somefile.ts
appQueryParams.reconnectOnCrash // getter would be called that would check if query param is present at THAT moment

wdym? seems ideal and clear solution

[Phoenix616]

I think typed proxy object should be used so anywhere in the app we can just use

// somefile.ts
appQueryParams.reconnectOnCrash // getter would be called that would check if query param is present at THAT moment

Checking if it's present at the moment it's queried is not ideal in all cases because the user can't really change those at runtime but I get what you mean.

wdym? seems ideal and clear solution

Well, I'm just not that well-versed in TypeScript and what features it has to offer hence why I said that I will look into it.

Also are you sure it's the best solution? It seems wasteful to create a new URLSearchParams object every time a value is queried e.g. here there is 6 of them after each other.

I mean I think I have the code working for this and can adjust it everywhere if you don't think that is that big of an issue. I'm just used to not wanting to waste even a little bit of memory and garbage collector time from Java server development.

[Phoenix616]

Also while I am able to get it to work crudely via declaring a window.appQueryParams at the top of the src/index.ts I don't seem to be able to declare them in the src/globals.d.ts file where I would expect that to be located

I tried this code:

declare const appQueryParams = new Proxy({}, {
  get (target, property) {
    if (typeof property !== "string") {
      return false
    }
    return new URLSearchParams(window.location.search)?.get(property)
  },
}) as any

But that throws this error:

ServersListProvider.tsx:97 Uncaught ReferenceError: appQueryParams is not defined
    at ./src/react/ServersListProvider.tsx (ServersListProvider.tsx:97:23)
    at __webpack_require__ (worldInteractions.ts:423:32)
    at fn (worldInteractions.ts:423:32)
    at ./src/react/AppStatusProvider.tsx (AppStatus.tsx:77:2)
    at __webpack_require__ (worldInteractions.ts:423:32)
    at fn (worldInteractions.ts:423:32)
    at ./src/utils.ts (topRightStats.ts:129:4)
    at __webpack_require__ (worldInteractions.ts:423:32)
    at fn (worldInteractions.ts:423:32)
    at ./src/globalState.ts (globalDomListeners.ts:35:1)

which does not make sense as the globals are imported in the src/index.ts before basically anything else? Or is that not the entry point to the app?

[zardoy]

Sorry for asking you to make it! I actually wanted to implement it myself, will update your branch soon, sorry for the confusion. Would be good if you could focus on finishing item frames branch instead, thank you