-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ReactiveUI.Avalonia migration #3936
base: main
Are you sure you want to change the base?
Conversation
/// </summary> | ||
/// <param name="builder">This builder.</param> | ||
/// <returns>The builder.</returns> | ||
public static AppBuilder UseReactiveUI(this AppBuilder builder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extension method can be replaced with IWantsToRegisterStuff
. Is this a recommended approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
considering one of the things on the wishlist for reactiveui is to improve the start up and trimming etc, stick with this?
/// <param name="lifetime">Pass in the Application.ApplicationLifetime property.</param> | ||
public AutoSuspendHelper(IApplicationLifetime lifetime) | ||
{ | ||
RxApp.SuspensionHost.IsResuming = Observable.Never<Unit>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With Avalonia 11.1 we can have this observable properly implemented. But likely should be done in dedicated PR, as well as it raises minimal Avalonia version (right now this PR targets 11.0).
This is used in RoutedViewHost on mobile platforms - on Desktop it should follow what other desktop platforms do (probably return a constant); in general, don't worry about this too much
We probably don't need it, but it would be nice to get In general, the idea behind
"Avalonia.ReactiveUI is deprecated, please move to ReactiveUI.Avalonia" is a phrase no user of either of our platforms should ever have to think about because it is pants-on-head silly. Can we make ".UseReactiveUI" in Avalonia.ReactiveUI and make it depend on ReactiveUI.Avalonia? |
More of a nuget thing. Both reactiveui and reactiveui have reserved namespaces so neither of us can push to each other packages. |
Also nuget has an automatic redirect functionality to at least point the user. |
What kind of change does this PR introduce?
Pushes
ReactiveUI.Avalonia
backend to this repository and updates it to match other backends more.Additionally,
IntegrationTests.Avalonia
was added.What is the current behavior?
Avalonia integration is owned by Avalonia.ReactiveUI package, maintained by Avalonia core team.
Functionally, that package is not up to date with new ReactiveUI features, as well as some existing features implemented inconsistently compared to other backends.
What is the new behavior?
New ReactiveUI.Avalonia package is going to be created, with code in this repository.
This PR also already adds some features, that were missing previously:
ICreatesObservableForProperty
is a low hanging fruitAnd some are pending:
ICreatesCommandBinding
- does it make sense to have Avalonia specific implementation? From WinForms implementation, I don't see a benefit.IPlatformOperations
- how this one is used? Avalonia11.2
got Screen.Orientation API, which we can use, but it requires a window/toplevel to get screen from, as there can be multiple. Or we can assume main window from global statics.What might this PR break?
Users who currently use Avalonia.ReactiveUI. NuGet package redirects should be created, and users should be notified.
Open questions
ReactiveUI.Tests
project, but these only run for windows frameworks? How would I add Avalonia implementation there?.UseReactiveUI()
app builder method, it's also used in our ReactiveUI templates. It is different from other backends, which provideIWantsToRegisterStuff
implementation instead. My question is, would it be preferable to do the same in Avalonia? My only concern is trimmability of library, how is it handled in other backends?Please check if the PR fulfills these requirements
cc @glennawatson @ChrisPulman