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

Bug: ContentDialog is changing the application-scope default button style (setting BorderBrush) #1764

Closed
robloo opened this issue Dec 15, 2019 · 16 comments
Labels
area-Dialogs needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team

Comments

@robloo
Copy link
Contributor

robloo commented Dec 15, 2019

Describe the bug

Opening a ContentDialog will change the application-scope default button style and add a border brush that wasn't previously there. The application will now be using the modified button style until it is restarted.

This initially appeared related to #1405 which had content dialog styling fixes for Bug: CornerRadius Inconsistency/Missing in ContentDialog/MessageDialog #1345. However, that no longer appears to be true.

Steps to reproduce the bug

  1. Startup an application normally, all button styles are correct
  2. Show a ContentDialog from the CloseRequested application event, then close the dialog (cancel)
  3. Make sure to open no other ContentDialogs before step 2
  4. After a visual tree update (navigation to/from a page) the application buttons have a different/incorrect style with a new border brush

Edit: This is more complex than it initially appeared. Even when reverting from WinUI 2.3 back to 2.2 it still was present. Then it became apparent:

  • This may only be an issue when opening a ContentDialog from the CloseRequested application event which is a restricted API.
  • This is only an issue if the ContentDialog opened by the CloseRequested event is the FIRST content dialog shown. If another ContentDialog is opened first (during normal application life-cycle), the button styles will not be changed.

Expected behavior

The ContentDialog should not modify the application-scoped default button style in any way.

Screenshots

Normal Button Style:

pic2

Button Style after ContentDialog is opened then closed:

pic3

Version Info

Microsoft.UI.Xaml 2.3.191211002
Desktop Windows 10 Home 1903

@robloo
Copy link
Contributor Author

robloo commented Dec 15, 2019

@yaichenbaum I believe you mentioned in the past you use the CloseRequested event to open a content dialog prompting the user what to do with any unsaved changes. If you have a chance, and you think it's worth your time, could you see if this appears in your app?

@yaira2
Copy link
Contributor

yaira2 commented Dec 15, 2019

@robloo I am using custom buttons in the dialog, so I don't see this behavior.

@robloo
Copy link
Contributor Author

robloo commented Dec 16, 2019

@yaichenbaum, Ok, thanks a lot for taking a look.

@YuliKl
Copy link

YuliKl commented Dec 16, 2019

@robloo thank you for reporting this bug. Can you provide a repro app to help us investigate?

@YuliKl YuliKl added needs-author-feedback Asked author to supply more information. team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Dec 16, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 16, 2019

@YuliKl Thanks for your response. It's likely I'm missing something in the steps to reproduce but I don't have a lot of time to spend on this at the moment. So I'll see if I can provide more details but can't guarantee a repro app at the moment.

@msft-github-bot msft-github-bot added needs-triage Issue needs to be triaged by the area owners and removed needs-author-feedback Asked author to supply more information. labels Dec 16, 2019
@StephenLPeters StephenLPeters added needs-author-feedback Asked author to supply more information. and removed needs-triage Issue needs to be triaged by the area owners labels Dec 17, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 17, 2019

@YuliKl After going through the styles I found two lines that were not deleted when migrating to WinUI 2.3 (see Themes\Styles.xaml in the sample app source code at the bottom). These two lines changed the default border brush for the button (I was customizing most controls before WinUI 2.3 because 2px borders were undesirable, it's great borders are now improved by default!).

So that means most of, if not all of, this was a bug on my part. The question that is keeping me from closing this right now is why don't my border customizations always show? Why does it take the ContentDialog to make them appear? Something was either changed with styling precedence I don't understand or there is possibly still a bug some place. (At this point I'm thinking it's my mistake again).

Screen Shot

Issue

Source Code

App4.zip

@msft-github-bot msft-github-bot added needs-triage Issue needs to be triaged by the area owners and removed needs-author-feedback Asked author to supply more information. labels Dec 17, 2019
@YuliKl
Copy link

YuliKl commented Dec 18, 2019

@robloo glad to hear that you were able to solve most of the problem.

The question that is keeping me from closing this right now is why don't my border customizations always show? Why does it take the ContentDialog to make them appear?

@MikeHillberg any ideas about these questions?

@StephenLPeters
Copy link
Contributor

@robloo could you provide the app you used to make this gif? It would help a lot for our debugging purposes.

@StephenLPeters StephenLPeters added needs-author-feedback Asked author to supply more information. and removed needs-triage Issue needs to be triaged by the area owners labels Dec 19, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 19, 2019

Hi @StephenLPeters, thanks for taking a look at this. The source code is in the zip file at the bottom of my last post -- sorry it's a bit hard to see.

@msft-github-bot msft-github-bot added needs-triage Issue needs to be triaged by the area owners and removed needs-author-feedback Asked author to supply more information. labels Dec 19, 2019
@ranjeshj ranjeshj added area-Dialogs and removed needs-triage Issue needs to be triaged by the area owners question labels Dec 19, 2019
@ranjeshj ranjeshj removed their assignment Dec 19, 2019
@StephenLPeters
Copy link
Contributor

Hi @StephenLPeters, thanks for taking a look at this. The source code is in the zip file at the bottom of my last post -- sorry it's a bit hard to see.

Oh awesome! I totally missed that, thank you!

@kaiguo
Copy link
Contributor

kaiguo commented Dec 27, 2019

This should be a bug on application side according to #1345 (comment), closing.

@kaiguo kaiguo closed this as completed Dec 27, 2019
@robloo
Copy link
Contributor Author

robloo commented Dec 28, 2019

@kaiguo This is NOT the same as #1345. Per my comments above, yes, I'm changing the style of the button. However, why doesn't the custom border always appear on the button? Why does it only show up after the ContentDialog is opened? There may be a bug still in the framework.

Please re-open this or provide an explanation for:

The question that is keeping me from closing this right now is why don't my border customization always show? Why does it take the ContentDialog to make them appear?

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Dec 28, 2019
@ranjeshj ranjeshj reopened this Jan 2, 2020
@ranjeshj ranjeshj added needs-assignee-attention Assignee needs to follow-up or investigate this issue and removed needs-triage Issue needs to be triaged by the area owners labels Jan 2, 2020
@marcelwgn
Copy link
Collaborator

@StephenLPeters Seems like a framework issue, so it probably needs WinUI 3.

@StephenLPeters StephenLPeters removed the needs-assignee-attention Assignee needs to follow-up or investigate this issue label Sep 1, 2020
@StephenLPeters
Copy link
Contributor

I'm not sure where the issue is, it could be a framework issue, but it could also be a weird by design quirk with how the app is importing the winui 2 package.. I think this could be looked into more before we push it to winui 3, however I don't think we have the resources.

@robloo
Copy link
Contributor Author

robloo commented Sep 1, 2020

I certainly agree there is no need to look at this before WinUI 3.0. Let's wait until WinUI 3.0 is solid and then I can test again to see if it has been fixed. I expect it is a strange case that might just disappear by then.

@StephenLPeters StephenLPeters added the needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) label Sep 1, 2020
@robloo
Copy link
Contributor Author

robloo commented Jul 19, 2023

I'm closing this because I don't care to check if it still exists in WinUI3. It wasn't a critical issue (although indicated some fundamental flaws in styling) and someone else can bring it up in the future if it still exists.

@robloo robloo closed this as completed Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Dialogs needs-winui-3 Indicates that feature can only be done in WinUI 3.0 or beyond. (needs winui 3) team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

8 participants