Skip to content

feat: Connect Device dialog matching design philosophy#488

Merged
tylerkron merged 4 commits intomainfrom
claude/distracted-gagarin-0ef665
Apr 18, 2026
Merged

feat: Connect Device dialog matching design philosophy#488
tylerkron merged 4 commits intomainfrom
claude/distracted-gagarin-0ef665

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

Test plan

  • Launch app on Windows and open Connect Device dialog
  • WiFi tab: scan indicator appears, discovered devices render as tiles with purple stripe, selection highlights with accent border, Connect button enables on selection
  • Manual WiFi tab: IP + port fields accept input, Connect button wires to manual-wifi command
  • USB tab: serial devices render with cyan stripe, scan + connect flow works
  • Manual USB tab: port field + Connect button
  • Firmware tab: HID devices render with amber stripe, Open Firmware button wires through
  • Close button + MetroWindow chrome still behave correctly
  • Confirm dialog looks cohesive with the Devices pane beneath it

🤖 Generated with Claude Code

Redesigns the Connect Device dialog to match the dark visual system
established in the Channels, Devices, Settings, and navigation-rail
panes. Tile-based device lists with colored type stripes (purple for
WiFi, cyan for USB, amber for firmware), segmented tab strip, accent
pill actions. Palette pulled from DesignTokens.xaml — only the two
connection-type stripe brushes remain local per the color-carries-
meaning principle.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner April 18, 2026 20:24
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Redesign Connect Device dialog with dark tile-based UI system

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Redesigns Connect Device dialog with dark tile-based UI matching design system
• Implements segmented tab strip with colored connection-type stripes (purple WiFi, cyan USB, amber
  firmware)
• Replaces ListView with styled ListBox tiles featuring accent borders on selection
• Adds accent pill buttons, dark text fields, and scanning state overlays
• Pulls palette from DesignTokens.xaml; increases dialog size to 560×500
Diagram
flowchart LR
  A["Old ListView<br/>layout"] -->|"Replace with<br/>tile-based ListBox"| B["Device tiles<br/>with stripes"]
  C["Basic buttons"] -->|"Style as<br/>accent pills"| D["Rounded accent<br/>buttons"]
  E["Simple TabControl"] -->|"Add segmented<br/>strip styling"| F["Dark tab control<br/>with borders"]
  G["Inline text fields"] -->|"Wrap in<br/>dark borders"| H["Styled dark<br/>text fields"]
  I["No scanning UI"] -->|"Add overlay<br/>with spinner"| J["Centered scanning<br/>state"]
  B --> K["Cohesive dark<br/>dialog"]
  D --> K
  F --> K
  H --> K
  J --> K
Loading

Grey Divider

File Changes

1. Daqifi.Desktop/View/ConnectionDialog.xaml ✨ Enhancement +458/-105

Complete UI redesign with tiles, stripes, and dark styling

• Adds comprehensive Window.Resources section with custom styles for segmented tabs, tile list
 boxes, accent pill buttons, dark text fields, and field labels
• Replaces ListView controls with styled ListBox controls featuring 3px colored stripe indicators
 (purple for WiFi, cyan for USB, amber for firmware)
• Redesigns device tile templates with two-column layout: left column for device info (name,
 address/port, serial), right column for type badge and firmware version
• Implements segmented tab strip with dark background, border separators, and active/hover states
• Adds scanning state overlays with ProgressRing and descriptive text for each tab
• Restructures layout with Grid-based action bars separated by top borders instead of DockPanel
• Updates window title to "CONNECT DEVICE", increases dimensions to 560×500, sets Grid background to
 Surface color
• Removes image references and replaces with styled badge elements

Daqifi.Desktop/View/ConnectionDialog.xaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 18, 2026

Code Review by Qodo

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

Grey Divider


Action required

1. Close races async connect🐞 Bug ☼ Reliability
Description
The Connect buttons execute async connect commands and then immediately close the window; the
Closing handler calls ConnectionDialogViewModel.Close(), which cancels/disposes discovery while the
connect task may still be running, risking intermittent exceptions and double-dispose behavior.
Code

Daqifi.Desktop/View/ConnectionDialog.xaml[R279-284]

