Implemented '546162 - Generate overrides code fix ui'#3699
Conversation
|
I implemented it here too. |
|
Vaclav had some issues on the design: http://vncr.in/ovIf |
|
Your version is definitely better, as it handles the pickmembers options. I'll remove the commit from my PR. |
|
Can you attach a screenshot of the UI? |
|
@Therzok: Why did you implement that - that's what we've task assignments for |
|
@mkrueger was easy copy-paste, wanted to have it functional before handing off codefixes service to QA. |
|
|
|
Not sure what's with the dialog buttons - the margins should be corrected by xwt automatically IMO. |
There was a problem hiding this comment.
Tha paddings/margins of an Xwt.Dialog can't be adjusted, because the native APIs don't allow it (see the Gtk.Dialog docs). The only way to have the dialog buttons "aligned" is to use a custom dialog with simple buttons inside the content area, without all the dialog result handling (it's a toolkit limitation, not Xwt).
But all the layout is not a big issue, if we use the native backend on Mac, the layout will be better there.
| { | ||
| PickMembersResult IPickMembersService.PickMembers(string title, ImmutableArray<ISymbol> members, ImmutableArray<PickMembersOption> options) | ||
| { | ||
| var dialog = new PickMembersDialog (); |
There was a problem hiding this comment.
Wrap this whole thing in an IdeApp.RunWhenIdle() and a Xwt.Toolkit.NativeEngine.Invoke to make the dialog native and to make sure that you don't lock the UI.
There was a problem hiding this comment.
I don't think that works here because I would need to return
res.Task.Result. And if it locks it would lock at that. However I can implement it like that - but no idea if it really would help in that case.
There was a problem hiding this comment.
Locks the UI in that case - it's running on the UI thread and the task gets added to the idle threat and res.Task.Result is locking there.
It's IMO wrong here to use that in this case because the actions run on the UI thread.
There was a problem hiding this comment.
Oh ok, I didn't look into the actual context, so if it doesn't come from some background task, then RunWhenIdle() doesn't make sense. But can we make it native using Xwt.Toolkit.NativeEngine.Invoke?
| this.Height = 321; | ||
| this.Resizable = false; | ||
|
|
||
| Show (); |
There was a problem hiding this comment.
Don't call Show() in the constructor.
| var dialog = new PickMembersDialog (); | ||
| try { | ||
| dialog.Init (title, members, options); | ||
| bool performChange = dialog.Run () == Xwt.Command.Ok; |
There was a problem hiding this comment.
shouldn't the dialog be transient? Using dialog.Run (Xwt.MessageDialog.RootWindow) will make it modal for the main IDE window.
There was a problem hiding this comment.
No should be modal. The result is required for making the editor to continue it's operation.
| var checkBoxCellView = new CheckBoxCellView (symbolIncludedField); | ||
| checkBoxCellView.Editable = true; | ||
| checkBoxCellView.Toggled += delegate { UpdateOkButton (); }; | ||
| listViewPublicMembers.Columns.Add ("", checkBoxCellView); |
There was a problem hiding this comment.
I won't use an additional column for this, just pack everything in one column, or even better use a ListBox.
There was a problem hiding this comment.
ListBox sounds reasonable - I usually just tend to use the more complex list view :)
|
Also see https://github.com/mono/monodevelop/blob/master/main/src/addins/MonoDevelop.PackageManagement/MonoDevelop.PackageManagement/LicenseAcceptanceService.cs#L48-L56 for a good native dialog example. |
3c6d553 to
9376984
Compare
|
@mkrueger This has been approved. Please fix the conflicts and merge. Thanks! |
slluis
left a comment
There was a problem hiding this comment.
Please add corresponding release notes.
https://devdiv.visualstudio.com/DevDiv/_workitems?id=546162&_a=edit
Test instructions :
class Test {
|
}