-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[net9.0][Enhancement] FontImageSource defaults and style #23602
[net9.0][Enhancement] FontImageSource defaults and style #23602
Conversation
Hey there @kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
/rebase |
5809314
to
722e81b
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
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.
@kubaflo can we add a test for this? We can add a xaml unit test and maybe a a screenshot uitest test to make it looks ok at runtime.
@rmarinho Sure, I can add tests on the weekend. The reason I haven't added them is that was that I wasn't sure if you wanted this feature in MAUI :) |
722e81b
to
4ba4c7a
Compare
@rmarinho I've added a XAML and a UITest |
MergedStyle is set at VisualElement currently. it should probably be set for everything that can get a Style. This is a workaround, and if we go that route, we probably will have to apply it at multiple places |
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.
@StephaneDelcroix does this PR make sense to merge?
I can't quite extract from your last comment if you're thinking this is a good way to go.
Sounds like you're thinking we need a little bit more discussion here?
/rebase |
4ba4c7a
to
a7f3462
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
a7f3462
to
9a877cd
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
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.
Chatted with @StephaneDelcroix and he gave this one the thumbs up
Description of Change
Issues Fixed
Fixes #13474