Fix setting window navigation update issue #3547
Conversation
…stead of IsInitialized flag
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv, onesounds Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the navigation update issues in the settings window by resetting the page type when navigation is not triggered by a button click and by using nullability checks for the view model instead of the IsInitialized flag.
- Updated the SettingsWindowViewModel to include a PageType property with property change notification.
- Modified settings pane view files to use a null-check on the view model and update PageType accordingly.
- Changed WelcomePage files to use a null check for view model initialization.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Flow.Launcher/ViewModel/SettingWindowViewModel.cs | Added PageType property with change notification. |
| Flow.Launcher/SettingWindow.xaml.cs | Updated event subscription pattern for view model property changes. |
| SettingsPane* files | Updated OnNavigatedTo logic to check for null view models and reset PageType. |
| WelcomePage* files | Replaced IsInitialized check with null check for view model initialization. |
📝 Walkthrough""" WalkthroughThis change refactors the initialization logic for multiple settings and welcome pages by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingWindow
participant SettingWindowViewModel
participant SettingsPane (any)
participant NavigationView
User->>SettingWindow: Open Settings
SettingWindow->>SettingWindowViewModel: Initialize, set DataContext
SettingWindow->>NavigationView: Attach PropertyChanged handler
User->>NavigationView: Select or navigate to a page (mouse/button/side button)
NavigationView->>SettingsPane: Navigate to selected page
SettingsPane->>SettingWindowViewModel: Set PageType to current page type
SettingWindowViewModel-->>SettingWindow: Notify PropertyChanged(PageType)
SettingWindow->>NavigationView: Update selected item to match PageType
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs (1)
14-15: "Ioc" flagged by spell checkerThe static analysis tool flags "Ioc" as an unrecognized word, but this is a common abbreviation for Inversion of Control in dependency injection contexts.
Consider adding "Ioc" to the project's custom dictionary to prevent these spell check warnings in the future.
🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 15-15:
Iocis not a recognized word. (unrecognized-spelling)
[warning] 14-14:
Iocis not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 14-14: Spell check warning:
Iocis not a recognized word. (unrecognized-spelling)
[warning] 15-15: Spell check warning:
Iocis not a recognized word. (unrecognized-spelling)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs(1 hunks)Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs
- Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs
- Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs
🧰 Additional context used
🧬 Code Graph Analysis (1)
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs (5)
Flow.Launcher/ViewModel/WelcomeViewModel.cs (1)
WelcomeViewModel(5-67)Flow.Launcher/Resources/Pages/WelcomePage4.xaml.cs (1)
OnNavigatedTo(13-24)Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs (1)
OnNavigatedTo(15-26)Flow.Launcher/Resources/Pages/WelcomePage3.xaml.cs (1)
OnNavigatedTo(13-24)Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
OnNavigatedTo(16-27)
🪛 GitHub Check: Check Spelling
Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs
[warning] 13-13:
Ioc is not a recognized word. (unrecognized-spelling)
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs
[warning] 15-15:
Ioc is not a recognized word. (unrecognized-spelling)
[warning] 14-14:
Ioc is not a recognized word. (unrecognized-spelling)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs
[warning] 13-13: Spell check warning: Ioc is not a recognized word. (unrecognized-spelling)
[warning] 28-28: Spell check warning: Ioc is not a recognized word. (unrecognized-spelling)
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs
[warning] 14-14: Spell check warning: Ioc is not a recognized word. (unrecognized-spelling)
[warning] 15-15: Spell check warning: Ioc is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (4)
Flow.Launcher/Resources/Pages/WelcomePage2.xaml.cs (2)
14-15: Improved initialization pattern with dependency injectionThe change from lazy initialization to eager initialization with dependency injection is a good improvement. Making the view model readonly prevents accidental reassignment and ensures consistent state throughout the page lifecycle.
🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 15-15:
Iocis not a recognized word. (unrecognized-spelling)
[warning] 14-14:
Iocis not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 14-14: Spell check warning:
Iocis not a recognized word. (unrecognized-spelling)
[warning] 15-15: Spell check warning:
Iocis not a recognized word. (unrecognized-spelling)
19-21: Consistent page number reset during navigationMoving the page number reset before the initialization check ensures that the page type is always updated correctly regardless of how the navigation was triggered (via buttons or otherwise). This directly addresses the navigation update issue mentioned in the PR objectives.
Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs (2)
12-13: Improved initialization pattern with dependency injectionSimilar to WelcomePage2, this change implements a more robust initialization approach using dependency injection. This pattern is already used elsewhere in this class (line 28 with
_translater), making this change consistent with the established code style.🧰 Tools
🪛 GitHub Check: Check Spelling
[warning] 13-13:
Iocis not a recognized word. (unrecognized-spelling)🪛 GitHub Actions: Check Spelling
[warning] 13-13: Spell check warning:
Iocis not a recognized word. (unrecognized-spelling)
17-20: Consistent page number reset during navigationMoving the page number reset before the initialization check ensures that navigation state is properly maintained. This change, consistent with other welcome pages, directly supports the PR objective of addressing navigation update issues in the settings window.
Fix setting window navigation update issue
If the navigation is not triggered by button click, so we need to reset the page type in all setting pages.
And in setting window, we need to update the selected item in navigation view.
Check view model null instead of
IsInitializedflagIf we use forward/backward button,
IsInitializedis still true so we need to check nullability of view model instance instead.Test
Confirmed by vzabrodin.
Resolve #3546.