Allow set window class name via options for Windows#3828
Conversation
WalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
v2/pkg/options/windows/windows.go (1)
121-123: LGTM! Consider adding validation and a constant for the default value.The addition of the
WindowClassNamefield to theOptionsstruct is well-placed and documented. It aligns with the PR objectives by allowing users to specify a custom class name for the window.Suggestions for improvement:
- Consider adding validation for the
WindowClassNameto ensure it meets Windows requirements for class names.- It might be beneficial to define a constant for the default value "wailsWindow" for consistency and easier maintenance.
Here's a suggested implementation incorporating these improvements:
const DefaultWindowClassName = "wailsWindow" type Options struct { // ... other fields ... // Class name for the window. If empty or invalid, DefaultWindowClassName will be used. WindowClassName string } // ValidateWindowClassName checks if the provided class name is valid for Windows func ValidateWindowClassName(className string) bool { // Implement validation logic here // For example, check length, allowed characters, etc. return true // Placeholder }This approach provides a clear default value and allows for validation of the class name.
v2/internal/frontend/desktop/windows/window.go (1)
73-79: LGTM! Consider a minor readability improvement.The implementation of a configurable window class name is well done and aligns with the PR objectives. It maintains backward compatibility while allowing for customization.
For slightly improved readability, consider using a ternary-like operation:
windowClassName := "wailsWindow" if windowsOptions != nil { windowClassName = winc.FirstNonEmpty(windowsOptions.WindowClassName, windowClassName) }This assumes the existence of a
FirstNonEmptyhelper function, which you might need to implement if it doesn't exist:func FirstNonEmpty(s ...string) string { for _, v := range s { if v != "" { return v } } return "" }This suggestion is optional and doesn't affect the functionality of the code.
website/docs/reference/options.mdx (2)
102-104: LGTM! Consider clarifying the default behavior in the comment.The new
WebviewGpuDisabledfield is well-placed and appropriately typed. The comment is clear, but it could be enhanced slightly.Consider updating the comment to explicitly mention the default behavior:
-// Disable GPU hardware acceleration for the webview +// Disable GPU hardware acceleration for the webview. Default is false (GPU acceleration enabled).
809-815: LGTM! Minor formatting suggestion.The new
WindowClassNamefield is well-documented and appropriately placed within the Windows options.Consider adjusting the formatting for consistency with other options:
-#### WindowClassName +#### WindowClassName - -Class name for the window. If empty, 'wailsWindow' will be used. - -Name: WindowClassName<br/> -Type: `string` +Class name for the window. If empty, 'wailsWindow' will be used. + +Name: WindowClassName<br/> +Type: `string`This aligns the format with other option descriptions in the document.
website/src/pages/changelog.mdx (1)
16-16: Minor typo in the Unreleased section headerThere's a small typo in the Unreleased section header. The word "Added" is repeated.
Consider removing the duplicate "Added":
-## [Unreleased] ### Added - Added option to set window class name on Windows. +## [Unreleased] ### Added - Option to set window class name on Windows.🧰 Tools
🪛 LanguageTool
[duplication] ~16-~16: Possible typo: you repeated a word
Context: ...f vulnerabilities. ## [Unreleased] ### Added - Added option to set window class name on Wind...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- v2/internal/frontend/desktop/windows/window.go (1 hunks)
- v2/pkg/options/windows/windows.go (1 hunks)
- website/docs/reference/options.mdx (3 hunks)
- website/src/pages/changelog.mdx (1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/src/pages/changelog.mdx
[duplication] ~16-~16: Possible typo: you repeated a word
Context: ...f vulnerabilities. ## [Unreleased] ### Added - Added option to set window class name on Wind...(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (5)
v2/pkg/options/windows/windows.go (1)
121-123: Summary: Valuable addition with minor suggestions for improvementThe addition of the
WindowClassNamefield is a valuable enhancement that aligns well with the PR objectives. It allows developers to set a unique class name for their applications, which is particularly useful for obtaining window handles for various Windows functions.Key points:
- The change is minimal and focused, which is good for maintainability.
- The new field is well-documented within the struct.
- We've suggested adding validation and a constant for the default value to improve robustness.
- Verification of the field's usage in other parts of the codebase is recommended to ensure full implementation.
Overall, this change appears to be a positive addition to the project, addressing the need for custom window class names in Wails applications on Windows.
v2/internal/frontend/desktop/windows/window.go (1)
Line range hint
1-479: Overall, the changes look good. Consider additional testing and documentation.The implementation of the configurable window class name is well-executed and fulfills the PR objectives without introducing breaking changes. The code is consistent with the existing style and doesn't raise any apparent security or performance concerns.
To ensure the changes work as expected across different scenarios, consider the following:
- Add unit tests to verify the behavior with different
WindowClassNamevalues.- Update the documentation to reflect this new configurable option.
- Consider adding a comment explaining the purpose of the custom window class name for future maintainers.
To verify the impact of these changes, you can run the following script:
This script will help ensure that the new feature is properly tested, documented, and explained.
website/docs/reference/options.mdx (2)
Line range hint
327-332: LGTM! Important compatibility information added.This note provides crucial information about compatibility issues between vite v5.0.0+ and wails v2, along with guidance for users and a link to more detailed information.
The addition of this note is valuable for users and helps prevent potential issues. It's well-placed within the AssetServer options context.
Line range hint
1-1000: Overall changes improve documentation and add valuable options.The updates to this file enhance the Wails options documentation in several ways:
- Addition of the
WindowClassNameoption for Windows, allowing for greater customization.- Introduction of the
WebviewGpuDisabledoption, providing more control over hardware acceleration.- Important compatibility note regarding vite and wails versions.
These changes provide developers with more flexibility and crucial information for using the Wails framework effectively.
The additions and modifications are well-integrated into the existing documentation structure and provide valuable enhancements to the Wails options.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~818-~818: When ‘Mac-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ... Type:string### Mac This defines Mac specific options. Name: Mac
Type: `...(SPECIFIC_HYPHEN)
website/src/pages/changelog.mdx (1)
Line range hint
1-1000: Missing changelog entry for v2.0.0-beta.41The changelog appears to be missing an entry for version v2.0.0-beta.41. This could be an oversight, or there might not have been any significant changes in this version.
To verify if this version exists and if there were any changes, you can run the following script:
If there were changes, consider adding an entry for this version in the changelog.
🧰 Tools
🪛 LanguageTool
[duplication] ~16-~16: Possible typo: you repeated a word
Context: ...f vulnerabilities. ## [Unreleased] ### Added - Added option to set window class name on Wind...(ENGLISH_WORD_REPEAT_RULE)
[duplication] ~19-~19: Possible typo: you repeated a word
Context: ...app/wails/pull/3828) by @APshenkin ### Fixed - Fixed cross compilation failed with CGO [PR](...(ENGLISH_WORD_REPEAT_RULE)
|
|
||
| // Class name for the window. If empty, 'wailsWindow' will be used. | ||
| WindowClassName string |
There was a problem hiding this comment.
💡 Codebase verification
WindowClassName is not utilized in window creation functions
The WindowClassName field is defined but not used in any window creation calls. Please ensure that this field is correctly integrated into the window creation logic.
- Update the window creation functions to utilize the
WindowClassNamefield where appropriate.
🔗 Analysis chain
Verify usage of the new WindowClassName field
The addition of the WindowClassName field looks good. To ensure full implementation:
- Verify that this new option is properly utilized in the window creation logic.
- Update any relevant documentation or examples to showcase this new feature.
- Consider adding a unit test to verify the behavior when this option is set.
Run the following script to check for usage of the new field:
This will help ensure that the new option is properly integrated into the existing codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usage of WindowClassName in the codebase
# Test: Search for WindowClassName usage
echo "Searching for WindowClassName usage:"
rg --type go 'WindowClassName'
# Test: Search for window creation logic that might need updating
echo "\nSearching for window creation logic:"
rg --type go 'CreateWindowEx|RegisterClassEx'
Length of output: 2332
Script:
#!/bin/bash
# Description: Verify if WindowClassName is used in window creation functions
# Search for WindowClassName near CreateWindowEx and RegisterClassEx
echo "Searching for WindowClassName usage near CreateWindowEx:"
rg --type go -C 2 'CreateWindowEx' | rg 'WindowClassName'
echo -e "\nSearching for WindowClassName usage near RegisterClassEx:"
rg --type go -C 2 'RegisterClassEx' | rg 'WindowClassName'
Length of output: 366
|
|
Thanks for this @APshenkin 🙏 |
Description
There could be cases, when developers need to get window handle and pass it to some Windows functions. For example, you may want to pass window handle to nCrypt API (https://learn.microsoft.com/en-us/windows/win32/seccng/key-storage-property-identifiers#ncrypt_window_handle_property). This change will allow to set unique className for your app, so that searching for window handle will not be affected if different Wails based apps are presented on the system
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor.Test Configuration
Please paste the output of
wails doctor. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdxwith details of this PRSummary by CodeRabbit
New Features
WindowClassNameoption for Windows application windows.Bug Fixes
Documentation
WindowClassNamefield and other configuration options.Chores