Replies: 5 comments
-
It seems more complicated and potentially error-prone in code to manage two different states for the teaching tip: one for when the control is closed and another when the tip is open. That two state logic is usually only needed for input controls. I would expect a control like this to always update whenever a property is changed -- regardless of if the tip is opened or closed. In that context I think this is behaving correctly? Edit: It's also bad practice to be changing a property like this while the tip is shown. However, it should be elegantly handled in WinUI regardless. |
Beta Was this translation helpful? Give feedback.
-
@robloo The point here is that it is not changing the tip to a light-dismissable tip, resulting in a tip which cannot be closed by the user any longer (no close button, Esc key and clicking elsewhere on the app UI surface (aka light-dimiss) doesn't work). So if "live-update" is desired for this property, then we have to make sure the tip is hosted in a proper light-dismissable popup. If changing the popup from a non-light-dismissable popup to a light-dismissable popup requires the popup to be closed and re-opened, then we can either pospone the IsLightDismissEnabled required changes until the user opens the tip the next time, or automatically close and re-open the tip. That looks potentially problematic though due to app logic which might run when a tip is closed or other issues such as lost focus (if focus was inside the tip at the time it was closed). And indeed, creating a quick test code, changes to the Popup.IsLightDismissEnabled property while the popup is open are not processed and require the popup to be closed and re-opened again (*). So with the considerations above, I think postponing the updates to the tip after IsLightDismissEnabled was changed until the tip is opened again does sound like a reasonable option here (and with an appropiate note in the documentation). (*) If this is the intended behavior, we should consider calling this out in the documentation. |
Beta Was this translation helpful? Give feedback.
-
I tend to agree, right now it seems like we are in the worst of both worlds. I think we did this because the light dismiss property drives a vistual state which hides the close button but it is not possible to change a popup's light dismiss behavior while it is open. So this was just kind of the easy thing to do. The best solution would be to make the popup light dismissible immediately but that would require winui3 to implement, the next best solution would be to not remove the close button until the next time the tip is opened. |
Beta Was this translation helpful? Give feedback.
-
@StephenLPeters @robloo Seeing as this is not likely to be a critical issue (i.e. this is, to my recollection, the first time the current behavior was mentioned to be an issue) I'd be fine if we broaden the scope here and create a tracking issue for the We can always re-evaluate this decision if the need arises but this looks to be a fix which would be "nice to have" for now and not something critical. Would you guys be fine with that? |
Beta Was this translation helpful? Give feedback.
-
@Felix-Dev This definitely seems to be an issue with the Popup that needs to be better handled in the future. The TeachingTip would then just pass along the value and not worry about different states. If it's too complicated to fix it's also something I would be fine documenting with a warning not to change it while the pop-up is open (although it still needs to avoid getting stuck open). |
Beta Was this translation helpful? Give feedback.
-
This question is about whether we should change the current behavior of changing the
IsLightDismissEnabled
property while the teaching tip is open. Note the documentation which describes the current behavior:Visualized, this behavior looks like this (changing IsLightDismissEnabled from
false
totrue
):As the documentation describes correctly, at that point, the tip can no longer be closed by direct user interactions (Esc key doesn't close tip and clicking elsewhere on the app UI surface (light-dismiss) also doesn't close the tip).
As such, I would suggest we change the teaching tip implementation so that changes to the
IsLightDismissEnabled
setting will only take effect when the tip is re-opened. In other words, in the example above, enablingIsLightDismissEnabled
will not change the opened tip's UI at all (the close button will remain visible and the tip's background stays the same) and only the the re-opened tip will reflect these changes.Reading through the documentation quoted above "[...]reconfiguring between light-dismiss and explicit-dismiss will [...] require the teaching tip to be closed and reopened for changes to [this property] to take affect", it seems to me that the suggested behavior is the intended behavior and the current behavior we are seeing sounds more like a bug which should be fixed.
@StephenLPeters @SavoySchuler FYI (as the team members who worked on the TeachingTip).
Beta Was this translation helpful? Give feedback.
All reactions