Skip to content

feat: Add server selection dialog for speed test - #623

Merged
PeterJhongLinksys merged 5 commits into
dev-2.0.0from
feat/server-selection-dialog
Feb 11, 2026
Merged

feat: Add server selection dialog for speed test#623
PeterJhongLinksys merged 5 commits into
dev-2.0.0from
feat/server-selection-dialog

Conversation

@AustinChangLinksys

Copy link
Copy Markdown
Collaborator

Summary

  • Add server selection dialog before running speed test when servers are available
  • Update SpeedTestWidget with showServerSelectionDialog parameter
  • Add tests for server selection dialog functionality (IVER-SERV_DLG, DHOME-SERV_DLG, STV-SERVER)

Test plan

  • Verify server selection dialog appears when tapping Go button with servers available
  • Verify selecting a server triggers speed test
  • Verify cancelling dialog does not run speed test
  • Run flutter test test/page/instant_verify/views/localizations/instant_verify_view_test.dart --name "server selection"
  • Run flutter test test/page/health_check/views/localizations/speed_test_view_test.dart --name "server selection"

🤖 Generated with Claude Code

- Add server selection dialog before running speed test when servers are available
- Update SpeedTestWidget with showServerSelectionDialog parameter
- Add IVER-SERV_DLG and DHOME-SERV_DLG tests for dialog functionality
- Add STV-SERVER tests for server selection on Speed Test view
- Add healthCheckServersData test data for server list

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add server selection dialog for speed test functionality

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add server selection dialog for speed test across multiple views
  - Shows dialog when tapping Go button with available servers
  - Allows users to select server before running speed test
• Refactor server selection into reusable dialog component
  - Extract common dialog logic from SpeedTestView
  - Implement in SpeedTestWidget and CustomSpeedTest
• Replace server selection modal with dropdown in SpeedTestView
  - Simplifies UI with persistent dropdown instead of icon button
  - Improves accessibility and discoverability
• Add comprehensive tests for server selection functionality
  - Test server dialog in dashboard, instant verify, and speed test views
  - Verify dialog appearance and server selection flow
Diagram
flowchart LR
  User["User taps Go button"]
  Check{"Servers available?"}
  Dialog["Show server selection dialog"]
  Select["User selects server"]
  Test["Run speed test with selected server"]
  User --> Check
  Check -->|Yes| Dialog
  Check -->|No| Test
  Dialog --> Select
  Select --> Test
Loading

Grey Divider

File Changes

1. lib/page/dashboard/views/components/widgets/atomic/speed_test.dart ✨ Enhancement +52/-5

Add server selection dialog to CustomSpeedTest widget

• Add _runSpeedTestWithServerSelection method to handle server selection before speed test
• Add _showServerSelectionDialog method to display server list in a dialog
• Update Go button onPressed to call new server selection method instead of directly running test
• Import dialog and HealthCheckServer dependencies

lib/page/dashboard/views/components/widgets/atomic/speed_test.dart


2. lib/page/health_check/shared_widgets/speed_test_widget.dart ✨ Enhancement +93/-21

Add server selection dialog support to SpeedTestWidget

• Add showServerSelectionDialog parameter to control dialog behavior
• Add _runSpeedTestWithServerSelection method to handle server selection logic
• Add _showServerSelectionDialog method to display server selection dialog
• Update all Go button and retry button handlers to use new server selection method
• Add text overflow handling for download/upload speed display with Flexible wrapper
• Import dialog and HealthCheckServer dependencies

lib/page/health_check/shared_widgets/speed_test_widget.dart


3. lib/page/health_check/views/speed_test_view.dart ✨ Enhancement +36/-49

Replace modal dialog with dropdown for server selection

• Remove server selection dialog method and related imports
• Replace icon button with persistent AppDropdown for server selection
• Set showServerSelectionDialog: false in SpeedTestWidget to use dropdown instead
• Refactor layout to include server dropdown above speed test widget on both mobile and desktop
• Simplify server selection UX from modal dialog to dropdown component

lib/page/health_check/views/speed_test_view.dart