+                        <Button HorizontalAlignment="Right"
+                                Style="{StaticResource AccentPillButton}"
+                                CommandParameter="{Binding ElementName=DeviceList, Path=SelectedItems}"
+                                Command="{Binding ConnectCommand}"
+                                Click="btnConnect_Click"
+                                Content="Connect"/>
Evidence
The buttons invoke async commands (ConnectCommand/ConnectSerialCommand/etc.) and also call
btnConnect_Click which closes the window. Closing triggers MetroWindow_Closing which calls
vm.Close(), and vm.Close() cancels/disposes discovery (including a fire-and-forget
StopSerialDiscoveryAsync) while the connect command may still be running (e.g., ConnectSerialAsync
awaits StopSerialDiscoveryAsync). This creates a concrete race between connect execution and
cleanup/disposal.

Daqifi.Desktop/View/ConnectionDialog.xaml[12-14]
Daqifi.Desktop/View/ConnectionDialog.xaml[279-285]
Daqifi.Desktop/View/ConnectionDialog.xaml[390-396]
Daqifi.Desktop/View/ConnectionDialog.xaml.cs[16-25]
Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[188-197]
Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[424-430]
Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[447-487]

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

## Issue description
Connect buttons both run async connect commands and immediately close the window. The window Closing handler calls `ConnectionDialogViewModel.Close()` which cancels/disposes discovery while connect may still be running, creating a race/double-dispose risk.

## Issue Context
- `Button` has both `Command=...` and `Click="btnConnect_Click"`.
- `btnConnect_Click` calls `Close()`.
- `Closing="MetroWindow_Closing"` calls `vm.Close()`.
- `vm.Close()` cancels/disposes discovery (serial stop is fire-and-forget).

## Fix Focus Areas
- Daqifi.Desktop/View/ConnectionDialog.xaml[279-285]
- Daqifi.Desktop/View/ConnectionDialog.xaml[390-396]
- Daqifi.Desktop/View/ConnectionDialog.xaml.cs[16-25]
- Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[424-487]

## Suggested fix approach
- Remove `Click="btnConnect_Click"` from connect buttons and do **not** close the dialog until the async command completes.
- If you want to preserve “auto-close on success”, implement an explicit close request after connect completion (e.g., ViewModel raises an event/message; the view closes in response), and ensure `vm.Close()` cleanup runs only after connect completes.
- Alternatively, move command execution into an `async` click handler (no `Command` binding) that `await`s the relevant VM method, then calls `Close()`.

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



Remediation recommended

2. Connect closes with no selection🐞 Bug ≡ Correctness
Description
The WiFi/USB Connect buttons are always enabled and always close the dialog; if no items are
selected, the command receives an empty SelectedItems list and no connection is attempted, but the
dialog still closes.
Code

Daqifi.Desktop/View/ConnectionDialog.xaml[R279-284]

+                        <Button HorizontalAlignment="Right"
+                                Style="{StaticResource AccentPillButton}"
+                                CommandParameter="{Binding ElementName=DeviceList, Path=SelectedItems}"
+                                Command="{Binding ConnectCommand}"
+                                Click="btnConnect_Click"
+                                Content="Connect"/>
Evidence
The WiFi Connect button binds CommandParameter to DeviceList.SelectedItems and calls
btnConnect_Click which closes the dialog unconditionally. In the VM, ConnectAsync iterates the
provided enumerable; when empty, it performs no connects. Result: user can click Connect without
selecting any device and the dialog closes without connecting.

Daqifi.Desktop/View/ConnectionDialog.xaml[279-285]
Daqifi.Desktop/View/ConnectionDialog.xaml.cs[16-19]
Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[176-186]

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

## Issue description
Connect is clickable with no selection and closes the dialog, but the VM connects to nothing when `SelectedItems` is empty.

## Issue Context
The button binds `CommandParameter` to `SelectedItems` and also closes the window via `Click="btnConnect_Click"`.

## Fix Focus Areas
- Daqifi.Desktop/View/ConnectionDialog.xaml[279-285]
- Daqifi.Desktop/View/ConnectionDialog.xaml[390-396]
- Daqifi.Desktop/View/ConnectionDialog.xaml.cs[16-19]
- Daqifi.Desktop/ViewModels/ConnectionDialogViewModel.cs[176-197]

## Suggested fix approach
- Prevent closing/connecting when the selection is empty:
 - Add `IsEnabled` logic to the Connect buttons based on selection (e.g., a small `Count>0` converter for `SelectedItems.Count`), **and/or**
 - Add a guard in `ConnectAsync`/`ConnectSerialAsync` to return early when `selectedItems` is empty, and only close the window after at least one device was queued/connected.

