Skip to content

Port System.Windows.Forms.Design components #632

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
Merged
merged 17 commits into from
Apr 5, 2019
Merged

Port System.Windows.Forms.Design components #632

merged 17 commits into from
Apr 5, 2019

Conversation

@JuditRose
Copy link
Contributor

@JuditRose JuditRose commented Mar 25, 2019 •
edited
Loading

Fixes #585

Components copied from framework.

@JuditRose JuditRose requested a review from a team as a code owner March 25, 2019 21:39
@JuditRose JuditRose mentioned this pull request Mar 25, 2019
Closed
6 tasks
@Tanya-Solyanik
Copy link
Contributor

Tanya-Solyanik commented Mar 26, 2019 

// Shared between dlls

please remove

Refers to: src/Common/src/CompModSwitches.cs:12 in 0acc17c. [](commit_id = 0acc17c839d686490efef0257478a271d6952e0d, deletion_comment = False)

Tanya-Solyanik
Tanya-Solyanik reviewed Mar 26, 2019
View reviewed changes
Tanya-Solyanik
Tanya-Solyanik reviewed Mar 26, 2019
View reviewed changes
@Tanya-Solyanik
Copy link
Contributor

Tanya-Solyanik commented Mar 26, 2019 

    public static extern IntPtr SetParent(HandleRef hWnd, HandleRef hWndParent);

SetParent is already available here, you don;t have to add it in NativeMethods. Please review other methods that you added to NativeMethods class as well. Perhaps all you need to do is to change the call site to Unsafe class name.

Refers to: src/Common/src/UnsafeNativeMethods.cs:1001 in 0acc17c. [](commit_id = 0acc17c839d686490efef0257478a271d6952e0d, deletion_comment = False)

@JuditRose JuditRose force-pushed the WFI-585-port-system-windows-forms-design-components branch from 0acc17c to 1969dc2 Compare March 26, 2019 17:18
@codecov
Copy link

codecov bot commented Mar 26, 2019 •
edited
Loading

Codecov Report

Merging #632 into master will decrease coverage by 1.4604%.
The diff coverage is 0.12187%.

@@                 Coverage Diff                 @@
##              master        #632         +/-   ##
===================================================
- Coverage   27.34119%   25.88078%   -1.46041%     
===================================================
  Files            948         975         +27     
  Lines         254700      268427      +13727     
  Branches       33853       35934       +2081     
===================================================
- Hits           69638       69471        -167     
- Misses        181175      195083      +13908     
+ Partials        3887        3873         -14
Flag Coverage Δ
#Debug 25.88078% <0.12187%> (-1.46041%) ⬇️
#production 17.80493% <0.12187%> (-1.14705%) ⬇️
#test 98.68298% <ø> (+0.01118%) ⬆️

@JuditRose
Copy link
Contributor Author

JuditRose commented Mar 27, 2019

Thanks for the comments, all addressed/fixed

@zsd4yr
Copy link
Contributor

zsd4yr commented Mar 28, 2019

LGTM @JuditRose

Tanya-Solyanik
Tanya-Solyanik reviewed Mar 29, 2019
View reviewed changes
Tanya-Solyanik
Tanya-Solyanik reviewed Mar 29, 2019
View reviewed changes
@Tanya-Solyanik
Copy link
Contributor

Tanya-Solyanik commented Mar 29, 2019

consider creating an issue to use these consts from the common assembly once it's available

Refers to: src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/DesignerActionPanel.cs:30 in 870ea14. [](commit_id = 870ea14, deletion_comment = False)

@Tanya-Solyanik
Copy link
Contributor

Tanya-Solyanik commented Mar 29, 2019 

                _formActive = true;

PLease scan all files you are porting for TODO comments, we are not supposed to have them.

Refers to: src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/DesignerActionPanel.cs:1299 in 870ea14. [](commit_id = 870ea14, deletion_comment = False)

Tanya-Solyanik
Tanya-Solyanik reviewed Mar 29, 2019
View reviewed changes
src/System.Windows.Forms.Design/src/System/Windows/Forms/Design/Behavior/BehaviorService.cs
// I don't think so, because the malicious control would still be on the frame.
new NamedPermissionSet("FullTrust").Assert();

foreach (AdornerWindow adornerWindow in AdornerWindow.s_adornerWindowList)
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik Mar 29, 2019

Choose a reason for hiding this comment

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

this is related to CAS, can go away

Copy link
Contributor Author

@JuditRose JuditRose Mar 29, 2019

Choose a reason for hiding this comment

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

Which part can go away?

Copy link
Contributor

@zsd4yr zsd4yr Apr 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@Tanya-Solyanik

Tanya-Solyanik
Tanya-Solyanik suggested changes Mar 29, 2019
View reviewed changes
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

🕐

@wjk wjk mentioned this pull request Mar 30, 2019
Merged
@zsd4yr zsd4yr requested a review from Tanya-Solyanik April 4, 2019 15:36
Tanya-Solyanik
Tanya-Solyanik approved these changes Apr 5, 2019
View reviewed changes
Copy link
Contributor

@Tanya-Solyanik Tanya-Solyanik 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, thanks!

@JuditRose JuditRose merged commit 28fc930 into master Apr 5, 2019
@zsd4yr zsd4yr deleted the WFI-585-port-system-windows-forms-design-components branch April 5, 2019 22:39
@JuditRose JuditRose mentioned this pull request Apr 9, 2019
Closed
59 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@hughbe hughbe hughbe left review comments

@zsd4yr zsd4yr zsd4yr approved these changes

@Tanya-Solyanik Tanya-Solyanik Tanya-Solyanik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JuditRose @Tanya-Solyanik @zsd4yr @hughbe