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

Modified docs for dialogs in PresentationFramework #9239

Merged
merged 9 commits into from
Sep 7, 2023

Conversation

dipeshmsft
Copy link
Member

@dipeshmsft dipeshmsft commented Aug 31, 2023

Summary

In .NET 8 Preview 7, we made changes to the common dialog API in WPF. We introduced OpenFolderDialog and CommonIItemDialog API and made few changes to (Open\Save)FileDialog, FileDialog and CommonDialog APIs

Here is the API proposal regarding the changes in the API : dotnet/wpf#7689 (comment)

Describing the changes -

  • FileDialog : OnFileOK -> OnItemOK, Options removed. HookProc removed.
  • OpenFileDialog : Removed override for CheckPermissionsToShowDialog.
  • Other APIs were moved from FileDialog to CommonItemDialog
  • New properties are added in CommonItemDialog, OpenFileDialog, SaveFileDialog

@learn-build-service-prod
Copy link

Learn Build status updates of commit 046fcec:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Win32/CommonItemDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialogCustomPlace.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFolderDialog.xml ✅Succeeded View
xml/Microsoft.Win32/SaveFileDialog.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

Left some suggestions for you to consider

xml/Microsoft.Win32/SaveFileDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
@learn-build-service-prod

This comment was marked as outdated.

@learn-build-service-prod

This comment was marked as outdated.

@dipeshmsft
Copy link
Member Author

@dotnet/docs, I have made the necessary changes to the PR. There is a dependency on these documents for a blog that we will publish by the end of the week. Could you review this PR before that?

Also, once the PR is merged how much time does it take to reflect on Microsoft Docs ?

@dipeshmsft
Copy link
Member Author

We removed properties HookProc and Options from FileDialog. However, the AssemblyInfo contains 8.0.0.0 in the AssemblyVersion tag. That needs to be removed as well, right?

xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/CommonItemDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/OpenFolderDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/SaveFileDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/SaveFileDialog.xml Outdated Show resolved Hide resolved
xml/Microsoft.Win32/SaveFileDialog.xml Outdated Show resolved Hide resolved
@gewarren
Copy link
Contributor

gewarren commented Sep 5, 2023

Also, once the PR is merged how much time does it take to reflect on Microsoft Docs ?

Publishing happens every weeknight around 10 pm PST.

@gewarren
Copy link
Contributor

gewarren commented Sep 5, 2023

We removed properties HookProc and Options from FileDialog. However, the AssemblyInfo contains 8.0.0.0 in the AssemblyVersion tag. That needs to be removed as well, right?

I guess you are talking about this.
It should be removed automatically if the API was removed. Did you remove the API after the reference assembly was generated? If so, it will be picked up in the RC 1 update.

@dipeshmsft
Copy link
Member Author

It should be removed automatically if the API was removed. Did you remove the API after the reference assembly was generated? If so, it will be picked up in the RC 1 update.

This API was removed in .NET 8 Preview 7. I generated the docs after these changes were merged.

@gewarren
Copy link
Contributor

gewarren commented Sep 5, 2023

This API was removed in .NET 8 Preview 7. I generated the docs after these changes were merged.

Did you remove it from the reference assembly as well?

@dipeshmsft
Copy link
Member Author

dipeshmsft commented Sep 5, 2023

This API was removed in .NET 8 Preview 7. I generated the docs after these changes were merged.

Did you remove it from the reference assembly as well?

In reference assembly file, HookProc still exists in CommonDialog, however HookProc and Options are removed from FileDialog. https://github.com/dotnet/wpf/blob/7a6188f601e6464a98e11608a9c1948bc3307fff/src/Microsoft.DotNet.Wpf/src/PresentationFramework/ref/PresentationFramework.cs#L3-L50

@learn-build-service-prod
Copy link

Learn Build status updates of commit 90e39e8:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Win32/CommonItemDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialogCustomPlace.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFolderDialog.xml ✅Succeeded View
xml/Microsoft.Win32/SaveFileDialog.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@gewarren
Copy link
Contributor

gewarren commented Sep 5, 2023

In reference assembly file, HookProc still exists in CommonDialog, however HookProc and Options are removed from FileDialog. https://github.com/dotnet/wpf/blob/7a6188f601e6464a98e11608a9c1948bc3307fff/src/Microsoft.DotNet.Wpf/src/PresentationFramework/ref/PresentationFramework.cs#L3-L50

Okay, I see that these two APIs were removed in the docs update for Preview 7, and they don't show as applicable to the Windows Desktop 8 moniker

image

