updated object inspector settings #871
Conversation
ilexp
added
Editor
| @@ -53,9 +54,36 @@ public bool EmptySelection | |||
| public ObjectInspector(int runtimeId) | |||
| { | |||
| this.InitializeComponent(); | |||
| this.buttonLock.CheckedChanged += (sender, args) => this.UserSettings.Locked = this.buttonLock.Checked; | |||
There was a problem hiding this comment.
The winforms designer is not working for me anymore so I added the changed handler myself here.
Not exactly sure why its not working anymore on visual studio 16.6.5.
There was a problem hiding this comment.
Since the designers failure is not "by design" (ha.. sorry 😅), but will likely be fixed in some future version, I'd prefer to put the event registration line manually into the proper spot inside InitializeComponent and make the registered method an actual named method in the class following the naming scheme. That way, the designer will pick it up without issues once it starts working again.
| public ObjectInspectorState UserSettings | ||
| { | ||
| get { return this.userSettings; } | ||
| set { this.userSettings = value; } | ||
| } |
There was a problem hiding this comment.
Style: This property should follow the grouping of the others, i.e. not have a newline following it.
| @@ -28,7 +23,13 @@ public partial class ObjectInspector : DockContent | |||
| private ObjectSelection.Category displayCat = ObjectSelection.Category.None; | |||
|
|
|||
| private ExpandState gridExpandState = new ExpandState(); | |||
|
|
|||
| private ObjectInspectorState userSettings; | |||
There was a problem hiding this comment.
Local style: Either remove the table-like alignment from all the fields, or make the new field align with the existing ones, as to not introduce inconsistency.
| @@ -53,9 +54,36 @@ public bool EmptySelection | |||
| public ObjectInspector(int runtimeId) | |||
| { | |||
| this.InitializeComponent(); | |||
| this.buttonLock.CheckedChanged += (sender, args) => this.UserSettings.Locked = this.buttonLock.Checked; | |||
There was a problem hiding this comment.
Since the designers failure is not "by design" (ha.. sorry 😅), but will likely be fixed in some future version, I'd prefer to put the event registration line manually into the proper spot inside InitializeComponent and make the registered method an actual named method in the class following the naming scheme. That way, the designer will pick it up without issues once it starts working again.
| @@ -30,7 +24,7 @@ internal static ObjectInspectorPlugin Instance | |||
|
|
|||
| private List<ObjectInspector> objViews = new List<ObjectInspector>(); | |||
| private bool isLoading = false; | |||
|
|
|||
| private ObjectInspectorSettings userSettings; | |||
There was a problem hiding this comment.
Local style: Looks like the fields are meant to be laid out table-style, but the usage of inline tabs for alignment messes it up. Can you do a drive-by fix here and either remove the alignment from all the fields, or fix it by using inline-spaces? If you do the latter, make sure to also include the new field.
| @@ -30,7 +24,7 @@ internal static ObjectInspectorPlugin Instance | |||
|
|
|||
| private List<ObjectInspector> objViews = new List<ObjectInspector>(); | |||
| private bool isLoading = false; | |||
|
|
|||
| private ObjectInspectorSettings userSettings; | |||
There was a problem hiding this comment.
Style: Please do an explicit value assignment to null for consistency and readability reasons.
| } | ||
| protected override void LoadUserData(XElement node) | ||
|
|
There was a problem hiding this comment.
Style: Remove the newline here to keep the current member grouping in place, or (if changing the group layout) keep together SaveUserData and LoadUserData in the same group.
|
|
||
| public class ObjectInspectorState | ||
| { | ||
| private int id; |
There was a problem hiding this comment.
Style: Please group all the fields and all the properties separately, as seen here.
| public List<ObjectInspectorState> ObjectInspectors { get; set; } = new List<ObjectInspectorState>(); | ||
| } | ||
|
|
||
| public class ObjectInspectorState |
There was a problem hiding this comment.
Class and public properties need XML docs! 🙂
No description provided.