Skip to content

Conversation

@not-elm
Copy link
Owner

@not-elm not-elm commented Jul 23, 2025

Fixed an issue where the embedded webview position would sometimes be incorrect when the window moved.

Fixed an issue where the embedded webview position would sometimes be incorrect when the window moved.
@not-elm not-elm requested a review from Copilot July 23, 2025 13:44
@not-elm not-elm added the bug Something isn't working label Jul 23, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes webview positioning issues that occurred when windows were moved or resized. The changes address the tracking and positioning of embedded webviews by improving event handling and component management.

  • Enhanced window event handling to track both window moves and resizes
  • Updated component insertion/removal to use safer try_* methods
  • Optimized webview positioning updates with deduplication logic

Reviewed Changes

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

File Description
crates/bevy_webview_wry/src/webview/protocol.rs Switched to safer try_insert and try_remove methods for component management
crates/bevy_webview_wry/src/webview/load_webview.rs Added window move tracking, improved event handling with deduplication, and refined webview positioning logic
Comments suppressed due to low confidence (1)

crates/bevy_webview_wry/src/webview/load_webview.rs:337

  • [nitpick] The variable name 'windows' could be more descriptive. Consider renaming to 'processed_windows' or 'handled_windows' to clarify its purpose as a deduplication tracker.
    let mut windows = bevy::platform::collections::HashSet::new();

App, Commands, Entity, Name, NonSend, NonSendMut, Or, Plugin, PreUpdate, Query, Res, Window,
With, Without,
};
use bevy::prelude::*;
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using wildcard imports (use bevy::prelude::*;) can make code less readable and may introduce naming conflicts. Consider importing only the specific items needed or using the previous explicit import style.

Suggested change
use bevy::prelude::*;
use bevy::prelude::{App, Plugin, PreUpdate, Update, WindowMoved, on_event};

Copilot uses AI. Check for mistakes.
wry_webview
.ns_window()
.setFrame_display(ns_window.contentRectForFrameRect(ns_window.frame()), true);
let wry_ns_window = wry_webview.ns_window();
Copy link

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from true to false for the display parameter in setFrame_display should be documented. This parameter controls whether the window is immediately redrawn, and the change may have performance implications.

Suggested change
let wry_ns_window = wry_webview.ns_window();
let wry_ns_window = wry_webview.ns_window();
// The `display` parameter is set to `false` to avoid immediate redraw of the window.
// This is intentional to optimize performance, as the redraw is handled elsewhere.

Copilot uses AI. Check for mistakes.
@not-elm not-elm merged commit 8297e93 into main Jul 23, 2025
5 checks passed
@not-elm not-elm deleted the fix-webview-tracking branch July 23, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants