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

BladeItem Brushes somehow causes app crash in secondary view #1218

Closed
JustinXinLiu opened this issue Jun 14, 2017 · 12 comments
Closed

BladeItem Brushes somehow causes app crash in secondary view #1218

JustinXinLiu opened this issue Jun 14, 2017 · 12 comments

Comments

@JustinXinLiu
Copy link
Contributor

I came across this StackOverflow question and was able to find a workaround for it - turned out the issue is related to two brush properties defined in BladeItem.

My answer works but I feel like this could be a bug in the framework as I can't see why they would cause this kind of hard crash.

@IbraheemOsama
Copy link
Member

Ping @skendrot

@skendrot
Copy link
Contributor

For version 2.0 I would like to see these properties removed in favor of Title, TitleTemplate, and CloseButtonContent, CloseButtonContentTemplate. How's that sound @nmetulev?

Not sure why setting the properties would cause whatever issue this is to go away. They both default to Black

public static readonly DependencyProperty TitleBarForegroundProperty = DependencyProperty.Register(
       nameof(TitleBarForeground), 
       typeof(Brush), 
       typeof(BladeItem), 
       new PropertyMetadata(new SolidColorBrush(Colors.Black)));

@JustinXinLiu
Copy link
Contributor Author

Yeah that's what I thought. The code looks fine to me. I will raise it with the UI team.

@nmetulev
Copy link
Contributor

@skendrot, Agree, I'll add this to 2.0 milestone. Is there a quick fix we can add for 1.5?

@skendrot
Copy link
Contributor

Note: I haven't tested the issue
It's possible setting the foregrounds in the default style would fix this.

But, it's a very small corner case and it would be fixed in the 2.0 version by removing them in favor of Templates

@nmetulev
Copy link
Contributor

OK, let's keep it for 2.0 then

@nmetulev
Copy link
Contributor

ping

@nmetulev
Copy link
Contributor

Moving this to 2.1. Should we simply add the default brushes?

@nmetulev nmetulev modified the milestones: v2.1, v2.0 Aug 30, 2017
@windowstoolkitbot
Copy link

This issue seems inactive. It will automatically be closed in 14 days if there is no activity.

@nmetulev
Copy link
Contributor

ping @JustinXinLiu, @skendrot, should we just add the setters in default styles for this one? Too late to do any breaking changes

@JustinXinLiu
Copy link
Contributor Author

Yeah that'd be the easiest way to fix it for now, unless @skendrot has already started the changes he proposed earlier?

@skendrot skendrot self-assigned this Oct 26, 2017
@skendrot
Copy link
Contributor

I'll work on getting the above properties added without making breaking changes

@skendrot skendrot mentioned this issue Nov 10, 2017
4 tasks
nmetulev added a commit that referenced this issue Nov 11, 2017
@ghost ghost locked as resolved and limited conversation to collaborators Nov 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants