Conversation
There was a problem hiding this comment.
Pull request overview
タスクトレイから「既定のプロファイル」をまとめて適用できるようにするため、appsettings に既定プロファイルIDの保存先を追加し、設定UIと適用処理（および未設定時の案内ダイアログ）を実装するPRです。
Changes:
- appsettings.json に既定プロファイルIDリスト（
defaultProfileIds）を追加 - 設定画面に「既定のプロファイル」セクションを追加し、チェックで保存できるように更新
- トレイメニューから既定プロファイルを順に適用し、未設定時はダイアログで設定誘導
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WindowController.Core/Models/AppSettings.cs | 既定プロファイルIDの永続化フィールドを追加 |
| src/WindowController.App/ViewModels/SettingsViewModel.cs | 既定プロファイル一覧のロード/保存/リセットロジックを追加 |
| src/WindowController.App/SettingsWindow.xaml | 設定UIに既定プロファイル選択用DataGridを追加 |
| src/WindowController.App/DefaultProfilesMissingDialog.xaml.cs | 既定プロファイル未設定時のダイアログ挙動を追加 |
| src/WindowController.App/DefaultProfilesMissingDialog.xaml | 未設定時ダイアログのUIを追加 |
| src/WindowController.App/App.xaml.cs | トレイメニューから既定プロファイルを適用する処理を追加 |
| README.md | トレイ/ホットキー/設定の説明を更新、appsettings.json の説明を追記 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+183
to
+198
| private void LoadDefaultProfiles() | ||
| { | ||
| DefaultProfiles.Clear(); | ||
| var defaultIds = _appSettingsStore.Data.DefaultProfileIds; | ||
| foreach (var profile in _profileStore.Data.Profiles) | ||
| { | ||
| var item = new DefaultProfileItem | ||
| { | ||
| ProfileId = profile.Id, | ||
| ProfileName = profile.Name, | ||
| IsDefault = defaultIds.Contains(profile.Id) | ||
| }; | ||
| item.PropertyChanged += DefaultProfileItem_PropertyChanged; | ||
| DefaultProfiles.Add(item); | ||
| } | ||
| } |
There was a problem hiding this comment.
DefaultProfileIds に profiles.json から削除されたプロファイルIDが残っている場合、ここでは一覧に表示されず UI から解除できません（設定ファイルにゴミが残り続けます）。LoadDefaultProfiles() で _profileStore.Data.Profiles に存在しないIDを検出して DefaultProfileIds から除去して保存するか、欠損IDを「見つからないプロファイル」として一覧に表示して解除できるようにしてください。
Comment on lines
+205
to
+214
| var ids = _appSettingsStore.Data.DefaultProfileIds; | ||
| if (item.IsDefault) | ||
| { | ||
| if (!ids.Contains(item.ProfileId)) | ||
| ids.Add(item.ProfileId); | ||
| } | ||
| else | ||
| { | ||
| ids.Remove(item.ProfileId); | ||
| } |
There was a problem hiding this comment.
DefaultProfileIds は「順に適用」とされていますが、チェックON時に ids.Add() しているためクリック順（OFF→ONで末尾に移動）で順序が変わり、UI 上の並び順とも一致しません。適用順を安定させたい場合は、変更のたびに DefaultProfiles の表示順で IsDefault==true のIDを再構築して保存する等、順序決定ロジックを明確にしてください。
src/WindowController.App/App.xaml.cs
Outdated
Comment on lines
+245
to
+297
| if (_profileApplier == null || _appSettingsStore == null || _profileStore == null) | ||
| return; | ||
|
|
||
| var defaultIds = _appSettingsStore.Data.DefaultProfileIds; | ||
| if (defaultIds.Count == 0) | ||
| { | ||
| var shouldPopup = _mainWindow == null || !_mainWindow.IsVisible || _mainWindow.WindowState == WindowState.Minimized; | ||
|
|
||
| if (shouldPopup) | ||
| { | ||
| var dlg = new DefaultProfilesMissingDialog(); | ||
|
|
||
| // WPF throws if Owner is set to a Window that has never been shown. | ||
| if (_mainWindow is { IsVisible: true }) | ||
| { | ||
| dlg.Owner = _mainWindow; | ||
| dlg.WindowStartupLocation = WindowStartupLocation.CenterOwner; | ||
| } | ||
| else | ||
| { | ||
| dlg.WindowStartupLocation = WindowStartupLocation.CenterScreen; | ||
| } | ||
|
|
||
| var openSettings = dlg.ShowDialog() == true; | ||
| if (openSettings) | ||
| ShowSettingsWindow(); | ||
| } | ||
| else if (_viewModel != null) | ||
| { | ||
| _viewModel.StatusText = "既定のプロファイルが設定されていません"; | ||
| } | ||
| _log?.Information("No default profiles configured"); | ||
| return; | ||
| } | ||
|
|
||
| nint appHwnd = 0; | ||
| if (_mainWindow != null) | ||
| { | ||
| var helper = new WindowInteropHelper(_mainWindow); | ||
| appHwnd = helper.Handle; | ||
| } | ||
|
|
||
| var messages = new List<string>(); | ||
| foreach (var profileId in defaultIds) | ||
| { | ||
| var result = await _profileApplier.ApplyByIdAsync(profileId, false, appHwnd); | ||
| var profile = _profileStore.FindById(profileId); | ||
| var name = profile?.Name ?? profileId; | ||
| messages.Add(result.ToStatusMessage(name)); | ||
| } | ||
|
|
||
| if (_viewModel != null) | ||
| _viewModel.StatusText = string.Join(" / ", messages); |
There was a problem hiding this comment.
トレイメニューの Click ハンドラが async void になるため、ApplyDefaultProfilesAsync() 内で例外が発生するとプロセスが落ちる可能性があります。ApplyDefaultProfilesAsync() を try/catch で囲んでログ出力＋StatusText 反映を行うか、ハンドラ側で例外を捕捉して安全に失敗扱いにしてください。
Suggested change
| if (_profileApplier == null || _appSettingsStore == null || _profileStore == null) | |
| return; | |
| var defaultIds = _appSettingsStore.Data.DefaultProfileIds; | |
| if (defaultIds.Count == 0) | |
| { | |
| var shouldPopup = _mainWindow == null || !_mainWindow.IsVisible || _mainWindow.WindowState == WindowState.Minimized; | |
| if (shouldPopup) | |
| { | |
| var dlg = new DefaultProfilesMissingDialog(); | |
| // WPF throws if Owner is set to a Window that has never been shown. | |
| if (_mainWindow is { IsVisible: true }) | |
| { | |
| dlg.Owner = _mainWindow; | |
| dlg.WindowStartupLocation = WindowStartupLocation.CenterOwner; | |
| } | |
| else | |
| { | |
| dlg.WindowStartupLocation = WindowStartupLocation.CenterScreen; | |
| } | |
| var openSettings = dlg.ShowDialog() == true; | |
| if (openSettings) | |
| ShowSettingsWindow(); | |
| } | |
| else if (_viewModel != null) | |
| { | |
| _viewModel.StatusText = "既定のプロファイルが設定されていません"; | |
| } | |
| _log?.Information("No default profiles configured"); | |
| return; | |
| } | |
| nint appHwnd = 0; | |
| if (_mainWindow != null) | |
| { | |
| var helper = new WindowInteropHelper(_mainWindow); | |
| appHwnd = helper.Handle; | |
| } | |
| var messages = new List<string>(); | |
| foreach (var profileId in defaultIds) | |
| { | |
| var result = await _profileApplier.ApplyByIdAsync(profileId, false, appHwnd); | |
| var profile = _profileStore.FindById(profileId); | |
| var name = profile?.Name ?? profileId; | |
| messages.Add(result.ToStatusMessage(name)); | |
| } | |
| if (_viewModel != null) | |
| _viewModel.StatusText = string.Join(" / ", messages); | |
| try | |
| { | |
| if (_profileApplier == null || _appSettingsStore == null || _profileStore == null) | |
| return; | |
| var defaultIds = _appSettingsStore.Data.DefaultProfileIds; | |
| if (defaultIds.Count == 0) | |
| { | |
| var shouldPopup = _mainWindow == null || !_mainWindow.IsVisible || _mainWindow.WindowState == WindowState.Minimized; | |
| if (shouldPopup) | |
| { | |
| var dlg = new DefaultProfilesMissingDialog(); | |
| // WPF throws if Owner is set to a Window that has never been shown. | |
| if (_mainWindow is { IsVisible: true }) | |
| { | |
| dlg.Owner = _mainWindow; | |
| dlg.WindowStartupLocation = WindowStartupLocation.CenterOwner; | |
| } | |
| else | |
| { | |
| dlg.WindowStartupLocation = WindowStartupLocation.CenterScreen; | |
| } | |
| var openSettings = dlg.ShowDialog() == true; | |
| if (openSettings) | |
| ShowSettingsWindow(); | |
| } | |
| else if (_viewModel != null) | |
| { | |
| _viewModel.StatusText = "既定のプロファイルが設定されていません"; | |
| } | |
| _log?.Information("No default profiles configured"); | |
| return; | |
| } | |
| nint appHwnd = 0; | |
| if (_mainWindow != null) | |
| { | |
| var helper = new WindowInteropHelper(_mainWindow); | |
| appHwnd = helper.Handle; | |
| } | |
| var messages = new List<string>(); | |
| foreach (var profileId in defaultIds) | |
| { | |
| var result = await _profileApplier.ApplyByIdAsync(profileId, false, appHwnd); | |
| var profile = _profileStore.FindById(profileId); | |
| var name = profile?.Name ?? profileId; | |
| messages.Add(result.ToStatusMessage(name)); | |
| } | |
| if (_viewModel != null) | |
| _viewModel.StatusText = string.Join(" / ", messages); | |
| } | |
| catch (System.Exception ex) | |
| { | |
| _log?.Error(ex, "Failed to apply default profiles"); | |
| if (_viewModel != null) | |
| { | |
| _viewModel.StatusText = "既定のプロファイルの適用に失敗しました"; | |
| } | |
| } |
Comment on lines
+52
to
55
| | **GUI を開く** | メイン画面を表示 | | ||
| | **設定…** | 設定画面を表示 | | ||
| | **プロファイルを適用(配置のみ)** | メイン画面を表示（適用は GUI でプロファイルを選択） | | ||
| | **終了** | アプリを終了 | |
There was a problem hiding this comment.
README のトレイメニュー説明が実装と不一致です。実装では「既定のプロファイルを適用」ですが、README では「プロファイルを適用(配置のみ)」になっています。実際のメニュー名と動作（既定プロファイルを適用する）に合わせて更新してください。
Comment on lines
+86
to
+100
| ### appsettings.json について | ||
|
|
||
| `appsettings.json` はアプリ全体の設定（`profiles.json` の保存先・ホットキー設定など）を保持します。 | ||
|
|
||
| ```json | ||
| { | ||
| "profilesPath": "C:\\Path\\To\\profiles.json", | ||
| "hotkeys": { | ||
| "showGui": { "key": "W", "ctrl": true, "alt": true, "shift": false, "win": false }, | ||
| "profiles": { | ||
| "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx": { "key": "1", "ctrl": true, "alt": false, "shift": false, "win": false } | ||
| } | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
appsettings.json の例に今回追加した defaultProfileIds が含まれていないため、既定プロファイル機能の設定方法が読み手に伝わりません。README の JSON 例に defaultProfileIds（配列）を追記し、適用順序の扱いも併記してください。
Comment on lines
+26
to
+31
| /// <summary> | ||
| /// Profile Ids to apply when "既定のプロファイルを適用" is executed. | ||
| /// Multiple profiles can be set and all will be applied in order. | ||
| /// </summary> | ||
| [JsonPropertyName("defaultProfileIds")] | ||
| public List<string> DefaultProfileIds { get; set; } = new(); |
There was a problem hiding this comment.
DefaultProfileIds を追加したことで AppSettings のデフォルト値/シリアライズ仕様が増えています。既存の AppSettingsTests があるので、少なくとも「既定値は空配列」「(de)serialize 後も保持される」「フィールド欠落時は空配列になる」を確認するテストを追加してください。
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.
No description provided.