-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Make IsActivated property public in Window.cs #30987
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
Conversation
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.
Pull Request Overview
This PR makes the IsActivated property in the Window class publicly accessible by changing its visibility from internal to public. This allows external consumers of the Microsoft.Maui.Controls API to check whether a window is currently activated.
- Changes the visibility modifier of the
IsActivatedproperty frominternaltopublic
|
Needs to update public api files ? |
mattleibow
left a comment
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.
For public properties, this really should be a bindable readonly property
Also
Can you add somrthing to the mutli-window sample page
- a binding to a label on that.
And maybe update/duplicate the tests here:
- src/Controls/tests/Core.UnitTests/WindowsTests.cs
- src/Controls/tests/DeviceTests/Elements/Window/WindowTests.Windows.cs
This is the wondow property on VisualElement, so ignore the name:
static readonly BindablePropertyKey WindowPropertyKey = BindableProperty.CreateReadOnly(
nameof(Window), typeof(Window), typeof(VisualElement), null, propertyChanged: OnWindowChanged);
/// <summary>Bindable property for <see cref="Window"/>.</summary>
public static readonly BindableProperty WindowProperty = WindowPropertyKey.BindableProperty;
/// <summary>
/// Gets the <see cref="Window"/> that is associated with an element. This is a read-only bindable property.
/// </summary>
public Window Window => (Window)GetValue(WindowProperty);
/// <inheritdoc/>
Window IWindowController.Window
{
get => (Window)GetValue(WindowProperty);
set => SetValue(WindowPropertyKey, value);
}I would expect something like this:
static readonly BindablePropertyKey IsActivatedPropertyKey = BindableProperty.CreateReadOnly(
nameof(IsActivated), typeof(bool), typeof(Window), false);
public static readonly BindableProperty IsActivatedProperty = IsActivatedPropertyKey.BindableProperty;
public bool IsActivated
{
get => (bool)GetValue(IsActivatedProperty)
private set => SetValue(IsActivatedPropertyKey, value);
}Also, we will need to remove the _isActivated field in window.cs.
|
Hi @@pictos. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
BTW, is still missing DeviceTests, will implement it this week. |
|
This is great! Slowly bindable property-izing he world. This will be nice to do binding to IsActivated. Can you maybe also add a test for that using a binding to that property. Just to make sure that it does work. |
rmarinho
left a comment
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.
Possible to add a DeviceTest ? clean up. the comment out code.
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
The `IsActivated` property in the `Microsoft.Maui.Controls` namespace was changed from `internal` to `public`. This increases its visibility, allowing it to be accessed from outside the assembly.
Changed Window.IsActivated to use a read-only BindableProperty and made its setter private. This restricts modification of the activation state to internal code, improving encapsulation and preventing external changes.
Changed Window.IsActivated setter to use IsActivatedPropertyKey for proper encapsulation. Added unit tests to verify window activation and deactivation behavior, including Appearing event handling. Included new using directives for async test support.
e8c888d to
dd37806
Compare
mattleibow
left a comment
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.
Matthew is super happy!
|
Bumped you to p/0, so you should get in the door for RC1. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
This is a super useful change. Kudos @pictos. |
Making this public will allow devs to filter and find the
Windowthat has focus, it can make easier to show popups and perform some actions.Related to #30762
Make IsActivated property public in Window.cs
The
IsActivatedproperty in theMicrosoft.Maui.Controlsnamespacewas changed from
internaltopublic. This increases its visibility,allowing it to be accessed from outside the assembly.