Implement the new design from #649

[sstidl]

User description

I will close #649 to follow up here in this PR. Thanks to @Timendus there for implementing it.

The when the merge is ready we will have a new design as proposed by @Chris-ZoGo in #585

---

PR Type

Enhancement, Documentation, Tests

---

Description

• Major frontend redesign implementing fromScratch Studio's UI

  • New HTML/CSS/JS frontend with modern design and UX
  • Responsive layout for desktop and mobile
  • Enhanced server selection and telemetry features

• Docker and build system updates for new frontend

  • Dockerfiles and entrypoints updated to deploy new frontend
  • Test scripts and compose files adapted for new modes

• Added documentation for new frontend and testing

  • Usage, configuration, and credits in frontend/README.md
  • Test instructions in docker/test/README.md

• Added sample server lists and settings for frontend and tests

---

Changes walkthrough 📝

	Relevant files
Enhancement	16 files index.html New main HTML implementing redesigned frontend UI                +151/-0  index.js New JavaScript for interactive frontend logic                        +397/-0  index.css Main CSS for new frontend, imports modular styles                +85/-0    colors.css Color variables for new frontend theme                                      +36/-0    fonts.css Font-face definitions for Inter font                                          +22/-0    main.css Main content and text styling for frontend                              +58/-0    button.css Button styling for test and share actions                                +83/-0    server-selector.css Styles for server selection dropdown                                          +171/-0  results.css Gauge and results display styling                                                +260/-0  dialog.css Dialog/modal styling for privacy and sharing                          +132/-0  server-list.json Example server list for frontend server selector                  +409/-0  settings.json Example settings for frontend configuration                            +9/-0      entrypoint.sh Update entrypoint for new frontend deployment logic            +6/-3      Dockerfile Add frontend directory to build context                                    +1/-0      Dockerfile.alpine Add frontend and PHP extension for Alpine builds                  +2/-1      ui.php Adjust server list logic for new modes                                      +14/-2
Documentation	2 files README.md Documentation for new frontend usage and credits                  +76/-0    README.md Add documentation for test procedure                                          +4/-0
Tests	3 files docker-compose.yml Update test compose for new frontend and dual mode              +27/-0    test-script.sh Improve test script robustness and timing                                +9/-2      servers.json Example multi-server list for test scenarios                          +409/-0

---

Need help?

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

Check out the documentation for more information.

[qodo-merge-for-open-source]

PR Reviewer Guide 🔍