ⓘ 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 thread Daqifi.Desktop/View/ConnectionDialog.xaml
@tylerkron
Copy link
Copy Markdown
Contributor Author

Response to Qodo review

Bug #1 — Close races async connect: Disagreed for this PR. Replied in the inline thread — the Click="btnConnect_Click" + Command=... pattern is pre-existing (unchanged since the SVN migration in ec6602d), not a regression introduced by this redesign. Proper fix needs async lifecycle coordination in ConnectionDialogViewModel; flagging as a follow-up.

Bug #2 — Connect closes with no selection: Disagreed for this PR, same reasoning — pre-existing behavior. ListBox.SelectedItems is not a DependencyProperty and doesn't raise PropertyChanged, so wiring IsEnabled to SelectedItems.Count requires either a SelectionChanged handler or a behavior; the buttons also don't share a consistent param shape (some bind SelectedItems, some bind text inputs). A holistic fix that disables every action button on its own empty-state condition, plus guards the VM commands to no-op and not auto-close on empty input, belongs together with Bug #1 in a follow-up reliability/UX PR, not here.

Addresses the two Qodo-flagged bugs on this PR. Both are pre-existing
behavior since the SVN migration in ec6602d; consolidating the fix here
since it touches the same buttons and code paths as the visual redesign.

The Connect buttons wired both `Command` (async) and
`Click="btnConnect_Click"` (sync `Close()`), which closed the window
while the connect task was still running. The window's Closing handler
then disposed discovery resources a second time — racing any cleanup the
connect task had already started, and closing the dialog even when
nothing was selected (empty SelectedItems → no connect, but still
closed).

Replace the click handler with a CloseRequested event raised by the VM
only after each connect command's awaited work succeeds. Add empty-input
guards so clicking Connect with no selection no longer closes the
dialog. Make ConnectionDialogViewModel.Close idempotent so
double-invocation from the connect path and the Closing handler can't
double-dispose the discovery finders.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerkron
Copy link
Copy Markdown
Contributor Author

Update — consolidated the Qodo-flagged fixes into this PR after all (commit e6b13b8). Separating them made no practical sense: same buttons, same code paths, guaranteed conflict with whichever merged first. Folded the follow-up branch in here and closed #491.

Bug 1 (close race) — fixed: removed Click="btnConnect_Click" from the four action buttons; ConnectionDialogViewModel now raises a new CloseRequested event after each connect command's awaited work completes; the view subscribes and closes itself. ConnectionDialogViewModel.Close is idempotent so concurrent Closing + connect-path cleanup can't double-dispose.

Bug 2 (connect with no selection) — fixed: added explicit empty-input guards at the top of each connect command; the new ToStreamingDevices helper makes the check unambiguous. Empty selection → command returns early → CloseRequested not raised → dialog stays open.

Tests: added Daqifi.Desktop.Test/ViewModels/ConnectionDialogViewModelCloseTests.cs covering both empty-input paths and Close idempotency.

tylerkron and others added 2 commits April 18, 2026 14:46
The redesigned Connect Device dialog replaced the device-tile thumbnail
with styled badges, so Images/Nq.png is no longer referenced anywhere.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

📊 Code Coverage Report

Summary

Summary
Generated on: 4/18/2026 - 8:52:36 PM
Coverage date: 4/18/2026 - 8:52:01 PM - 4/18/2026 - 8:52:31 PM
Parser: MultiReport (4x Cobertura)
Assemblies: 3
Classes: 119
Files: 150
Line coverage: 17.4% (1509 of 8663)
Covered lines: 1509
Uncovered lines: 7154
Coverable lines: 8663
Total lines: 24692
Branch coverage: 17.8% (521 of 2921)
Covered branches: 521
Total branches: 2921
Method coverage: Feature is only available for sponsors

Coverage

