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

CommunityToolkit/Windows#445 #1

Merged

Conversation

Lightczx
Copy link

@Lightczx Lightczx commented Sep 30, 2024

@@ -30,4 +30,7 @@

<!-- Sets this up as a toolkit component's source project -->
<Import Project="$(ToolingDirectory)\ToolkitComponent.SourceProject.props" />
<ItemGroup>
<None Remove="NativeMethods.json" />
Copy link

Choose a reason for hiding this comment

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

This is redundant

@@ -0,0 +1,3 @@
{
"allowMarshaling": false
Copy link

Choose a reason for hiding this comment

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

Add scheme and preserve sig.

Suggested change
"allowMarshaling": false
"$schema": "https://aka.ms/CsWin32.schema.json",
"allowMarshaling": false,
"comInterop": {
"preserveSigMethods": [
"*"
]
}

Copy link
Author

Choose a reason for hiding this comment

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

We only use

RegisterClassEx
DefWindowProc
CreateWindowEx
WM_SETTINGCHANGE

Not a single COM interface gets imported.
So I don't think comInterop.preserveSigMethods is needed.

Copy link

Choose a reason for hiding this comment

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

Contributors may use com interfaces for other things. There's no reason to keep it out. Also add the schema.

Copy link
Author

Choose a reason for hiding this comment

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

@0x5bfa Done

Copy link

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍
EOF change in components/Helpers/src/MultiTarget.props may have to be reverted.

@Lightczx
Copy link
Author

@HO-COOH I'm gonna leave this to you.
busy for my own project.

@HO-COOH HO-COOH merged commit 70c489e into HO-COOH:feat/winui3-themelistener Sep 30, 2024
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.

3 participants