Skip to content

Allow parent window to be set before initialized - #264

Open
pathartl wants to merge 1 commit into
tryphotino:masterfrom
LANCommander:feature/set-window-parent
Open

Allow parent window to be set before initialized#264
pathartl wants to merge 1 commit into
tryphotino:masterfrom
LANCommander:feature/set-window-parent

Conversation

@pathartl

Copy link
Copy Markdown

This PR adds the ability to set the parent window on a new window before it's instantiated. This allows applications built with photino.Blazor to have new windows with parents.

photino.Blazor utilizes dependency injection when creating new windows, so each window is effectively a separate Blazor app. This creates the undesired UI effect where one application can look like multiple. i.e. on Windows, multi-window Photino Blazor applications will not combine under one taskbar item.

A check was put in place to make sure that a native instance of the current window does not already exist. As the parent is used as a startup parameter, this mirrors checks put in for other pre-instantiated parameters such as chromeless.

Copilot AI left a comment

Copy link
Copy Markdown

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 adds a fluent API to set a PhotinoWindow’s parent window prior to native instantiation, enabling multi-window apps (e.g., photino.Blazor) to associate child windows with a parent for better OS-level grouping behavior.

Changes:

  • Makes the internal _dotNetParent field mutable (was readonly).
  • Adds SetParent(PhotinoWindow parent) with a guard to prevent changes after native instantiation.

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


/// <summary>
/// Gets the reference to parent PhotinoWindow instance.
/// This property can only be set in the constructor and it is optional.

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

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

The XML doc for Parent says it "can only be set in the constructor", but _dotNetParent is now mutable and can be set via SetParent(...). Please update the Parent documentation to reflect that it can be set either in the constructor or via SetParent (before native instantiation).

Suggested change
/// This property can only be set in the constructor and it is optional.
/// This property is optional and can be set in the constructor or via <c>SetParent(...)</c>
/// before native instantiation.

Copilot uses AI. Check for mistakes.
/// <param name="parent">The window that should be used as this window's parent</param>
public PhotinoWindow SetParent(PhotinoWindow parent)
{
Log($".SetParent({parent.Id})");

Copilot AI Apr 9, 2026

Copy link

Choose a reason for hiding this comment

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

SetParent logs parent.Id without validating parent. Since PhotinoWindow allows parent = null in the constructor, calling SetParent(null) will currently throw a NullReferenceException while logging. Please either (a) explicitly disallow null by throwing a clear exception before logging, or (b) support clearing the parent and log safely (e.g., handle null in the log message).

Suggested change
Log($".SetParent({parent.Id})");
Log($".SetParent({(parent is null ? "null" : parent.Id.ToString())})");

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants