Merged
Conversation
…tcNow DateTime.UtcNow causes an implicit conversion when assigned to a DateTimeOffset, which loses the explicit UTC offset semantics. DateTimeOffset.UtcNow is the correct and explicit way to capture the current UTC time as a DateTimeOffset. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…served exceptions The window-based FrontShowAsync overload used fire-and-forget `_ = HandleCloseInternalAsync(...)` while the view-based overload correctly awaited it. This meant any exception thrown during dialog close (OnDialogClosingAsync, OnDialogClosedAsync) would be silently lost as an UnobservedTaskException. Fixed to await the close task consistently with the view overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The faulted task branch only read t.Exception to mark it observed but discarded the information entirely. Exceptions from fire-and-forget Show() dialog operations would vanish without any trace, making them impossible to diagnose. Now logs the exception via Debug.WriteLine so they are at least visible during development. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Manager Methods that return Task must follow the Async naming convention per .NET guidelines. IRegionManager had GoForward/GoBack without the Async suffix, while IRegion correctly used GoForwardAsync/GoBackAsync - causing inconsistency within the same framework. Updated all call sites in samples and tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… deprecation The XML docs already stated MaxCachedViews was obsolete, but without the [Obsolete] attribute the compiler emits no warning, making the deprecation notice invisible to callers. Added the attribute so IDEs and the compiler surface the warning automatically. Internal usages are suppressed with #pragma warning disable CS0618. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Members (missing 'h') The public API methods had a typo: 'Wit' instead of 'With'. Introduced correctly-spelled AddSingletonWithAllMembers and AddTransientWithAllMembers as the canonical versions, and marked the misspelled originals with [Obsolete] pointing to the new names to preserve backwards compatibility without breaking existing callers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…avigationService 'Resovle' was a transposition typo. This is an internal private method so there is no public API impact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…U race Two issues fixed: 1. Used non-generic WeakReference which lacks type safety and requires casting. Replaced with WeakReference<object> to match the generic API. 2. TOCTOU (time-of-check/time-of-use) race: previously checked IsAlive then invoked the handler, but GC could collect the target between the two operations. Fixed by using TryGetTarget() to atomically obtain a strong reference, preventing the target from being collected while the handler runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…on after cancellation CancelAsync() only signals the CancellationToken; it does not wait for the job task to actually finish. Without awaiting WaitAllAsync() afterwards, a new navigation job could start while the previous job is still executing its catch/finally cleanup, causing concurrent navigation state corruption. Fixed by awaiting WaitAllAsync() after CancelAllAsync() in the CancelCurrent branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…text arguments Static factory methods (Success, Failure, Cancelled) were calling navigationContext.UpdateStatus() as a side effect, which violates the principle that factory methods should not modify their arguments. Callers cannot predict or control when the status changes, making the context lifecycle opaque. Moved all UpdateStatus() calls to the call sites (RegionBase, RegionManagerBase) so the state transition is explicit and visible at each navigation outcome site. Factory methods now only read the context's current state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…elf-comparison bug MergeFrom is called as NavigationOptions.Default.MergeFrom(userOptions), meaning 'this == Default'. Comparing other.X against Default.X was actually comparing against this.X — a self-comparison that would silently fail if Default had already been mutated, and prevented users from ever restoring a property back to its default value. Extracted factory default values as internal constants (DefaultMaxHistoryItems, etc.) and compare against those instead. This ensures the comparison is stable regardless of the current state of the Default instance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onAware boilerplate INavigationAware has 5 methods and 1 event that all must be implemented, even when only a subset of the lifecycle is relevant. This forces users to write empty method bodies throughout their codebase. NavigationAwareBase provides sensible no-op defaults: - InitializeAsync/OnNavigatedToAsync/OnNavigatedFromAsync/OnUnloadAsync: do nothing - IsNavigationTargetAsync: returns true (always reuse cached instance) - AsyncRequestUnloadEvent: declared; RequestUnloadAsync() helper raises it Users can now inherit from NavigationAwareBase and only override the methods they need. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed changes) Previously there was no way for a view model to prevent navigation away from itself. A common use case is an edit form that should confirm before discarding unsaved changes. INavigationGuard.CanNavigateAsync() is called during the OnBeforeNavigation pipeline stage, before OnNavigatedFromAsync. Returning false throws OperationCanceledException, which causes the framework to revert to the current view (existing cancel behavior). The guard is checked only when the current view model implements it; no behavior change for view models that do not implement the interface. Works in combination with NavigationAwareBase and direct INavigationAware implementations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ogging, analytics) Previously there was no way to intercept all navigations globally without modifying every view model. Common needs: authentication redirects, analytics tracking, request logging, A/B testing redirects. INavigationInterceptor has two hooks: - OnNavigatingAsync: runs before the navigation pipeline; throw OperationCanceledException to abort (treated as navigation cancellation by the framework) - OnNavigatedAsync: runs after successful navigation; exceptions are logged but do not affect the navigation result Register via RegisterNavigationInterceptor<T>() extension. Multiple interceptors are supported and are called in registration order. The interceptors are resolved as singletons via IEnumerable<INavigationInterceptor>. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cific IAvaloniaDialogService FrontShowAsync was documented as 'particularly useful in AvaloniaUI' yet lived in the core IDialogService interface, forcing WPF users to be aware of an Avalonia startup pattern irrelevant to them. WPF had no equivalent concept in its platform service. Changes: - Added IAvaloniaDialogService (AsyncNavigation.Avalonia) extending IDialogService with FrontShowViewAsync/FrontShowWindowAsync using clean optional-parameter signatures - Added AvaloniaDialogService (internal) implementing both interfaces, delegating FrontShow calls to the existing DialogService.FrontShowAsync implementation - Avalonia DI setup now registers AvaloniaDialogService under both IDialogService and IAvaloniaDialogService so resolving either returns the same instance - Updated IDialogService.FrontShowAsync doc comments to point Avalonia users to IAvaloniaDialogService without breaking existing callers WPF users: no change, IDialogService still has FrontShowAsync for cross-platform use. Avalonia users: resolve IAvaloniaDialogService for FrontShowViewAsync/FrontShowWindowAsync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lass' constraint Minor follow-up to the IAvaloniaDialogService commit: - Remove CS0109 warning by removing unnecessary 'new' modifier on interface methods (the methods do not hide any members from IDialogService) - Add 'where TWindow : class' to explicit interface implementations in AvaloniaDialogService to satisfy the nullable reference type constraint required by the Func<IDialogResult, TWindow?> parameter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01d7d7e528
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
64
to
+65
| await CancelAllAsync(); | ||
| await WaitAllAsync(); |
There was a problem hiding this comment.
In HandleExistingJobs, the CancelCurrent branch now does await WaitAllAsync() immediately after cancellation, but WaitAllAsync() propagates TaskCanceledException/OperationCanceledException from the jobs that were just canceled. When a running job correctly throws on cancellation (e.g., Task.Delay(..., ct)), this exception escapes RunJobAsync before the new job is added, so the replacement navigation never starts. This breaks the core CancelCurrent behavior under normal cancellable workloads.
Useful? React with 👍 / 👎.
Comment on lines
+184
to
187
| public async Task<NavigationResult> GoForwardAsync(string regionName, CancellationToken cancellationToken = default) | ||
| { | ||
| var region = GetRegion(regionName); | ||
| if (await region.CanGoForwardAsync()) |
There was a problem hiding this comment.
The new interceptor contract says hooks run for every navigation request, but GoForwardAsync/GoBackAsync bypass the interceptor pipeline and call region history navigation directly. In practice, auth/logging/analytics logic implemented via INavigationInterceptor will run for RequestNavigateAsync but be silently skipped for back/forward actions, creating inconsistent behavior depending on how navigation is triggered.
Useful? React with 👍 / 👎.
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
1 participant
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.