DAQiFi - 17.2%
Name Line Branch
DAQiFi 17.2% 17.8%
Daqifi.Desktop.App 5.4% 0%
Daqifi.Desktop.Channel.AbstractChannel 40.9% 27.7%
Daqifi.Desktop.Channel.AnalogChannel 58.7% 25%
Daqifi.Desktop.Channel.Channel 11.5% 0%
Daqifi.Desktop.Channel.ChannelColorManager 100% 100%
Daqifi.Desktop.Channel.DataSample 91.6%
Daqifi.Desktop.Channel.DigitalChannel 65.2% 25%
Daqifi.Desktop.Commands.CompositeCommand 0% 0%
Daqifi.Desktop.Commands.HostCommands 0%
Daqifi.Desktop.Commands.WeakEventHandlerManager 0% 0%
Daqifi.Desktop.Configuration.FirewallConfiguration 90.6% 66.6%
Daqifi.Desktop.Configuration.WindowsFirewallWrapper 64% 68.4%
Daqifi.Desktop.ConnectionManager 42.4% 39.2%
Daqifi.Desktop.Converters.BoolToActiveStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToConnectionStatusConverter 0% 0%
Daqifi.Desktop.Converters.BoolToStatusColorConverter 0% 0%
Daqifi.Desktop.Converters.BrushColorMatchConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToColorConverter 0% 0%
Daqifi.Desktop.Converters.ConnectionTypeToUsbConverter 0% 0%
Daqifi.Desktop.Converters.InvertedBoolToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.ListToStringConverter 0% 0%
Daqifi.Desktop.Converters.NotNullToVisibilityConverter 0% 0%
Daqifi.Desktop.Converters.OxyColorToBrushConverter 0% 0%
Daqifi.Desktop.Converters.StringRightConverter 0% 0%
Daqifi.Desktop.Device.AbstractStreamingDevice 42.9% 38.6%
Daqifi.Desktop.Device.DeviceMessage 0%
Daqifi.Desktop.Device.Firmware.BootloaderSessionStreamingDeviceAdapter 0% 0%
Daqifi.Desktop.Device.Firmware.WifiPromptDelayProcessRunner 0% 0%
Daqifi.Desktop.Device.NativeMethods 100%
Daqifi.Desktop.Device.SerialDevice.SerialStreamingDevice 27.6% 30.8%
Daqifi.Desktop.Device.WiFiDevice.DaqifiStreamingDevice 40.9% 39.4%
Daqifi.Desktop.DialogService.DialogService 0% 0%
Daqifi.Desktop.DialogService.ServiceLocator 0% 0%
Daqifi.Desktop.DiskSpace.DiskSpaceCheckResult 100%
Daqifi.Desktop.DiskSpace.DiskSpaceEventArgs 100%
Daqifi.Desktop.DiskSpace.DiskSpaceMonitor 88.2% 86.6%
Daqifi.Desktop.DuplicateDeviceCheckResult 100%
Daqifi.Desktop.Exporter.OptimizedLoggingSessionExporter 66.5% 62.7%
Daqifi.Desktop.Exporter.SampleData 100%
Daqifi.Desktop.Helpers.BooleanConverter`1 0% 0%
Daqifi.Desktop.Helpers.BooleanToInverseBoolConverter 0% 0%
Daqifi.Desktop.Helpers.BooleanToVisibilityConverter 0%
Daqifi.Desktop.Helpers.EnumDescriptionConverter 100% 100%
Daqifi.Desktop.Helpers.IntToVisibilityConverter 0% 0%
Daqifi.Desktop.Helpers.MinMaxDownsampler 98.6% 97.9%
Daqifi.Desktop.Helpers.MyMultiValueConverter 0%
Daqifi.Desktop.Helpers.NaturalSortHelper 100% 100%
Daqifi.Desktop.Helpers.VersionHelper 98.2% 66.2%
Daqifi.Desktop.Logger.DatabaseLogger 0% 0%
Daqifi.Desktop.Logger.DatabaseMigrator 0% 0%
Daqifi.Desktop.Logger.DeviceLegendGroup 100% 100%
Daqifi.Desktop.Logger.LoggedSeriesLegendItem 0% 0%
Daqifi.Desktop.Logger.LoggingContext 100%
Daqifi.Desktop.Logger.LoggingContextDesignTimeFactory 0%
Daqifi.Desktop.Logger.LoggingManager 0% 0%
Daqifi.Desktop.Logger.LoggingSession 16% 5%
Daqifi.Desktop.Logger.PlotLogger 0% 0%
Daqifi.Desktop.Logger.SessionDeviceMetadata 80%
Daqifi.Desktop.Logger.SummaryLogger 0% 0%
Daqifi.Desktop.Logger.TimestampGapDetector 95% 83.3%
Daqifi.Desktop.Loggers.ImportOptions 0%
Daqifi.Desktop.Loggers.ImportProgress 0% 0%
Daqifi.Desktop.Loggers.SdCardSessionImporter 0% 0%
Daqifi.Desktop.MainWindow 0% 0%
Daqifi.Desktop.Migrations.AddSamplesSessionTimeIndex 0%
Daqifi.Desktop.Migrations.AddSessionDeviceMetadata 0%
Daqifi.Desktop.Migrations.AddSessionSampleCount 0%
Daqifi.Desktop.Migrations.InitialSQLiteMigration 0%
Daqifi.Desktop.Migrations.LoggingContextModelSnapshot 0%
Daqifi.Desktop.Models.AddProfileModel 0%
Daqifi.Desktop.Models.DaqifiSettings 80.5% 83.3%
Daqifi.Desktop.Models.DebugDataCollection 6.6% 0%
Daqifi.Desktop.Models.DebugDataModel 0% 0%
Daqifi.Desktop.Models.Notifications 0%
Daqifi.Desktop.Models.SdCardFile 0% 0%
Daqifi.Desktop.Services.WindowsPrincipalAdminChecker 0%
Daqifi.Desktop.Services.WpfMessageBoxService 0%
Daqifi.Desktop.UpdateVersion.VersionNotification 0% 0%
Daqifi.Desktop.View.AddProfileConfirmationDialog 0% 0%
Daqifi.Desktop.View.AddprofileDialog 0% 0%
Daqifi.Desktop.View.ConnectionDialog 0% 0%
Daqifi.Desktop.View.DebugWindow 0% 0%
Daqifi.Desktop.View.DeviceLogsView 0% 0%
Daqifi.Desktop.View.DuplicateDeviceDialog 0% 0%
Daqifi.Desktop.View.ErrorDialog 0% 0%
Daqifi.Desktop.View.ExportDialog 0% 0%
Daqifi.Desktop.View.FirmwareDialog 0% 0%
Daqifi.Desktop.View.Flyouts.FirmwareFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LiveGraphFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.LoggedSessionFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.NotificationsFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.SummaryFlyout 0% 0%
Daqifi.Desktop.View.Flyouts.UpdateProfileFlyout 0% 0%
Daqifi.Desktop.View.MigrationStatusWindow 0% 0%
Daqifi.Desktop.View.MinimapInteractionController 0% 0%
Daqifi.Desktop.View.Prototype.ChannelsPanePrototype 0% 0%
Daqifi.Desktop.View.Prototype.DevicesPanePrototype 0% 0%
Daqifi.Desktop.View.SuccessDialog 0% 0%
Daqifi.Desktop.ViewModels.AddProfileConfirmationDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.AddProfileDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.ChannelsPaneViewModel 0% 0%
Daqifi.Desktop.ViewModels.ChannelTileViewModel 0% 0%
Daqifi.Desktop.ViewModels.ConnectionDialogViewModel 37.3% 39.1%
Daqifi.Desktop.ViewModels.DaqifiViewModel 14.2% 8.1%
Daqifi.Desktop.ViewModels.DeviceLogsViewModel 0% 0%
Daqifi.Desktop.ViewModels.DevicesPaneViewModel 0% 0%
Daqifi.Desktop.ViewModels.DeviceTileViewModel 0% 0%
Daqifi.Desktop.ViewModels.DuplicateDeviceDialogViewModel 0%
Daqifi.Desktop.ViewModels.ErrorDialogViewModel 0%
Daqifi.Desktop.ViewModels.ExportDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.FirmwareDialogViewModel 0% 0%
Daqifi.Desktop.ViewModels.SettingsViewModel 0% 0%
Daqifi.Desktop.ViewModels.SuccessDialogViewModel 85.7%
Daqifi.Desktop.WindowViewModelMapping.IWindowViewModelMappingsContract 0%
Daqifi.Desktop.WindowViewModelMapping.WindowViewModelMappings 0%
Sentry.Generated.BuildPropertyInitializer 100%
Daqifi.Desktop.Common - 30.8%
Name Line Branch
Daqifi.Desktop.Common 30.8% 16.6%
Daqifi.Desktop.Common.Loggers.AppLogger 33.7% 16.6%
Daqifi.Desktop.Common.Loggers.NoOpLogger 0%
Daqifi.Desktop.IO - 100%
Name Line Branch
Daqifi.Desktop.IO 100% ****
Daqifi.Desktop.IO.Messages.MessageEventArgs`1 100%

Coverage report generated by ReportGeneratorView full report in build artifacts

@tylerkron tylerkron merged commit 82180d2 into main Apr 18, 2026
2 checks passed
@tylerkron tylerkron deleted the claude/distracted-gagarin-0ef665 branch April 18, 2026 20:55
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.

1 participant