Filter unreachable servers from selector (newdesign UI)#769
Merged
Filter unreachable servers from selector (newdesign UI)#769
Conversation
Contributor
Review Summary by QodoFilter unreachable servers from selector and fix dropdown state management
WalkthroughsDescription• Filter unreachable servers from dropdown selector • Run server reachability checks before populating UI • Reset dropdown state on each population to prevent stale data • Prevent duplicate event listener registration on dropdown Diagramflowchart LR
A["Fetch server list"] --> B["Single server?"]
B -->|Yes| C["Show directly"]
B -->|No| D["Run reachability checks"]
D --> E["Filter dead servers"]
E --> F["Reset dropdown state"]
F --> G["Populate dropdown UI"]
G --> H["Register event listeners once"]
File Changes1. frontend/javascript/index.js
|
Contributor
Code Review by QodoSorry, something went wrongWe weren't able to complete the code review on our side. Please try againⓘ The new review experience is currently in Beta. Learn more |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to improve the “newdesign” frontend server selector by hiding unreachable (“dead”) speedtest servers from the dropdown list, based on the ping reachability results produced by the built-in Speedtest.selectServer logic.
Changes:
- Defer populating the server dropdown until after
selectServerhas pinged servers, then filter out servers withpingT === -1. - Add UI reset logic in
populateDropdownto support re-rendering and avoid duplicate event handler registration.
Comments suppressed due to low confidence (1)
frontend/javascript/index.js:201
- In the
servers.length === 1path,populateDropdownnow removes the.activeclass (line 193) and then returns without re-adding it. Since.server-selectorisdisplay: noneunless.activeis present, this will hide the server selector entirely when reachability filtering leaves exactly one server alive. Consider always adding.activebefore returning (or adding it specifically in the single-server branch) so the chosen server remains visible.
// Reset previous state (populateDropdown can be called multiple times)
serverSelector.classList.remove("single-server");
serverSelector.classList.remove("active");
serverList.classList.remove("active");
serverList.innerHTML = "";
// If we have only a single server, just show it
if (servers.length === 1) {
serverSelector.classList.add("single-server");
selectServer(servers[0]);
return;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the newdesign UI’s server selector to avoid showing unreachable speedtest servers by leveraging LibreSpeed’s built-in server ping/selection, and improves dropdown re-render behavior.
Changes:
- Delay dropdown population until after
Speedtest.selectServer()completes, then filter servers based on reachability (pingT). - Keep a fallback to show the full list when no servers are reachable.
- Reset dropdown DOM/class state on each render and ensure open/close handlers are only attached once.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
sstidl
added a commit
that referenced
this pull request
Apr 11, 2026
* Implement fromScratch design * Add some documentation * enable new design in docker (no alpine yet) * merge fixed docker images * alpine docker added new design * fix #685 * Implement fromScratch design * Add some documentation * enable new design in docker (no alpine yet) * alpine docker added new design * fix #685 * fix database permissions alpine, remove baby * hide serverselector on only one server * Update frontend/styling/server-selector.css Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com> * fix alpine image again * adjust settings.json in entrypoint so env vars are honored * Update frontend/javascript/index.js Co-authored-by: qodo-merge-for-open-source[bot] <189517486+qodo-merge-for-open-source[bot]@users.noreply.github.com> * Add feature switch for new design via config file, URL parameters, and Docker (#742) * Initial plan * Add feature switch for new design with config and URL parameter support Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Improve error handling and prevent infinite redirect loops Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Update Dockerfiles and entrypoint to support design feature switch Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Update design-switch.js Co-authored-by: qodo-free-for-open-source-projects[bot] <189517486+qodo-free-for-open-source-projects[bot]@users.noreply.github.com> * fix: copy actions in entrypoint * Restructure design switch to place both designs at root level Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Flatten frontend assets in Docker to eliminate frontend directory Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * fix: entrypoint settings & server-list disable entrypoint bash debug * add link to modern design --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> Co-authored-by: sstidl <sstidl@users.noreply.github.com> Co-authored-by: qodo-free-for-open-source-projects[bot] <189517486+qodo-free-for-open-source-projects[bot]@users.noreply.github.com> * add armv7 * reformat * Add GDPR_EMAIL environment variable for Docker deployments (#743) * Initial plan * Add GDPR_EMAIL environment variable for Docker deployments Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Improve GDPR_EMAIL handling with proper escaping and loop Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Refine GDPR_EMAIL processing - skip index.html and improve escaping Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Optimize GDPR_EMAIL sed commands and improve escaping Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * Clarify sed escaping comment for GDPR_EMAIL Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> * cleanup old EMAIL ENV Var * fix: line break in html prevented sed replacement * version 6.0.0pre1 * test: add mssql docker compose tests * Update Speedtest screen recording link in README * Filter unreachable servers from selector (newdesign UI) (#769) * Filter unreachable servers from selector (newdesign UI) * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * keep // servers in list Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix misleading comment --------- Co-authored-by: Lumi <lumi@openclaw.local> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Stefan Stidl <sti-github@stidl.com> * fix: server.json handling * Use server-list.json in classic frontend by default * Add configurable server list URLs to frontend and Docker --------- Co-authored-by: Timendus <mail@timendus.com> Co-authored-by: Stefan Stidl <stefan.stidl@ffg.at> Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com> Co-authored-by: qodo-merge-for-open-source[bot] <189517486+qodo-merge-for-open-source[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sstidl <12804296+sstidl@users.noreply.github.com> Co-authored-by: qodo-free-for-open-source-projects[bot] <189517486+qodo-free-for-open-source-projects[bot]@users.noreply.github.com> Co-authored-by: Stefan Stidl <sti-github@stidl.com> Co-authored-by: Lumi <lumi@openclaw.local> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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.
dont show dead servers