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.

Sign up for GitHub

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

Jump to bottom

Issus 4908 port save file dialog designer and folder browser dialog designer #9682

Merged
merged 10 commits into from
Aug 10, 2023
Merged

Issus 4908 port save file dialog designer and folder browser dialog designer #9682

merged 10 commits into from
Aug 10, 2023

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented Aug 8, 2023 •
edited
Loading

Related #4908

Proposed changes

  • Port SaveFileDialogDesigner and FolderBrowserDialogDesigner to runtime

Customer Impact

  • SaveFileDialogDesigner and FolderBrowserDialogDesigner can be designed in runtime

Regression?

  • No

Risk

  • Minimal

Screenshots

Before

BeforeChang

After

AfterChang

Test methodology

  • Manually (added SaveFileDialogDesigner and FolderBrowserDialogDesigner to DemoConsole test app)

Accessibility testing

Test environment(s)

  • .net 8.0.100-rc.1.23407.2
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 requested a review from a team as a code owner August 8, 2023 08:16
@ghost ghost assigned LeafShi1 Aug 8, 2023
@LeafShi1 LeafShi1 mentioned this pull request Aug 8, 2023
Closed
56 tasks
SimonZhao888
SimonZhao888 previously approved these changes Aug 8, 2023
View reviewed changes
dreddy-work
dreddy-work reviewed Aug 8, 2023
View reviewed changes
...ystem.Windows.Forms.Design/src/System/Windows/Forms/Design/FolderNameEditor.FolderBrowser.cs
@@ -94,7 +94,7 @@ public unsafe DialogResult ShowDialog(IWin32Window? owner)
}

// Retrieve the path from the IDList.
PWSTR selectedPath = default;
PWSTR selectedPath = pDisplayName;
Copy link
Member

Choose a reason for hiding this comment

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

why is this change for?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know why the value was set to "default" before, but the default value always points to "0", and when I set the property value, such as FolderBrowerDialog's SelectedPath, an exception occurs.
image

Compare with the code with NexFx, the selectedPath should use pDisplayName pointer.

@dreddy-work dreddy-work added the 📭 waiting-author-feedback The team requires more information from the author label Aug 8, 2023
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Aug 9, 2023
dreddy-work
dreddy-work previously approved these changes Aug 9, 2023
View reviewed changes
@dreddy-work dreddy-work added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Aug 9, 2023
@dreddy-work
Copy link
Member

please rebase.

@Tanya-Solyanik
Copy link
Member

Looks like feature/9.0 branch took a mad merge - nullability is missing in public API file?

src\System.Windows.Forms\src\System\Windows\Forms\TreeNodeCollection.cs(212,25): error RS0036: (NETCORE_ENGINEERING_TELEMETRY=Build) Symbol 'virtual System.Windows.Forms.TreeNodeCollection.AddRange(params System.Windows.Forms.TreeNode![]! nodes) -> void' is missing nullability annotations in the declared API (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md)

@Tanya-Solyanik Tanya-Solyanik added 📭 waiting-author-feedback The team requires more information from the author Net9.0 labels Aug 10, 2023
@LeafShi1
Copy link
Member Author

LeafShi1 commented Aug 10, 2023 •
edited
Loading

nullability
I removed Symbol 'virtual System.Windows.Forms.TreeNodeCollection.AddRange(System.Windows.Forms.TreeNode![]! nodes) -> void' from PublicAPI.Unshipped.txt and add "params" for it in PublicAPI.Shipped.txt

@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Aug 10, 2023
@LeafShi1
Removing not public API "virtual System.Windows.Forms.TreeNodeCollect…
6cb097a 
…ion.AddRange(System.Windows.Forms.TreeNode![]! nodes) -> void"

Removing not public API "virtual System.Windows.Forms.TreeNodeCollection.AddRange(System.Windows.Forms.TreeNode![]! nodes) -> void" from PublicAPI.Shipped.txt
@LeafShi1
Add nullability annotations for API "virtual System.Windows.Forms.Tre…
6229438 
…eNodeCollection.AddRange(params System.Windows.Forms.TreeNode![]! nodes) -> void" on PublicAPI.Unshipped.txt

Add nullability annotations for API "virtual System.Windows.Forms.TreeNodeCollection.AddRange(params System.Windows.Forms.TreeNode![]! nodes) -> void" on  PublicAPI.Unshipped.txt
@LeafShi1 LeafShi1 merged commit 6b3846a into dotnet:feature/9.0 Aug 10, 2023
9 checks passed
@ghost ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Aug 10, 2023
@LeafShi1 LeafShi1 deleted the Issus_4908_Port_SaveFileDialogDesigner_and_FolderBrowserDialogDesigner branch August 11, 2023 01:29
@Ashley-Li
Copy link

Ashley-Li commented Aug 11, 2023 •
edited
Loading

Verified this PR in feature/9.0 branch, now SaveFileDialogDesigner and FolderBrowserDialogDesigner are supported in runtime, we can modify SaveFileDialog/FolderBrowserDialog in DemoConsole app by their some properties:

9682.mp4

@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Epica3055 Epica3055 Epica3055 approved these changes

@dreddy-work dreddy-work dreddy-work left review comments

@SimonZhao888 SimonZhao888 SimonZhao888 left review comments

@Tanya-Solyanik Tanya-Solyanik Awaiting requested review from Tanya-Solyanik

Assignees

@LeafShi1 LeafShi1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@LeafShi1 @dreddy-work @Tanya-Solyanik @Ashley-Li @SimonZhao888 @Epica3055