(Review updated until commit dda7841)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 585 - Fully compliant Compliant requirements: Implemented redesigned UI based on fromScratch Studio's designs Implemented desktop view design Implemented mobile view design Created responsive layout that works on different screen sizes Maintained core functionality of the speed test Improved usability and user experience

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The code lacks robust error handling for network failures or API errors. For example, in the applySettingsJSON and applyServerListJSON functions, errors are caught but only logged to console, with no user-facing error messages. async function applySettingsJSON() { try { const response = await fetch("settings.json"); const settings = await response.json(); if (!settings || typeof settings !== "object") { return console.error("Settings are empty or malformed"); } for (let setting in settings) { testState.speedtest.setParameter(setting, settings[setting]); if ( setting == "telemetry_level" && settings[setting] && settings[setting] != "off" && settings[setting] != "disabled" && settings[setting] != "false" ) { testState.telemetryEnabled = true; document.querySelector("#privacy-warning").classList.remove("hidden"); } } } catch (error) { console.error("Failed to fetch settings:", error); } } /** * Load server list from server-list.json on the server and populate the * dropdown */ async function applyServerListJSON() { try { const response = await fetch("server-list.json"); const servers = await response.json(); if (!servers || !Array.isArray(servers) || servers.length === 0) { return console.error("Server list is empty or malformed"); } testState.servers = servers; populateDropdown(testState.servers); if (servers.length > 1) { testState.speedtest.addTestPoints(servers); testState.speedtest.selectServer((server) => { if (server) { selectServer(server); } else { alert( "Can't reach any of the speedtest servers! But you're on this page. Something weird is going on with your network." ); } }); } } catch (error) { console.error("Failed to fetch server list:", error); } } Security Concern The alert message on line 157-160 could expose information about the network configuration that might be useful for attackers. Consider using a more generic error message. alert( "Can't reach any of the speedtest servers! But you're on this page. Something weird is going on with your network." ); } Logic Issue The conditional structure has a mismatched 'fi' statement. The 'fi' on line 49 closes the if statement started on line 44, but there's no matching 'fi' for the if statement started on line 42. # Set up index.php for frontend-only or standalone modes if [[ "$MODE" == "frontend" || "$MODE" == "dual" ]]; then cp -av /speedtest/frontend/* /var/www/html/ elif [ "$MODE" == "standalone" ]; then cp -av /speedtest/frontend/* /var/www/html/ echo '[{"name":"local","server":"/backend", "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "", "sponsorURL": "", "id":1 }]' > /var/www/html/server-list.json fi

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Latest suggestions up to 4fc0932

Category	Suggestion	Impact
Possible issue	Fix mode logic inconsistency There's a logical error in the conditional blocks. In the first block, if mode is "standalone" or "backend", SPEEDTEST_SERVERS is set to an empty array. But then in the dual mode block, it tries to add a server to SPEEDTEST_SERVERS, which would be empty if mode is "standalone". This contradicts the entrypoint.sh logic where standalone mode should have a local server. docker/ui.php [14-30] -if($mode=="standalone" || $mode=="backend"){ ?> +if($mode=="backend"){ ?> var SPEEDTEST_SERVERS=[]; <?php } else { ?> var SPEEDTEST_SERVERS= <?= file_get_contents('/servers.json') ?: '[]' ?>; <?php } -if ($mode=="dual"){ ?> -// add own server in dual mode +if ($mode=="dual" || $mode=="standalone"){ ?> +// add own server in dual or standalone mode SPEEDTEST_SERVERS.unshift({ "name":"This Server", "server":document.location.href+"backend/", "id":1, "dlURL":"garbage.php", "ulURL":"empty.php", "pingURL":"empty.php", "getIpURL":"getIP.php", "pingT":-1 }) Apply this suggestion Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical logical error in the mode handling. The current code sets SPEEDTEST_SERVERS to empty for standalone mode but then tries to add a server in dual mode only, which would cause standalone mode to have no servers. The fix properly aligns with the entrypoint.sh logic.	High
	Fix mode configuration consistency The server-list.json file is only created for standalone mode, but the ui.php logic expects it for dual mode as well. This creates an inconsistency where dual mode might not have the proper server configuration. docker/entrypoint.sh [43-49] # Set up index.php for frontend-only or standalone modes -if [[ "$MODE" == "frontend" || "$MODE" == "dual" ]]; then +if [[ "$MODE" == "frontend" || "$MODE" == "dual" || "$MODE" == "standalone" ]]; then cp -av /speedtest/frontend/* /var/www/html/ -elif [ "$MODE" == "standalone" ]; then - cp -av /speedtest/frontend/* /var/www/html/ - echo '[{"name":"local","server":"/backend", "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "", "sponsorURL": "", "id":1 }]' > /var/www/html/server-list.json + if [[ "$MODE" == "standalone" ]]; then + echo '[{"name":"local","server":"/backend", "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "", "sponsorURL": "", "id":1 }]' > /var/www/html/server-list.json + fi fi Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential inconsistency between the entrypoint.sh and ui.php handling of dual mode. The improved code consolidates the logic for copying frontend files and creates a more maintainable structure that ensures proper configuration across different modes.	Medium
General	Fix duplicate server names Multiple servers have the same name "Amsterdam, Netherlands" with different IDs (51 and 94). This can cause confusion for users when selecting a server and may lead to unexpected behavior if the application relies on unique server names. frontend/server-list.json [2-12] { - "name": "Amsterdam, Netherlands", + "name": "Amsterdam, Netherlands (Clouvider)", "server": "//ams.speedtest.clouvider.net/backend", "id": 51, "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "Clouvider", "sponsorURL": "https://www.clouvider.co.uk/" }, Apply this suggestion Suggestion importance[1-10]: 7 __ Why: Having multiple servers with identical names ("Amsterdam, Netherlands") but different IDs (51 and 94) could confuse users when selecting a server. Adding the sponsor name to differentiate them improves usability and prevents potential selection errors.	Medium
Possible issue	Complete test loop implementation The test script has a loop structure but doesn't contain any actual commands inside the loops, making them ineffective. Add commands to test each container combination. docker/test/test-script.sh [15-16] for db in sqlite pg mysql; do for distro in alpine debian; do + echo "Testing $distro with $db database..." + w3m -dump http://speedtest-${distro}-${db}:${PORT}/ || echo "Failed to connect to $distro with $db" + done +done Apply this suggestion Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the test script has empty loops without any actual testing commands. Adding the w3m commands to test each container combination is critical for the test script to function properly.	High
Security	Fix privacy policy placeholder The privacy policy contains a placeholder email address that needs to be replaced with a real contact email. This could cause compliance issues with privacy regulations if deployed as-is. frontend/index.html [145] -<a href="mailto:PUT@YOUR_EMAIL.HERE">TO BE FILLED BY DEVELOPER</a> +<a href="mailto:${EMAIL}">Contact Us</a> Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion identifies a critical issue where a placeholder email address in the privacy policy could cause compliance problems with privacy regulations. Using an environment variable would properly configure the contact email.	Medium
	Use explicit HTTPS protocol Protocol-relative URLs (starting with "//") can cause issues in certain contexts, especially when accessed via file:// protocol or in certain embedded contexts. Using explicit HTTPS is more secure and reliable. frontend/server-list.json [4-11] { "name": "Amsterdam, Netherlands", - "server": "//ams.speedtest.clouvider.net/backend", + "server": "https://ams.speedtest.clouvider.net/backend", "id": 51, "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "Clouvider", "sponsorURL": "https://www.clouvider.co.uk/" }, [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: Protocol-relative URLs can cause issues in certain contexts like file:// access or embedded environments. Using explicit HTTPS improves security and reliability, making this a valuable suggestion that affects multiple server entries in the file.	Medium
General	Handle invalid gauge values The rotation calculation for the gauge could produce NaN if speed is negative, which would cause visual issues. Add a check to ensure speed is a positive number before performing calculations. frontend/javascript/index.js [382-383] -// Make sure we stay within bounds at all times +// Make sure we stay within bounds at all times and handle invalid values +if (isNaN(rotation)) return 0; return Math.max(Math.min(rotation, maxRotation), minRotation); Apply this suggestion Suggestion importance[1-10]: 7 __ Why: This suggestion addresses a potential bug where NaN values could cause visual issues in the gauge display. Adding a check for NaN values improves the robustness of the UI rendering code.	Medium
	Fix dialog positioning The large fixed margin-top of 23rem pushes dialogs far down the page, potentially making them inaccessible on smaller screens. While there is a mobile media query, it only applies at max-width 800px, leaving medium-sized screens with potential issues. frontend/styling/dialog.css [10-14] -margin-top: 23rem; +margin: auto; background: var(--popup-background-color); border: none; border-radius: 0.8rem; Apply this suggestion Suggestion importance[1-10]: 6 __ Why: The fixed margin-top of 23rem could push dialogs too far down on medium-sized screens, potentially causing accessibility issues. The suggestion to use auto margin is reasonable and would improve the dialog positioning across different screen sizes.	Low
More

---

Previous suggestions

✅ Suggestions up to commit dda7841

Category	Suggestion	Impact
Possible issue	Fix conditional logic error There's a logical error in the conditional checks. The "dual" mode is first checked with "standalone" (line 1), then separately (line 7). This means in "dual" mode, SPEEDTEST_SERVERS will be set to empty array first, then the code tries to add a server to it with unshift(), which won't work as expected. docker/ui.php [14-19] -if($mode=="standalone" || $mode=="backend"){ ?> +if($mode=="standalone"){ ?> +var SPEEDTEST_SERVERS=[]; +<?php } else if($mode=="backend") { ?> var SPEEDTEST_SERVERS=[]; <?php } else { ?> var SPEEDTEST_SERVERS= <?= file_get_contents('/servers.json') ?: '[]' ?>; <?php } if ($mode=="dual"){ ?> Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical logical error in the conditional checks. In dual mode, SPEEDTEST_SERVERS would be set to an empty array first, then the code would try to add a server to it with unshift(), which would result in incorrect behavior. The fix properly separates the conditions.	High
	✅ Add missing CSS units The width and height properties are missing units (px, rem, etc.), which can cause rendering issues. Add appropriate units to these values. frontend/styling/server-selector.css [31-39] & > div.chevron { content: ""; position: absolute; display: block; - width: 32; - height: 32; + width: 32px; + height: 32px; right: 1.8rem; top: 1rem; } [Suggestion has been applied] Suggestion importance[1-10]: 8 __ Why: The width and height properties are missing units (px, rem, etc.), which will cause rendering inconsistencies across browsers. This is a clear CSS error that needs to be fixed.	Medium
	Add missing sanity check The script is missing the actual sanitycheck call mentioned in the comment. The script installs w3m but doesn't use it to perform any tests on the containers. docker/test/test-script.sh [1-10] #!/bin/sh ## simple script to call sanitycheck on all containers PORT=8080 if ! apk add w3m; then echo "Failed to install w3m package" exit 1 fi +# Function to perform sanity check +perform_sanity_check() { + echo "Performing sanity check on $1..." + w3m -dump http://localhost:$PORT/ +} + Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the script installs w3m but doesn't actually use it to perform any sanity checks as mentioned in the comment. Adding a function to perform these checks would improve the script's functionality and align with its stated purpose.	Medium
	Add missing display property The dialog element is missing the display: flex property which is required for flex-direction, align-items, and justify-content to work properly when the dialog is not open. frontend/styling/dialog.css [3-8] dialog { + display: flex; flex-direction: column; align-items: center; justify-content: center; width: 70vw; height: 70vh; margin: auto; margin-top: 23rem; Suggestion importance[1-10]: 6 __ Why: The dialog element needs display: flex for the flex properties to work properly when the dialog is not open. The animation works when open because of the explicit display:flex in the [open] selector.	Low
Security	Use explicit HTTPS protocol The server URLs starting with "//" (protocol-relative URLs) may cause issues in some contexts. Consider using explicit HTTPS protocol for better security and reliability. frontend/server-list.json [2-12] { "name": "Amsterdam, Netherlands", - "server": "//ams.speedtest.clouvider.net/backend", + "server": "https://ams.speedtest.clouvider.net/backend", "id": 51, "dlURL": "garbage.php", "ulURL": "empty.php", "pingURL": "empty.php", "getIpURL": "getIP.php", "sponsorName": "Clouvider", "sponsorURL": "https://www.clouvider.co.uk/" }, Suggestion importance[1-10]: 7 __ Why: Using protocol-relative URLs (starting with "//") can cause issues in certain contexts, especially when accessed via file:// protocol. Explicitly using HTTPS improves security and reliability.	Medium
Possible issue	Fix logarithmic calculation The Math.log10(minSpeed + 1) calculation is problematic because minSpeed is 0, so this always evaluates to Math.log10(1) which equals 0. This makes the denominator in the power calculation potentially very small, which could lead to unexpected results. Remove the unnecessary logarithm of minSpeed. frontend/javascript/index.js [364-384] function mbpsToRotation(speed, oscillate) { speed = Number(speed); if (speed <= 0) return 0; const minSpeed = 0; const maxSpeed = 10000; // 10 Gbps maxes out the gauge const minRotation = 0; const maxRotation = 180; - // Can't do log10 of values less than one, +1 all to keep it fair - const logMinSpeed = Math.log10(minSpeed + 1); + // Use logarithmic scale for better visualization of wide range of speeds const logMaxSpeed = Math.log10(maxSpeed + 1); const logSpeed = Math.log10(speed + 1); - const power = (logSpeed - logMinSpeed) / (logMaxSpeed - logMinSpeed); + const power = logSpeed / logMaxSpeed; const oscillation = oscillate ? 1 + 0.01 * Math.sin(Date.now() / 100) : 1; const rotation = power * oscillation * maxRotation; // Make sure we stay within bounds at all times return Math.max(Math.min(rotation, maxRotation), minRotation); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a mathematical issue where calculating log10(minSpeed + 1) is redundant since minSpeed is 0. The improved code simplifies the calculation while maintaining the logarithmic scale, making the gauge calculation more efficient and clearer.	Medium
	Add null check The selectServer function doesn't handle potential errors if the server parameter is null or undefined, which could happen if server selection fails. Add a check to prevent potential errors when trying to access properties of an invalid server object. frontend/javascript/index.js [212-216] function selectServer(server) { + if (!server) { + console.error("Cannot select server: Invalid server object"); + return; + } testState.speedtest.setSelectedServer(server); testState.selectedServerDirty = true; testState.state = READY; } Suggestion importance[1-10]: 6 __ Why: The suggestion adds a defensive null check to prevent potential errors if the server parameter is undefined or null. This is a good defensive programming practice that improves code robustness, especially since server selection could potentially fail.	Low
General	Add error handling The function doesn't handle errors that might occur when trying to access the clipboard API, which may fail due to permissions or browser compatibility issues. Add try-catch to handle potential errors gracefully. frontend/javascript/index.js [98-108] async function copyLinkButtonClickHandler() { - const link = document.querySelector("img#results").src; - await navigator.clipboard.writeText(link); - const button = document.querySelector("#copy-link"); - button.classList.add("active"); - button.textContent = "Copied!"; - setTimeout(() => { - button.classList.remove("active"); - button.textContent = "Copy link"; - }, 3000); + try { + const link = document.querySelector("img#results").src; + await navigator.clipboard.writeText(link); + const button = document.querySelector("#copy-link"); + button.classList.add("active"); + button.textContent = "Copied!"; + setTimeout(() => { + button.classList.remove("active"); + button.textContent = "Copy link"; + }, 3000); + } catch (error) { + console.error("Failed to copy to clipboard:", error); + alert("Could not copy link to clipboard. Your browser may not support this feature."); + } } Suggestion importance[1-10]: 7 __ Why: The suggestion adds proper error handling for clipboard operations which might fail due to permissions or browser compatibility issues. This improves user experience by providing feedback when clipboard operations fail instead of silently failing.	Medium
	Improve dialog positioning The fixed margin-top: 23rem for dialogs can cause them to appear partially off-screen on smaller viewports. This should be removed or adjusted in the mobile media query to ensure dialogs are properly centered on all devices. frontend/styling/dialog.css [10] -margin-top: 23rem; +margin: auto; Suggestion importance[1-10]: 7 __ Why: The fixed margin-top of 23rem could cause dialogs to appear off-screen on smaller viewports. Changing to just margin: auto would improve responsive behavior and ensure dialogs are properly centered on all devices.	Medium
	Fix gauge pointer positioning Using position: fixed for gauge pointers can cause positioning issues when scrolling or when the page has multiple gauges. This is a hack that may lead to unexpected behavior. Consider using a different approach like position: absolute with proper z-index management. frontend/styling/results.css [118-120] -position: fixed; +position: absolute; top: calc(var(--gauge-width) / 2 - var(--speed-width) / 3); left: var(--progress-width); +z-index: 2; Suggestion importance[1-10]: 5 __ Why: Using position: fixed for gauge pointers can cause positioning issues when scrolling or with multiple gauges. While the code includes a comment acknowledging this as a hack, switching to position: absolute with proper z-indexing would be more reliable.	Low

[Copilot]

Pull Request Overview

This PR implements the new frontend design for LibreSpeed by introducing a complete redesign of the user interface, updating the build system, and adding documentation and tests for the new features.

• Introduces a modern HTML/CSS/JS frontend in index.html and index.js
• Updates Docker configuration and test scripts to support new frontend modes
• Provides updated documentation and sample configuration files

Reviewed Changes

Copilot reviewed 17 out of 31 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
frontend/javascript/index.js	New interactive JS logic for handling speed test state and UI updates
frontend/index.html	Redesigned HTML structure and UI elements based on the new design
frontend/README.md	Updated documentation for using and configuring the new frontend
docker/test/docker-compose.yml	Enhanced Docker compose file to support dual frontend/server modes
docker/test/README.md	Documentation for running end-to-end tests using the updated test configurations

Files not reviewed (14)

• Dockerfile: Language not supported
• Dockerfile.alpine: Language not supported
• docker/entrypoint.sh: Language not supported
• docker/test/test-script.sh: Language not supported
• docker/ui.php: Language not supported
• frontend/settings.json: Language not supported
• frontend/styling/button.css: Language not supported
• frontend/styling/colors.css: Language not supported
• frontend/styling/dialog.css: Language not supported
• frontend/styling/fonts.css: Language not supported
• frontend/styling/index.css: Language not supported
• frontend/styling/main.css: Language not supported
• frontend/styling/results.css: Language not supported
• frontend/styling/server-selector.css: Language not supported