It is weird that the assembly version 8.0.0.0 is still listed under these APIs though (Options and HookProc). @huangmin-ms Is this a bug?

@huangmin-ms
Copy link
Contributor

huangmin-ms commented Sep 6, 2023

In reference assembly file, HookProc still exists in CommonDialog, however HookProc and Options are removed from FileDialog. https://github.com/dotnet/wpf/blob/7a6188f601e6464a98e11608a9c1948bc3307fff/src/Microsoft.DotNet.Wpf/src/PresentationFramework/ref/PresentationFramework.cs#L3-L50

Okay, I see that these two APIs were removed in the docs update for Preview 7, and they don't show as applicable to the Windows Desktop 8 moniker

image It is weird that the assembly version 8.0.0.0 is still listed under these APIs though ([Options](https://github.com/dotnet/dotnet-api-docs/blob/90e39e8d88613aeb71d3af4cdabdfa405b1f03ee/xml/Microsoft.Win32/FileDialog.xml#L745) and [HookProc](https://github.com/dotnet/dotnet-api-docs/blob/90e39e8d88613aeb71d3af4cdabdfa405b1f03ee/xml/Microsoft.Win32/FileDialog.xml#L602)). @huangmin-ms Is this a bug?

Yes, it is. Can you create a bug for us? And this bug may not have a higher priority since there is no impact for production.

@dipeshmsft
Copy link
Member Author

dipeshmsft commented Sep 6, 2023

Yes, it is. Can you create a bug for us? And this bug may not have a higher priority since there is no impact for production.

@gewarren , in that case can I go ahead and remove the assembly version 8.0.0.0 from the necessary APIs.

In FileDialog.xml there are two types of APIs - 1) which were completely removed 2) which were moved up to the base class CommonItemDialog. Do we need to remove assembly version 8.0.0.0 in both cases ?

@gewarren
Copy link
Contributor

gewarren commented Sep 6, 2023

@gewarren , in that case can I go ahead and remove the assembly version 8.0.0.0 from the necessary APIs.

Yes, you can. It won't affect anything though.

In FileDialog.xml there are two types of APIs - 1) which were completely removed 2) which were moved up to the base class CommonItemDialog. Do we need to remove assembly version 8.0.0.0 in both cases ?

Remove it in both cases.

@gewarren
Copy link
Contributor

gewarren commented Sep 6, 2023

@huangmin-ms I logged https://dev.azure.com/ceapex/Engineering/_workitems/edit/896871.

@dipeshmsft
Copy link
Member Author

Yes, you can. It won't affect anything though.

Why so ?

@gewarren I have made the changes, is this PR ready to merge now?

@learn-build-service-prod
Copy link

Learn Build status updates of commit 38d0d41:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Win32/CommonItemDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialogCustomPlace.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFolderDialog.xml ✅Succeeded View
xml/Microsoft.Win32/SaveFileDialog.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@gewarren
Copy link
Contributor

gewarren commented Sep 6, 2023

Yes, you can. It won't affect anything though.

Why so ?

It only appears in the source (ECMAXML) and doesn't affect the published docs at all.

@gewarren I have made the changes, is this PR ready to merge now?

Yes, good for me.

Co-authored-by: Genevieve Warren <[email protected]>
@learn-build-service-prod
Copy link

Learn Build status updates of commit a692be0:

⚠️ Validation status: warnings

File Status Preview URL Details
xml/Microsoft.Win32/CommonItemDialog.xml ⚠️Warning View Details
xml/Microsoft.Win32/FileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialogCustomPlace.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFolderDialog.xml ✅Succeeded View
xml/Microsoft.Win32/SaveFileDialog.xml ✅Succeeded View

xml/Microsoft.Win32/CommonItemDialog.xml

  • Line 0, Column 0: [Warning: xref-not-found] Cross reference not found: 'NF:shobjidl_core.IFileDialogEvents.OnFileOk'.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@learn-build-service-prod
Copy link

Learn Build status updates of commit e370b1b:

✅ Validation status: passed

File Status Preview URL Details
xml/Microsoft.Win32/CommonItemDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/FileDialogCustomPlace.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFileDialog.xml ✅Succeeded View
xml/Microsoft.Win32/OpenFolderDialog.xml ✅Succeeded View
xml/Microsoft.Win32/SaveFileDialog.xml ✅Succeeded View

For more details, please refer to the build report.

For any questions, please:

@dipeshmsft
Copy link
Member Author

@gewarren , the build succeeded. Can we go ahead with the merge ?

@gewarren gewarren merged commit 73e7058 into dotnet:main Sep 7, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants