Add more information to zipmod list and properties #234

2025-10-30T01:46:05Z

ManlyMarco commented

2025-10-30 01:46:05 +00:00

(Migrated from github.com)

No description provided.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-10-30 01:50:06 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR enhances the sideloader viewer UI with a resizable split container layout and improves property display for mod metadata. The key changes include restructuring the UI for better flexibility, adding a new column for content type, and improving the browsability of manifest data in property grids.

Replaces fixed layout with nested split containers for resizable panels
Adds ContentsKind column to display mod content types
Enhances property grid display with TypeConverters and ToString methods

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
SideloaderViewer.resx	Updates UI layout metadata to use split containers instead of fixed docking
SideloaderViewer.Designer.cs	Adds splitContainer1 and splitContainer2 controls with proper initialization
SideloaderModsWindow.resx	Adds ContentsKind column metadata and reorders existing column properties
SideloaderModsWindow.Designer.cs	Implements the new olvColumnContentType column in the object list view
SideloaderModInfo.cs	Removes [Browsable(false)] from Manifest property to show it in property grid
MigrationInfo.cs	Adds ReadOnly attribute and ToString method for better property grid display
Manifest.cs	Adds TypeConverter attributes and [Browsable(false)] to ManifestDocumentRoot

Files not reviewed (2)

src/KKManager/Windows/Content/SideloaderModsWindow.Designer.cs: Language not supported
src/KKManager/Windows/ToolWindows/Properties/Viewers/SideloaderViewer.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull Request Overview This PR enhances the sideloader viewer UI with a resizable split container layout and improves property display for mod metadata. The key changes include restructuring the UI for better flexibility, adding a new column for content type, and improving the browsability of manifest data in property grids. - Replaces fixed layout with nested split containers for resizable panels - Adds ContentsKind column to display mod content types - Enhances property grid display with TypeConverters and ToString methods ### Reviewed Changes Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | SideloaderViewer.resx | Updates UI layout metadata to use split containers instead of fixed docking | | SideloaderViewer.Designer.cs | Adds splitContainer1 and splitContainer2 controls with proper initialization | | SideloaderModsWindow.resx | Adds ContentsKind column metadata and reorders existing column properties | | SideloaderModsWindow.Designer.cs | Implements the new olvColumnContentType column in the object list view | | SideloaderModInfo.cs | Removes [Browsable(false)] from Manifest property to show it in property grid | | MigrationInfo.cs | Adds ReadOnly attribute and ToString method for better property grid display | | Manifest.cs | Adds TypeConverter attributes and [Browsable(false)] to ManifestDocumentRoot | </details> <details> <summary>Files not reviewed (2)</summary> * **src/KKManager/Windows/Content/SideloaderModsWindow.Designer.cs**: Language not supported * **src/KKManager/Windows/ToolWindows/Properties/Viewers/SideloaderViewer.Designer.cs**: Language not supported </details> --- 💡 <a href="/IllusionMods/KKManager/new/master/.github?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

src/KKManager.Core/Data/Zipmods/MigrationInfo.cs

					
				@ -59,0 +76,4 @@

				            if (IDOld != 0 || IDNew != 0)

				            {

				                sb.AppendFormat(" idOld=\"{0}\"", IDOld);

				                sb.AppendFormat(" idNew=\"{0}\"", IDNew);

Using the fully qualified type name System.Text.StringBuilder is unnecessary since using System.Text could be inferred or would typically be present. However, since this file already has a using statement added at the top, consider using just StringBuilder for consistency with the using System.ComponentModel pattern.

Using the fully qualified type name `System.Text.StringBuilder` is unnecessary since `using System.Text` could be inferred or would typically be present. However, since this file already has a `using` statement added at the top, consider using just `StringBuilder` for consistency with the `using System.ComponentModel` pattern.

src/KKManager.Core/Data/Zipmods/MigrationInfo.cs

The condition IDOld != 0 || IDNew != 0 may not correctly represent the intent. If the goal is to show ID values when either is non-zero, this is correct. However, if both IDs should be shown together or not at all, consider using && instead. Additionally, when only one ID is non-zero, showing both might be confusing since one will display 0.

            if (IDOld != 0 && IDNew != 0)
            {
                sb.AppendFormat(" idOld=\"{0}\"", IDOld);
                sb.AppendFormat(" idNew=\"{0}\"", IDNew);
            }
            else if (IDOld != 0)
            {
                sb.AppendFormat(" idOld=\"{0}\"", IDOld);
            }
            else if (IDNew != 0)
            {
                sb.AppendFormat(" idNew=\"{0}\"", IDNew);
            }

The condition `IDOld != 0 || IDNew != 0` may not correctly represent the intent. If the goal is to show ID values when either is non-zero, this is correct. However, if both IDs should be shown together or not at all, consider using `&&` instead. Additionally, when only one ID is non-zero, showing both might be confusing since one will display 0. ```suggestion if (IDOld != 0 && IDNew != 0) { sb.AppendFormat(" idOld=\"{0}\"", IDOld); sb.AppendFormat(" idNew=\"{0}\"", IDNew); } else if (IDOld != 0) { sb.AppendFormat(" idOld=\"{0}\"", IDOld); } else if (IDNew != 0) { sb.AppendFormat(" idNew=\"{0}\"", IDNew); } ```

Sign in to join this conversation.

No reviewers