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

Merge resources/part1 #27

Merged
merged 10 commits into from
Dec 1, 2023
Merged

Merge resources/part1 #27

merged 10 commits into from
Dec 1, 2023

Conversation

ghost1372
Copy link
Contributor

@ghost1372 ghost1372 commented Nov 30, 2023

Fix #22
This PR only covers the iNKORE.UI.WPF.Modern and iNKORE.UI.WPF.Modern.Controls will be done in next pr.

i used a tool called XAMLTools.MSBuild which, Combines multiple XAML files to one large file.
This is useful when you want to provide one Generic.xaml instead of multiple small XAML files.
Using one large XAML file not only makes it easier to consume, but can also drastically improving loading performance.

Things that have been done in this PR:

All MergedDictionary and Converters removed from xaml files

All Converters moved to a new File Called Converters.xaml

ControlsResources Removed and some of contents moved to ThemeResources.xaml file

Generic.xaml file removed and some of contents moved to a new file called CustomControlStuff.xaml

All custom controls are automatically merged into Generic.xaml file.

All styles are automatically merged into a new file named Theme.xaml

XamlControlsResources.cs file updated to Theme.xaml file instead of ControlsResources.xaml file.

@NotYoojun
Copy link
Member

NotYoojun commented Nov 30, 2023

Wait a minute, did you just put all the control styles in ONE XAML FILE?? I don't think this will be good for the work in future nor the following maintenance.

@ghost1372
Copy link
Contributor Author

ghost1372 commented Nov 30, 2023

Wait a minute, did you just put all the control styles in ONE XAML FILE?? I don't think this will be good for the work in future nor the following maintenance.

You have 2 files ControlsResources.xaml and Generic.xaml

You are currently putting all the files in one file!
All styles inside ControlsResources.xaml and all custom controls inside generic.xaml
But the important difference is that, You do this by nesting dictionaries (A dictionary inside a dictionary)...
But the changes I made, It puts all the styles in the theme.xaml file directly (i just changed ControlsResources to theme) and here i did not use nested dictionaries. i did the same for the custom control and all custom control things goes into generic.xaml file directly.
You have no problems with development and maintenance.
Do whatever you like, create as many files as you like (Also, you no longer need to reference styles, resources and controls in dictionaries), Write your codes, after saving Ctrl + S or Building your project, All dictionaries are automatically placed in the specified files.

if you look at csproj file you can see that we have a code:

<XAMLCombineItems Include="Controls\**\*.xaml">
		<TargetFile>Themes\Generic.xaml</TargetFile>
		<Visible>False</Visible>
	  </XAMLCombineItems>

This code specifies that the xaml files hosted in controls folder should be merged into a single file called generic.xaml
and if you look at generic.xaml file you can see this output:

<!--
    This code was generated by a tool.
    Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
-->
<!--
Source files:
Controls\AcrylicPanel.xaml
Controls\AnimatedVisualSource.xaml
Controls\Frame.xaml
Controls\Page.xaml
Controls\Primitives\TitleBarButton.xaml
Controls\Primitives\TitleBarControl.xaml
Themes\CustomControlStuff.xaml
Themes\FontIconFallback.xaml
Themes\ListViewHeaderItem.xaml
Themes\TextContextMenu.xaml
-->

so instead of this:

 <ResourceDictionary.MergedDictionaries>
     <ResourceDictionary Source="/iNKORE.UI.WPF.Modern;component/Controls/AcrylicPanel.xaml" />
     <ResourceDictionary Source="/iNKORE.UI.WPF.Modern;component/Controls/AnimatedVisualSource.xaml" />
..........
 </ResourceDictionary.MergedDictionaries>

we have actual codes...

@NotYoojun
Copy link
Member

NotYoojun commented Dec 1, 2023

Hi! I got the following exception when I try to run the sample app:
image

By the way can you change the name Themes/Theme.xaml to Themes/CombinedResources.xaml? I don't think the name Theme.xaml have a clear meaning.
Also, you may have to apply #28 to this pull.

@ghost1372
Copy link
Contributor Author

Hi! I got the following exception when I try to run the sample app:
image

By the way can you change the name Themes/Theme.xaml to Themes/CombinedResources.xaml? I don't think the name Theme.xaml have a clear meaning.
Also, you may have to apply #28 to this pull.

Sure, wait for it

@ghost1372
Copy link
Contributor Author

ghost1372 commented Dec 1, 2023

@NotYoojun i applied #28 and i renamed Theme.xaml to CombinedResources.xaml, also i fixed exception and now Demo apps can be run without any issues.
image

@NotYoojun
Copy link
Member

image There's another issue. Go to the Text -> TextBox on the sample app, then right click any textbox, the exception above will throw. Do you have a minute to solve that?

@ghost1372
Copy link
Contributor Author

image There's another issue. Go to the Text -> TextBox on the sample app, then right click any textbox, the exception above will throw. Do you have a minute to solve that?

i am working on it

@ghost1372
Copy link
Contributor Author

ghost1372 commented Dec 1, 2023

@NotYoojun go to Main branch and test it again, this bug still exist and does not related to this pr.
relative panel also has a bug and crash
System.InvalidOperationException: 'UIElement.Measure(availableSize) cannot be called with NaN size.'

@NotYoojun
Copy link
Member

OK, i fixed the TextBox issue and made a commit.

@NotYoojun NotYoojun merged commit 18e608e into iNKORE-NET:main Dec 1, 2023
@ghost1372
Copy link
Contributor Author

OK, i fixed the TextBox issue and made a commit.

excelent, thank you.
when you re-organize second library, i will be ready for second part.
also what do you think, we remove CombinedResources and we just use Generic.xaml?

@NotYoojun
Copy link
Member

Good idea, maybe you can have a try!

@NotYoojun
Copy link
Member

Wait a minute, I don't think this is a good action. You see, in XamlControlResources.cs there are some lines like this:
Generic.xaml will be loaded automatically by the WPF mechanics, while the CombinedResources.xaml will be loaded manually when user used XamlControlResources.

If you put them together, All the unnecessary stuff in Generic.xaml will be loaded twice. So I think it's better to keep the current structure. What do you think?

        internal static ResourceDictionary ControlsResources
        {
            get
            {
                if (_controlsResources == null)
                {
                    _controlsResources = new ResourceDictionary { Source = PackUriHelper.GetAbsoluteUri("Themes/CombinedResources.xaml") };
                }
                return _controlsResources;
            }
        }

OK, i fixed the TextBox issue and made a commit.

excelent, thank you. when you re-organize second library, i will be ready for second part. also what do you think, we remove CombinedResources and we just use Generic.xaml?

@ghost1372
Copy link
Contributor Author

Wait a minute, I don't think this is a good action. You see, in XamlControlResources.cs there are some lines like this: Generic.xaml will be loaded automatically by the WPF mechanics, while the CombinedResources.xaml will be loaded manually when user used XamlControlResources.

If you put them together, All the unnecessary stuff in Generic.xaml will be loaded twice. So I think it's better to keep the current structure. What do you think?

        internal static ResourceDictionary ControlsResources
        {
            get
            {
                if (_controlsResources == null)
                {
                    _controlsResources = new ResourceDictionary { Source = PackUriHelper.GetAbsoluteUri("Themes/CombinedResources.xaml") };
                }
                return _controlsResources;
            }
        }

OK, i fixed the TextBox issue and made a commit.

excelent, thank you. when you re-organize second library, i will be ready for second part. also what do you think, we remove CombinedResources and we just use Generic.xaml?

let me test and see what is happening

NotYoojun added a commit that referenced this pull request Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Performance and Simplify Xaml Resources
2 participants