Skip to content
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

Check Box Click Event Not Fired when added in a ContentDialog in WINUI #9343

Closed
ghost opened this issue Feb 16, 2024 · 2 comments
Closed

Check Box Click Event Not Fired when added in a ContentDialog in WINUI #9343

ghost opened this issue Feb 16, 2024 · 2 comments
Labels
area-Dialogs bug Something isn't working team-Controls Issue for the Controls team

Comments

@ghost
Copy link

ghost commented Feb 16, 2024

Describe the bug

The CheckBox click event is not fired when the checkbox is added in the contentDialog. Upon analysis, it was found that the CheckBox is unloaded before updating the view. Therefore, the CheckBox click event is not fired.

Steps to reproduce the bug

1.Run the sample.
2.Add the breakpoints in the Checkbox click event, Loaded event and Unloaded event.
3.Click the "Click Here" Button.
WINUI_Sample.zip

Expected behavior

The checkbox event should be triggered when the checkbox is clicked.

Screenshots

CheckBox_Query.mp4

NuGet package version

None

Windows version

No response

Additional context

No response

@ghost ghost added the bug Something isn't working label Feb 16, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Feb 16, 2024
@codendone codendone added area-Dialogs team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Feb 22, 2024
@SreemonPremkumarMuthukrishnan

Hi team,

Is there any update on this?

@JJBrychell
Copy link

JJBrychell commented Apr 1, 2024

The issue here is that you are removing the click handler in the Unloaded event and we have confirmed that there are currently some issues with the way the Loading/Loaded/Unloaded events are raised when an item is removed and then re-added to the tree in a single frame.

Unfortunately, we have not found a way to correct this issue without potentially breaking applications that may have inadvertently relied upon this behavior (including some internal Xaml code). Because of this, we will try to target a fix for the 2.0 release, but until then you will need to work around the issue and fortunately there appears to be such a workaround. What is happening is that the Unloaded event is getting fired asynchronously when the control is removed from the tree. But then it gets re-added and, by the time the event is actually processed by the application, the element is back in tree. To get around this issue, the application can look at the IsLoaded property on the element and only remove the handler if the value is false. Since the element is, in fact, in the tree, another Unloaded event will come when it is removed a second time.

Details:

First a bit of a side as to why this checkbox is getting removed and re-added. When content dialog is shown, it ends up being added to the tree (causing anything in it to be added to the tree), but then after the smoke layer is created (which happens during template application), it gets removed and re-added to ensure the correct z-order between the dialog content and the smoke layer. So from the perspective of a control, it gets added-removed-added, causing this issue.

Now to the events. Because of the nature of these tree related events, they are raised at different times and in different manners in order to assure that things get manipulated predictably (which they are, it just seems no correctly). For example, Loading and Loaded are raised synchronously since an application might want to modify the tree in them and want to limit the amount of stale UI we render. But Unloaded, is asynchronous since is primarily just resource cleanup and there is o need to delay a render frame for it. So basically, what happens is:

  • When an element is added to the tree, is marked as needing a Loading event. But we want some "order" to how we raise these (other than the order an application might add them) so we do not raise it immediately.
  • During the tree walk for layout, we see if any element needs a Loading event and if so we synchronously raise it. This gives us a very predictable order. But, the element is not actually loaded yet since we haven't expanded any templates, applied any resources or any of the other things that occur during a layout pass. So at this point, we request that when the layout pass is complete, we raise a Loaded event.
  • At the end of the layout pass, we then iterate over all the elements needing a Loaded event and (if there are still listeners) will raise those events synchronously.
  • And finally, when the element is removed from the tree we asynchronously raise the Unloaded event.

The issue occurs if you add then remove, then re-add the element in the same tick. Here is what happens:

  1. Element is added. It is marked as needing a Loading event.
  2. Element is removed. An Unloaded event is raised asynchronously (so it doesn't execute right away).
  3. Element is re-added. It is marked as needing a Loading event.
  4. During layout, we see the element needs a Loading event and raise it synchronously (executes immediately) and indicate that we should raise a Loaded event.
  5. Layout completes and we raise the Loaded event (executes immediately) and the frame is complete.
  6. Asynchronous events are fired, including the Unloaded event raised in step 2.

So now we have a unloaded event fired while the element is in the tree, thus causing this issue.

The fix:

Although we are still discussing options, we think the fix is that we would ensure that we don't raise an Unloaded event:

  • Until we have confirmed the Loaded event has been raised (we are still discussing the Loading event).
  • And the element has not been added back into the tree.

It is the first condition that we found breaks some internal code and could potentially break some applications.

We are going to close this issue with the work around. However, there a number of other open cause by this sequence of events that may not have a workaround and we hope to fix and resolve those in 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Dialogs bug Something isn't working team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

3 participants