View more (5)
4. test/page/dashboard/localizations/dashboard_home_view_test.dart 🧪 Tests +75/-5

Add server selection dialog test for dashboard home

• Add _speedTestDialogScreens list for screens that render SpeedTestWidget with Go button
• Add DHOME-SERV_DLG test case to verify server selection dialog appears on Go button tap
• Update image precaching to include multiple router and speedtest images
• Import HealthCheckServer and health_check_provider dependencies

test/page/dashboard/localizations/dashboard_home_view_test.dart


5. test/page/health_check/views/localizations/speed_test_selection_view_test.dart 🧪 Tests +101/-0

Add tests for speed test selection view

• New test file for SpeedTestSelectionView component
• Add STSEL-INIT-01 test to verify both speed test options are enabled
• Add STSEL-DISABLED-01 test to verify UI when router speed test is not supported
• Test connectivity state and health check module support mocking

test/page/health_check/views/localizations/speed_test_selection_view_test.dart


6. test/page/health_check/views/localizations/speed_test_view_test.dart 🧪 Tests +105/-10

Add server selection tests to speed test view

• Replace STV-ACTION-01 test with two new server-related tests
• Add STV-SERVER-01 test to verify server dropdown selection flow
• Add STV-SERVER-02 test to verify Go button runs test with selected server
• Update test documentation to reflect new server selection tests
• Import HealthCheckServer dependency

test/page/health_check/views/localizations/speed_test_view_test.dart


7. test/page/instant_verify/views/localizations/instant_verify_view_test.dart 🧪 Tests +45/-6

Add server selection dialog test for instant verify

• Add IVER-SERV_DLG test case to verify server selection dialog appears on Go button
• Verify dialog displays server list with correct server names
• Update test documentation to include server dialog test case
• Import HealthCheckServer and health_check_provider dependencies
• Simplify ping and traceroute tests by using pumpAndSettle instead of fixed duration pump

test/page/instant_verify/views/localizations/instant_verify_view_test.dart


8. test/test_data/health_check_state_data.dart 🧪 Tests +28/-0

Add mock server data for testing

• Add healthCheckServersData constant with mock server list containing 3 test servers
• Include server details: ID, name, location, country, hostname, and port
• Provides test data for server selection dialog tests across multiple views

test/test_data/health_check_state_data.dart


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Feb 10, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Hardcoded dropdown hint ----- 📘 Rule violation ✓ Correctness
Description
• The server dropdown uses a hardcoded hint string ('-----') instead of the app localization
mechanism. • This introduces a user-facing string that won’t be translated and breaks localization
consistency across locales. • It also forces tests to assert a literal placeholder rather than a
localized value, increasing brittleness.
Code

lib/page/health_check/views/speed_test_view.dart[R54-60]

