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

Fixed missing source file assets on some samples #134

Closed
wants to merge 1 commit into from

Conversation

Arlodotexe
Copy link
Member

This PR fixes an issue where the .xaml and .cs source files were failing to load in-app for some samples.

Related #18

@Arlodotexe Arlodotexe added bug 🐛 Something isn't working sample app 🖼 labels Sep 1, 2023
@Arlodotexe Arlodotexe self-assigned this Sep 1, 2023
@Arlodotexe
Copy link
Member Author

This has been tested on UWP and Wasm. Wasdk still needs testing, I ran into issues getting it to launch on my machine.

@michael-hawker
Copy link
Member

That fixed the Headered control ones, but the behaviors are still broken:

image

Probably because they're in a sub-directory for organization:

image

(Same with the ConstrainedBox samples.)

Suppose we should just flatten them for now? 😟 Would be nice to support folders for samples as Behaviors, Primitives, the Extensions are all larger projects.

Is there a better way we can think to test and validate this setup moving forward too?

@Arlodotexe
Copy link
Member Author

Arlodotexe commented Sep 1, 2023

I guess we just flatten them for now? There's not many options in terms of finding the subfolder path of a xaml or cs file.

@michael-hawker
Copy link
Member

Had a thought during lunch and came to a realization why this folderpath was there.

There's a couple of issues with different causes:

  1. The Headered control samples are using HeaderedContentControlExperiment.Samples namespace, but it should be HeaderedControlsExperiment.Samples instead as we trim the assembly name from the path here:

    var folderPath = typeNamespace.Replace(simpleAssemblyName, "").Trim('.').Replace('.', '/');

    Since they don't match, that was causing these ones not to load.

  2. Since we use the namespace as an identifier and replace . with / above...

    If the Behaviors and the ConstrainedBox samples in theirs sub-folders added the folder name to the namespace (as VS usually tries to do) instead of being in a flattened root namespace, then it should be able to properly load the samples (assuming our SG doesn't care about namespace...)

Going to test this out now by adjusting the namespaces of these three sets of samples.

michael-hawker added a commit to CommunityToolkit/Windows that referenced this pull request Sep 1, 2023
… app

We should think of making an analyzer to detect this, since we use it...
See discussion here about this issue: CommunityToolkit/Tooling-Windows-Submodule#134 (comment)
@michael-hawker
Copy link
Member

Fixed in CommunityToolkit/Windows#214 now! 🎉🎉🎉

No tooling changes required, going to close this.

michael-hawker added a commit to CommunityToolkit/Windows that referenced this pull request Sep 2, 2023
… app

We should think of making an analyzer to detect this, since we use it...
See discussion here about this issue: CommunityToolkit/Tooling-Windows-Submodule#134 (comment)
michael-hawker added a commit to CommunityToolkit/Windows that referenced this pull request Sep 2, 2023
… app

We should think of making an analyzer to detect this, since we use it...
See discussion here about this issue: CommunityToolkit/Tooling-Windows-Submodule#134 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working sample app 🖼
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants