-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
window.ToggleFrameless() api
#4317
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.
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
base: v3-alpha
Are you sure you want to change the base?
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI (HTML/JS)
participant Runtime JS (window.ts)
participant Go Backend (window.go/webview_window.go)
participant OS Window
User->>UI (HTML/JS): Click "Toggle Frameless" button
UI (HTML/JS)->>Runtime JS: window.wails.Window.ToggleFrameless()
Runtime JS->>Go Backend: Invoke ToggleFrameless method
Go Backend->>OS Window: Toggle frameless state
OS Window-->>Go Backend: Frameless state toggled
Go Backend-->>Runtime JS: Success/acknowledgement
Runtime JS-->>UI (HTML/JS): (Optional) Update UI or state
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
v3/internal/assetserver/bundledassets/runtime.js (1)
1-2
: 💡 Verification agent🧩 Analysis chain
Consider generating minified files during the build process instead of committing them.
Committing minified/bundled JavaScript files makes code reviews challenging and can lead to merge conflicts. It's generally better to commit only the source files and generate the minified versions as part of the build process.
Run the following script to check if there's a corresponding source file and build process:
Additionally, ensure that:
- The source code for this runtime is properly maintained
- Source maps are generated for debugging purposes
- The build process is documented
🏁 Script executed:
#!/bin/bash # Description: Look for the source file and build configuration for runtime.js # Check for source files in the runtime directory fd -t f -e ts -e js runtime v3/internal/runtime/ # Check for build configuration files fd -t f "package.json|tsconfig.json|webpack.config|rollup.config|vite.config" v3/ # Check if there's a build script that generates this file rg -A 5 "runtime.js|bundledassets" --glob "*.json" --glob "*.js" --glob "*.ts"Length of output: 66113
🏁 Script executed:
#!/bin/bash # Inspect build scripts in the runtime package jq '.scripts' v3/internal/runtime/desktop/@wailsio/runtime/package.jsonLength of output: 780
Automate bundling of
runtime.js
and remove it from VCSWe have a maintained TypeScript source and build in
v3/internal/runtime/desktop/@wailsio/runtime/
:
- Source:
v3/internal/runtime/desktop/@wailsio/runtime/src/runtime.ts
- Build scripts:
"build": "npm run build:code & npm run build:docs & wait"
in itspackage.json
But the minified result at
v3/internal/assetserver/bundledassets/runtime.js
is still checked into Git. Committing generated assets makes reviews brittle and invites merge conflicts.Action items:
- Move
bundledassets/runtime.js
out of version control (add to.gitignore
).- Add a Go
//go:generate
or CI step that:
- Runs
npm ci && npm run build
in the TS runtime package.- Invokes your assetserver bundler to copy/minify into
bundledassets/runtime.js
.- Document these steps (e.g. in CONTRIBUTING.md), so that regenerating the asset is automatic.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
🧹 Nitpick comments (1)
v3/examples/frameless/assets/index.html (1)
49-49
: Fix invalid CSS color value.The
lightred
color value is not a valid CSS color. Consider using a valid CSS color instead.- <div class="quarter" style="background-color: lightred;"><div>Not Draggable</div><button id="toggle-frameless">Toggle Frameless</button></div> + <div class="quarter" style="background-color: lightcoral;"><div>Not Draggable</div><button id="toggle-frameless">Toggle Frameless</button></div>Alternative valid options:
lightcoral
,lightpink
,#ffcccb
, orrgb(255, 182, 193)
.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/src/content/docs/changelog.mdx
(1 hunks)v3/examples/frameless/assets/index.html
(2 hunks)v3/internal/assetserver/bundledassets/runtime.js
(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/package.json
(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts
(1 hunks)v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts
(2 hunks)v3/pkg/application/messageprocessor_window.go
(8 hunks)v3/pkg/application/webview_window.go
(6 hunks)v3/pkg/application/window.go
(1 hunks)v3/pkg/events/events.go
(6 hunks)v3/pkg/events/events.txt
(2 hunks)v3/pkg/events/events_darwin.h
(1 hunks)v3/pkg/events/events_linux.h
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
v3/pkg/application/window.go (1)
v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (1)
ToggleFrameless
(460-462)
v3/pkg/events/events.go (1)
v3/pkg/application/messageprocessor_window.go (14)
WindowToggleFrameless
(50-50)WindowRestore
(33-33)WindowShow
(46-46)WindowUnFullscreen
(51-51)WindowUnMaximise
(52-52)WindowUnMinimise
(53-53)WindowZoom
(55-55)WindowZoomIn
(56-56)WindowZoomOut
(57-57)WindowZoomReset
(58-58)WindowHide
(21-21)WindowMaximise
(26-26)WindowMinimise
(27-27)WindowFullscreen
(17-17)
v3/pkg/application/webview_window.go (5)
v3/pkg/events/events.go (2)
ApplicationEventType
(3-3)WindowEventType
(4-4)v3/pkg/application/events.go (1)
ApplicationEvent
(11-16)v3/pkg/application/messageprocessor_dialog.go (1)
DialogError
(14-14)v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (2)
ToggleFrameless
(460-462)SetFrameless
(347-349)v3/pkg/application/mainthread.go (1)
InvokeSync
(23-32)
🪛 Biome (1.9.4)
v3/internal/assetserver/bundledassets/runtime.js
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
🔇 Additional comments (22)
v3/internal/runtime/desktop/@wailsio/runtime/package.json (1)
4-4
: LGTM! Appropriate version bump for new feature.The version increment from
3.0.0-alpha.67
to3.0.0-alpha.68
correctly follows semantic versioning conventions for alpha releases when adding new functionality.v3/pkg/events/events.txt (1)
15-15
: LGTM! Correctly placed and named event.The new
common:WindowToggleFrameless
event follows the established naming convention and is properly positioned in alphabetical order within the common window events section.v3/pkg/application/window.go (1)
79-79
: LGTM! Well-positioned interface method addition.The
ToggleFrameless()
method follows Go interface conventions and is logically grouped with other toggle methods. The signature is consistent with the existing toggle methods pattern.docs/src/content/docs/changelog.mdx (1)
36-36
: LGTM! Well-formatted changelog entry.The changelog entry follows the project's established format with proper attribution, PR reference, and code formatting. It's correctly placed in the "Added" section under "Unreleased".
v3/internal/runtime/desktop/@wailsio/runtime/src/event_types.ts (1)
220-220
: LGTM! Correctly implemented event type.The new
WindowToggleFrameless
event type follows the established naming conventions and is properly placed in alphabetical order within the Common events section. The event string uses the appropriate"common:"
namespace prefix for cross-platform functionality.v3/examples/frameless/assets/index.html (1)
37-40
: LGTM! Correct event handler implementation.The event listener for the toggle frameless button follows the same pattern as the existing minimize and close button handlers, properly calling
window.wails.Window.ToggleFrameless()
.v3/internal/runtime/desktop/@wailsio/runtime/src/window.ts (3)
54-54
: LGTM! Correct method constant addition.The new
ToggleFramelessMethod
constant is properly assigned the next sequential value (40) and follows the established naming convention.
55-62
: LGTM! Proper constant value adjustments.All subsequent method constants have been correctly incremented by 1 to maintain the sequential numbering after the insertion of
ToggleFramelessMethod
. This ensures no conflicts in method identifiers.
457-462
: LGTM! Correct method implementation.The
ToggleFrameless
method implementation follows the exact same pattern as other toggle methods (ToggleFullscreen
,ToggleMaximise
):
- Proper JSDoc documentation
- Correct return type (
Promise<void>
)- Uses the appropriate method constant
- Consistent formatting and style
v3/pkg/events/events_darwin.h (1)
9-142
: LGTM! Correct event ID adjustments for new event insertion.All macOS event ID constants have been systematically incremented by 1, which is necessary to accommodate the new
WindowToggleFrameless
event (ID 1038) while maintaining sequential, non-overlapping event IDs. TheMAX_EVENTS
value has been properly updated from 1188 to 1189.This change aligns with the cross-platform event system requirements and ensures consistency across all platform-specific event headers.
v3/pkg/application/messageprocessor_window.go (4)
50-58
: LGTM: Consistent event ID reorganization for new toggle functionality.The addition of
WindowToggleFrameless = 40
and the systematic increment of subsequent constant values maintains the proper sequence and prevents ID conflicts. This aligns with the coordinated event system updates across other files.
102-102
: LGTM: Proper method name mapping added.The mapping entry correctly associates the constant with the method name for debugging and logging purposes.
113-119
: LGTM: Improved code readability with multi-line formatting.The function signature formatting enhances readability without changing functionality. This follows good Go formatting practices for functions with multiple parameters.
397-399
: LGTM: Consistent implementation pattern for toggle functionality.The new case handler follows the established pattern:
- Calls the appropriate window method (
window.ToggleFrameless()
)- Responds with success using
m.ok(rw)
This is consistent with other toggle methods like
WindowToggleFullscreen
andWindowToggleMaximise
.v3/pkg/events/events_linux.h (1)
9-18
: LGTM: Coordinated event ID shift maintains proper sequence.The systematic increment of all Linux-specific event IDs by 1 is necessary to accommodate the new
WindowToggleFrameless
event inserted at ID 1038 in the common events. TheMAX_EVENTS
update to 1057 is also correct. This maintains non-overlapping event ID ranges across platforms.v3/pkg/events/events.go (4)
23-23
: LGTM: New window event type properly declared.The
WindowToggleFrameless
field is correctly added to thecommonEvents
struct, maintaining consistency with other window event types.
52-62
: LGTM: Systematic event ID assignment maintains sequence integrity.The new
WindowToggleFrameless
event is assigned ID 1038, properly positioned betweenWindowMinimise
(1037) andWindowRestore
(1039). All subsequent event IDs are correctly incremented by 1 to maintain the sequence without conflicts.
81-88
: LGTM: Platform-specific event IDs properly coordinated.All platform-specific event IDs (Linux, Mac, Windows) have been systematically incremented by 1 to maintain non-overlapping ranges after the insertion of the new common event. This ensures consistent event ID management across all platforms.
Also applies to: 231-362, 417-460
483-493
: LGTM: JavaScript event mapping properly updated.The new event mapping
1038: "common:WindowToggleFrameless"
is correctly added, and all subsequent mappings are properly incremented to reflect the new event ID sequence. This ensures proper event handling in the JavaScript runtime.v3/pkg/application/webview_window.go (2)
210-213
: LGTM: Improved code readability with consistent multi-line formatting.The function signature and call formatting changes enhance code readability without altering functionality. This follows Go best practices for functions with multiple parameters.
Also applies to: 293-295, 318-325, 331-337, 343-349, 355-362, 774-778, 799-802
1011-1019
: LGTM: Well-implemented toggle method following established patterns.The
ToggleFrameless
method implementation is excellent:
- Thread Safety: Properly uses
InvokeSync
for main thread execution- State Validation: Correctly checks for
nil impl
and destroyed window state- Logic: Toggles frameless state by calling
SetFrameless(!w.options.Frameless)
- Consistency: Follows the exact same pattern as
ToggleFullscreen()
andToggleMaximise()
The implementation provides the expected functionality of toggling the frameless state without requiring developers to manually track the current state.
v3/internal/assetserver/bundledassets/runtime.js (1)
1-1
: TheToggleFrameless()
method is correctly integrated.I can confirm that the new
ToggleFrameless()
method has been properly added to the Window class in the minified runtime, implementing the PR's main objective.🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: This array contains an empty slot.
Unsafe fix: Replace hole with undefined
(lint/suspicious/noSparseArray)
[error] 1-1: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: Do not add then to a class.
(lint/suspicious/noThenProperty)
Implements the
window.ToggleFrameless()
api internally it just uses thewindow.setFrameless
toggling it as the opposite of the current value so Developer does not need to card about the current state to swap itChanges:
messageprocessor_window.go
window.ts
Summary by CodeRabbit
New Features
Documentation
Style