+                child: AppDropdown<HealthCheckServer>(
+                  items: servers,
+                  value: selectedServer,
+                  itemAsString: (s) => s.toString(),
+                  hint: '-----',
+                  onChanged: (server) {
+                    if (server != null) {
Evidence
PR Compliance ID 12 requires user-facing strings to be introduced via the Flutter i18n/intl workflow
(ARB-based localization). The new dropdown hint is a hardcoded literal string in SpeedTestView,
which is user-visible and not localized.

CLAUDE.md
lib/page/health_check/views/speed_test_view.dart[54-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SpeedTestView` uses a hardcoded dropdown hint (`&amp;amp;amp;amp;amp;#x27;-----&amp;amp;amp;amp;amp;#x27;`) which is user-facing and not localized.
## Issue Context
The project uses ARB-based localization (`lib/l10n/*.arb`) and `loc(context)` lookups. Any new user-facing strings should be added via the localization workflow.
## Fix Focus Areas
- lib/page/health_check/views/speed_test_view.dart[47-66]
- lib/l10n/app_en.arb[785-805]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


✅ 2. Unused ref parameter 📘 Rule violation ✓ Correctness
Description
_showServerSelectionDialog declares a WidgetRef ref parameter that is never used. • This
typically triggers an analyzer hint (unused_parameter) and can fail CI if analyzer warnings are
treated as violations. • Keeping unused parameters also reduces code clarity and invites confusion
about intended behavior.
Code

lib/page/health_check/shared_widgets/speed_test_widget.dart[R672-676]

+  Future<HealthCheckServer?> _showServerSelectionDialog(
+    BuildContext context,
+    WidgetRef ref,
+    List<HealthCheckServer> servers,
+  ) async {
Evidence
PR Compliance ID 8 requires new/modified Dart code to introduce no new analyzer/linter violations.
The private method _showServerSelectionDialog includes an unused parameter (WidgetRef ref),
which commonly triggers analyzer warnings/hints such as unused_parameter.

CLAUDE.md
lib/page/health_check/shared_widgets/speed_test_widget.dart[672-676]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_showServerSelectionDialog` has an unused `WidgetRef ref` parameter, which can trigger analyzer warnings.
## Issue Context
The repository’s compliance requires no new analyzer/lint violations in modified Dart code.
## Fix Focus Areas
- lib/page/health_check/shared_widgets/speed_test_widget.dart[68-99]
- lib/page/health_check/shared_widgets/speed_test_widget.dart[670-699]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Dialog server list may overflow 📘 Rule violation ⛯ Reliability
Description
• The server selection dialog renders all servers inside a Column without any scrolling or
height constraints. • With a larger server list (or smaller screens), this can cause layout
overflow/exception and prevent the user from selecting a server. • This is an unhandled UI edge case
that should degrade gracefully (e.g., scrollable list with max height).
Code

lib/page/health_check/shared_widgets/speed_test_widget.dart[R682-695]

+          return Column(
+            mainAxisSize: MainAxisSize.min,
+            crossAxisAlignment: CrossAxisAlignment.stretch,
+            children: servers.map((server) {
+              return AppListTile(
+                key: Key('server_${server.serverID}'),
+                title: AppText.bodyMedium(server.serverName.isNotEmpty
+                    ? server.serverName
+                    : server.serverHostname),
+                onTap: () => Navigator.of(dialogContext, rootNavigator: true)
+                    .pop(server),
+              );
+            }).toList(),
+          );
Evidence
PR Compliance ID 3 requires explicit handling of edge cases and graceful degradation. The dialog
builds an unbounded list of tiles using a Column(... children: servers.map(...)), which can
overflow when servers is large or the viewport is small.

Rule 3: Generic: Robust Error Handling and Edge Case Management
lib/page/health_check/shared_widgets/speed_test_widget.dart[682-695]
lib/page/dashboard/views/components/widgets/atomic/speed_test.dart[64-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The server selection dialog uses a non-scrollable `Column` for an arbitrary-length server list, which can overflow.
## Issue Context
A server list size is external data and can grow. The dialog should handle small viewports and large lists gracefully.
## Fix Focus Areas
- lib/page/health_check/shared_widgets/speed_test_widget.dart[680-696]
- lib/page/dashboard/views/components/widgets/atomic/speed_test.dart[59-80]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Async state timing issues 🐞 Bug ⛯ Reliability
Description
• The Go action reads healthCheckProvider state synchronously, but that state is populated
asynchronously (loadData()); on fast user interaction this can produce timing-dependent behavior
(e.g., treating the internal module as unsupported, or skipping server selection because servers
is still empty). • This is hard to reproduce and will appear as inconsistent UX depending on
device/network speed rather than deterministic logic.
Code

lib/page/health_check/shared_widgets/speed_test_widget.dart[R76-99]

+    final isSpeedCheckSupported = ref
+        .read(healthCheckProvider.select((s) => s.isSpeedTestModuleSupported));
+
+    if (!isSpeedCheckSupported) {
+      // If not supported, navigate to the external test page.
+      context.pushNamed(RouteNamed.speedTestExternal);
+      return;
+    }
+
+    // Show server selection if enabled and servers are available
+    if (showServerSelectionDialog) {
+      final servers = ref.read(healthCheckProvider).servers;
+      if (servers.isNotEmpty) {
+        final selected =
+            await _showServerSelectionDialog(context, ref, servers);
+        if (selected == null) return; // User cancelled
+
+        ref.read(healthCheckProvider.notifier).setSelectedServer(selected);
+      }
+    }
+
+    // Run the speed test
+    ref.read(healthCheckProvider.notifier).runHealthCheck(Module.speedtest);
+  }
Evidence
HealthCheckProvider returns an empty initial state and loads modules/servers asynchronously. The
new Go handler uses ref.read(...) to decide whether to route externally and whether to show server
selection, so it can act on the still-empty initial state if tapped before loadData() completes.

lib/page/health_check/providers/health_check_provider.dart[37-66]
lib/page/health_check/providers/health_check_state.dart[45-83]
lib/page/health_check/shared_widgets/speed_test_widget.dart[72-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The speed-test “Go” handler makes decisions (internal vs external, whether to show server selection) based on `healthCheckProvider` state that is loaded asynchronously. If the user taps before `loadData()` completes, behavior becomes timing-dependent.
### Issue Context
`HealthCheckProvider.build()` returns `HealthCheckState.init()` immediately and triggers `loadData()` asynchronously. `SpeedTestWidget` uses `ref.read(...)` to decide flow at tap-time.
### Fix Focus Areas
- lib/page/health_check/shared_widgets/speed_test_widget.dart[68-99]
- lib/page/health_check/providers/health_check_provider.dart[37-67]
- lib/page/health_check/providers/health_check_state.dart[45-83]
### Suggested approach
- Introduce a simple “dataLoaded”/“modulesLoaded” flag in `HealthCheckState` (or infer via a sentinel) and disable the Go button until it’s true.
- If HealthCheckManager2 is supported, consider triggering/awaiting `loadServers()` before presenting selection logic (or disable Go until servers load).
- Keep external navigation only when you can confidently determine the module is unsupported (not merely ‘not loaded yet’).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


✅ 5. Go disable contract mismatch 🐞 Bug ✓ Correctness
Description
SpeedTestWidget documents that when showServerSelectionDialog is false, the Go button should
be disabled if no server is selected; however the Go button is always enabled and the handler always
runs. • SpeedTestView explicitly sets showServerSelectionDialog: false and allows
selectedServer to be null (dropdown shows a hint), so users can tap Go without selecting a server
despite the widget contract.
Code

lib/page/health_check/shared_widgets/speed_test_widget.dart[R551-566]

   // markerRadius: 2,
   centerBuilder: (context, value) {
+          // Button is always enabled - the method handles server selection logic
+          final buttonColor = Theme.of(context).colorScheme.primary;
+
     return SizedBox(
       width: meterSize == null ? 102 : meterSize! / 2.0,
       height: meterSize == null ? 102 : meterSize! / 2.0,
       child: Material(
         shape: const CircleBorder(),
-              color: Theme.of(context).colorScheme.primary,
+              color: buttonColor,
         child: InkWell(
           key: const Key('goBtn'),
           customBorder: const CircleBorder(),
-                onTap: () {
-                  final isSpeedCheckSupported = ref.watch(healthCheckProvider
-                      .select((s) => s.isSpeedTestModuleSupported));
-                  if (isSpeedCheckSupported) {
-                    ref
-                        .read(healthCheckProvider.notifier)
-                        .runHealthCheck(Module.speedtest);
-                  } else {
-                    // If not supported, navigate to the external test page.
-                    context.pushNamed(RouteNamed.speedTestExternal);
-                  }
-                },
+                onTap: () => _runSpeedTestWithServerSelection(context, ref),
           child: Center(
Evidence
The widget’s own API comment states the button should be disabled when dialog is off and there is no
server selection. The actual _startButton always wires onTap, and
_runSpeedTestWithServerSelection does not gate running on selected server presence when
showServerSelectionDialog is false. SpeedTestView configures that exact mode
(showServerSelectionDialog: false) while allowing selectedServer to be null.

lib/page/health_check/shared_widgets/speed_test_widget.dart[51-66]
lib/page/health_check/shared_widgets/speed_test_widget.dart[541-606]
lib/page/health_check/shared_widgets/speed_test_widget.dart[85-99]
lib/page/health_check/views/speed_test_view.dart[26-67]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SpeedTestWidget`’s `showServerSelectionDialog` contract says the Go button is disabled when no server is selected and the dialog is disabled. Current implementation always enables Go and always runs.
### Issue Context
`SpeedTestView` sets `showServerSelectionDialog: false` and allows `selectedServer` to remain null (dropdown hint), so the user can tap Go in a state the widget contract says should be disabled.
### Fix Focus Areas
- lib/page/health_check/shared_widgets/speed_test_widget.dart[51-66]
- lib/page/health_check/shared_widgets/speed_test_widget.dart[541-606]
- lib/page/health_check/shared_widgets/speed_test_widget.dart[72-99]
- lib/page/health_check/views/speed_test_view.dart[26-67]
### Suggested approach
- In `_startButton`, compute `isGoEnabled` based on:
- `showServerSelectionDialog == true` =&amp;amp;amp;amp;amp;gt; enabled
- `showServerSelectionDialog == false` =&amp;amp;amp;amp;amp;gt; enabled only if `ref.read(healthCheckProvider).selectedServer != null` (and/or `servers.isEmpty`)
- Alternatively, keep current behavior but update the docstring to reflect reality.
- If disabled, consider showing helper text near the dropdown (“Select a server to start”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +54 to +60
child: AppDropdown<HealthCheckServer>(
items: servers,
value: selectedServer,
itemAsString: (s) => s.toString(),
hint: '-----',
onChanged: (server) {
if (server != null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Hardcoded dropdown hint ----- 📘 Rule violation ✓ Correctness

• The server dropdown uses a hardcoded hint string ('-----') instead of the app localization
  mechanism.
• This introduces a user-facing string that won’t be translated and breaks localization consistency
  across locales.
• It also forces tests to assert a literal placeholder rather than a localized value, increasing
  brittleness.
Agent Prompt
## Issue description
`SpeedTestView` uses a hardcoded dropdown hint (`'-----'`) which is user-facing and not localized.

## Issue Context
The project uses ARB-based localization (`lib/l10n/*.arb`) and `loc(context)` lookups. Any new user-facing strings should be added via the localization workflow.

## Fix Focus Areas
- lib/page/health_check/views/speed_test_view.dart[47-66]
- lib/l10n/app_en.arb[785-805]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +672 to +676
Future<HealthCheckServer?> _showServerSelectionDialog(
BuildContext context,
WidgetRef ref,
List<HealthCheckServer> servers,
) async {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Unused ref parameter 📘 Rule violation ✓ Correctness

_showServerSelectionDialog declares a WidgetRef ref parameter that is never used.
• This typically triggers an analyzer hint (unused_parameter) and can fail CI if analyzer warnings
  are treated as violations.
• Keeping unused parameters also reduces code clarity and invites confusion about intended behavior.
Agent Prompt
## Issue description
`_showServerSelectionDialog` has an unused `WidgetRef ref` parameter, which can trigger analyzer warnings.

## Issue Context
The repository’s compliance requires no new analyzer/lint violations in modified Dart code.

## Fix Focus Areas
- lib/page/health_check/shared_widgets/speed_test_widget.dart[68-99]
- lib/page/health_check/shared_widgets/speed_test_widget.dart[670-699]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

- Use localization for dropdown hint instead of hardcoded '-----'
- Remove unused ref parameter from _showServerSelectionDialog
- Add scroll constraint to server list dialog to prevent overflow
- Update test to use localized hint text

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@AustinChangLinksys
AustinChangLinksys force-pushed the feat/server-selection-dialog branch from 754d7ec to afaf727 Compare February 10, 2026 06:00
@PeterJhongLinksys
PeterJhongLinksys merged commit 6dda14e into dev-2.0.0 Feb 11, 2026
2 checks passed
@PeterJhongLinksys
PeterJhongLinksys deleted the feat/server-selection-dialog branch February 11, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants