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.

Sign up for GitHub

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

Jump to bottom

feat: added the resource style extension #875

Merged
merged 5 commits into from
Oct 25, 2023
Merged

feat: added the resource style extension #875

merged 5 commits into from
Oct 25, 2023

Conversation

Marc-Antoine-Soucy
Copy link
Contributor

@Marc-Antoine-Soucy Marc-Antoine-Soucy commented Oct 13, 2023 •
edited by kazo0
Loading

closes #870

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

What is the new behavior?

Added an extension that allows you to add a resource dictionnary to a style.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@Marc-Antoine-Soucy
Copy link
Contributor Author

Marc-Antoine-Soucy commented Oct 13, 2023 •
edited
Loading

@kazo0 I only tried this on WinUI, but I will try the others monday morning. (it should work on all platforms since it worked on all platform with material)

The only thing I'm really not sure about is the type of sample page this should be? At the beginning, I put material, but it forces you to give a material and a material3 option. I put it as design agnostic, since the fact it works with "material" is not really the important thing, but yeah.

Should I modify the sample pages so that I can make a material sample page containing a single page? Or should that already be possible and I simply don't know how to do it?

Also, you should review the doc. I'm not sure what are the exact expectations for the docs. I try following the general patterns I found in other files.

kazo0
kazo0 reviewed Oct 14, 2023
View reviewed changes
doc/helpers/style-extensions.md Outdated Show resolved Hide resolved
doc/lightweight-styling.md Outdated Show resolved Hide resolved
doc/helpers/style-extensions.md Outdated Show resolved Hide resolved
doc/helpers/style-extensions.md Outdated Show resolved Hide resolved
...o.Toolkit.Samples/Uno.Toolkit.Samples.Shared/Content/Controls/StyleExtensionsSamplePage.xaml Outdated Show resolved Hide resolved
src/Uno.Toolkit.UI/Behaviors/StyleExtensions.cs Outdated Show resolved Hide resolved
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

1 similar comment
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@Marc-Antoine-Soucy Marc-Antoine-Soucy marked this pull request as ready for review October 16, 2023 17:48
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@Marc-Antoine-Soucy Marc-Antoine-Soucy force-pushed the dev/maso/styleresourceextensions branch 2 times, most recently from 5b65b8c to 7e08c66 Compare October 18, 2023 14:58
@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@kazo0 kazo0 enabled auto-merge (squash) October 24, 2023 15:46
Xiaoy312
Xiaoy312 approved these changes Oct 24, 2023
View reviewed changes
Copy link
Contributor

@Xiaoy312 Xiaoy312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also tested the case with <ResourceDictionary Source="my-special-palette.xaml" /> as value?
add it to the docs? (we can omit that from the sample)

src/Uno.Toolkit.UI/Behaviors/ResourceExtensions.cs Outdated Show resolved Hide resolved
src/Uno.Toolkit.RuntimeTests/Tests/ResourceExtensionsTest.cs Outdated Show resolved Hide resolved
doc/lightweight-styling.md Outdated Show resolved Hide resolved
@Marc-Antoine-Soucy
Copy link
Contributor Author

can you also tested the case with <ResourceDictionary Source="my-special-palette.xaml" /> as value? add it to the docs? (we can omit that from the sample)

This didnt work because of an error we will probably need to create an issue about:
5>C:\p\uno.toolkit.ui\samples\Uno.Toolkit.Samples\Uno.Toolkit.Samples.Skia.Gtk\Uno.UI.SourceGenerators\Uno.UI.SourceGenerators.XamlGenerator.XamlCodeGenerator\ResourceExtensionsSamplePage_6370eb0d10be25f855a4db5af287289b.cs(540,36,552,10): error CS1503: Argument 1: cannot convert from 'Windows.UI.Xaml.ResourceDictionary' to 'System.Collections.Generic.KeyValuePair<object, object>'

@github-actions
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-875.eastus2.azurestaticapps.net

@kazo0 kazo0 merged commit 8a7bbd0 into main Oct 25, 2023
24 checks passed
@kazo0 kazo0 deleted the dev/maso/styleresourceextensions branch October 25, 2023 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xiaoy312 Xiaoy312 Xiaoy312 approved these changes

@kazo0 kazo0 kazo0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[StyleExtensions] Resources Attached Property
3 participants
@Marc-Antoine-Soucy @Xiaoy312 @kazo0