Skip to content

Refresh how-to-open-files-using-the-openfiledialog-component #10522

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

Merged
merged 18 commits into from
Feb 28, 2019

Conversation

v-thepet
Copy link
Contributor

@v-thepet v-thepet commented Feb 16, 2019

Summary

Refreshed content and new code samples for review.

New code samples are in PR dotnet/samples#674.

Fixes #Issue_Number (if available)
See Task https://mseng.visualstudio.com/TechnicalContent/_queries/edit/1409952/?triage=true

@v-thepet
Copy link
Contributor Author

@rpetrusha - This is ready for your review, please see email. (I'll remove the code examples to the samples repo once we're sure where to put them.) Thanks.

Copy link
Contributor

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

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

This looks good, @v-thepet. I've left a number of comments and suggestions.

<PropertyGroup>
<OutputType>WinExe</OutputType>
<TargetFramework>netcoreapp3.0</TargetFramework>
<RootNamespace>folder name</RootNamespace>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I have the latest version of the 3.0 SDK (3.0.100-preview-010184), but this element isn't present.

Copy link
Contributor Author

@v-thepet v-thepet Feb 21, 2019

Choose a reason for hiding this comment

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

@rpetrusha - It looks like the .csproj file generated by Visual Studio 2019 does differ from the .csproj file generated by Visual Studio Code (although I thought I'd compared them). The VS Code-generated file has the "namespace" element and the VS file doesn't. Given the discrepancy, how about leaving out this code entirely and just say that they need to have a .NET Core Windows Forms project file.

[STAThread]
public static void Main()
{
Application.Run(new OpenFileDialogForm());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Application.Run(new OpenFileDialogForm());
Application.EnableVisualStyles();
Application.SetCompatibleTextRenderingDefault(false);
Application.Run(new OpenFileDialogForm());

The two lines should also be added to Main in the second example for both VB and C#.
Also need to add this code to the second set of files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rpetrusha - Application.SetCompatibleTextRenderingDefault(false); throws a runtime error unless it is the first thing in the file so I attempted to rearrange things.

@rpetrusha
Copy link
Contributor

@v-thepet, sample code can go in samples/snippets/winforms/open-files/example1/cs and samples/snippets/winforms/open-files/example1/vb for the first example, and samples/snippets/winforms/open-files/example2/cs and samples/snippets/winforms/open-files/example2/vb for the second example.

In terms of files to commit, there's no need for program.cs if the main source code file contains the entry point. There's also no need for the *.resx file, since the examples use no resources. There's also no need for the code-behind *.Designer.cs file if all necessary initialization is handled in the constructor.

It would be a good idea to replace the message box with a text box, and probably in the initialization code to better place the controls in the window.

@v-thepet
Copy link
Contributor Author

@rpetrusha - I think I've got all the changes, please re-review. I will put the examples up on samples where you designated, but since there is a lot new, I left them here for now for expediency. Thanks.

@rpetrusha
Copy link
Contributor

This looks good, @v-thepet. I think that the next step is to check the examples into the dotnet/samples repo, add code references to the samples repo here, and then rebuild when the samples merge.

@v-thepet
Copy link
Contributor Author

@rpetrusha - The code links are now in the article replacing the embedded code. The code PR is dotnet/samples#674. Thanks.

@rpetrusha
Copy link
Contributor

Closing and reopening to begin new build after dotnet/samples#674 was merged.

@rpetrusha rpetrusha closed this Feb 27, 2019
@rpetrusha rpetrusha reopened this Feb 27, 2019
@rpetrusha
Copy link
Contributor

This looks good, @v-thepet. Thanks for all the work you've put into improving this article. I'll merge your PR now.

@rpetrusha rpetrusha merged commit 25b6d66 into dotnet:master Feb 28, 2019
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.

2 participants