Skip to content
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

Width and Height getters on Window returns NaN on Mac #18060

Open
codecat opened this issue Jan 27, 2025 · 9 comments
Open

Width and Height getters on Window returns NaN on Mac #18060

codecat opened this issue Jan 27, 2025 · 9 comments
Labels
by-design The behavior reported in the issue is actually correct. question

Comments

@codecat
Copy link
Contributor

codecat commented Jan 27, 2025

Describe the bug

On Mac, the getters for Width and Height are returning NaN instead of the actual window size, when the size is not already set. It seems the difference between Windows and Mac is that on Mac, there is no entry in ValueStore._effectiveValues for it.

To Reproduce

Create a window where the xaml doesn't contain Width=".." Height="..". For example:

<Window xmlns="https://github.com/avaloniaui"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d"
        x:Class="TestApp.MainWindow"
        Title="Test App">
	<TextBlock>Hello world</TextBlock>
</Window>

Print the Width and Height anywhere:

protected override void OnOpened(EventArgs e)
{
	base.OnOpened(e);

	Console.WriteLine($"Width = {Width} Height = {Height}");
}

Expected behavior

On Windows, Width and Height are the actual window size. On Mac, the returned values are both NaN.

Avalonia version

master

OS

macOS

Additional context

This popped up when writing an integration test for #18004. I have not tested the behavior on Linux.

@Gillibald
Copy link
Contributor

You should always listen to Bounds -> RenderSize and not listen to (Width and Height) -> requested size

@timunie timunie added question by-design The behavior reported in the issue is actually correct. and removed bug labels Jan 27, 2025
@maxkatz6
Copy link
Member

@Gillibald I am not sure if that's the case with Window Height/Width.

@Gillibald
Copy link
Contributor

Gillibald commented Jan 28, 2025

The FrameSize/ClientSize should always be set in the end

@codecat
Copy link
Contributor Author

codecat commented Jan 28, 2025

This feels like a misleading API then. Perhaps the properties should be made private, or renamed (to something like RequestedWidth), but I understand also that this probably has to stay for xaml compatibility. (Unless there's an attribute that can change xaml property name separately from code property name?)

If this is "by design", it's not a very good design, imho.

@stevemonaco
Copy link
Contributor

The XML docs here could use some improvement.

/// <summary>
/// Gets or sets the width of the element.
/// </summary>
public double Width
{
get { return GetValue(WidthProperty); }
set { SetValue(WidthProperty, value); }
}
/// <summary>
/// Gets or sets the height of the element.
/// </summary>
public double Height
{
get { return GetValue(HeightProperty); }
set { SetValue(HeightProperty, value); }
}

WPF says it more accurately: Gets or sets the suggested height of the element.

Keep in mind that all layout properties on a control are suggestions: parent layout controls are free to completely ignore them if they wish although good ones comply as much as possible. Height, MinHeight, MaxHeight, alignments, padding, margin, etc.

This design is almost 20 years old from WPF, so it's both well-known and backwards-compatible. UWP follows the same. On Maui, there's Height (readonly) and HeightRequest which makes things a bit clearer for beginners.

@timunie
Copy link
Contributor

timunie commented Jan 28, 2025

I agree on the xml comments. It should also explain what NaN means for the system.

@robloo
Copy link
Contributor

robloo commented Jan 30, 2025

So Xamarin.Forms / MAUI actually made some changes in this area. They have HeightRequest and WidthRequested properties to make it clear that setting them doesn't guarantee the actual control size. Then Height/Width/Bounds in MAUI are read-only getters indicating it's what they actually are at the end of layout/arrange.

Avalonia follows WPF which is fine most of the time but doesn't make it completely clear in all cases like MAUI. I still prefer WPF/Avalonia naming for the brevity though. You just have to remember Height/Width are a request at the end of the day. In WPF you would be reading ActualHeight/ActualWidth.

WPF Avalonia MAUI
Height Height HeightRequest
Width Width WidthRequest
ActualHeight Bounds.Height Height (or use Bounds)
ActualWidth Bounds.Width Width (or use Bounds)

@codecat
Copy link
Contributor Author

codecat commented Jan 30, 2025

I see. On Windows, Width and Height are not NaN in the example from this issue; they are equal to Bounds.Width and Bounds.Height respectively, unlike on Mac.

Should the behavior on Windows change to match Mac, then?

codecat added a commit to codecat/Avalonia that referenced this issue Jan 30, 2025
@timunie
Copy link
Contributor

timunie commented Jan 30, 2025

I see. On Windows, Width and Height are not NaN in the example from this issue; they are equal to Bounds.Width and Bounds.Height respectively, unlike on Mac.

Should the behavior on Windows change to match Mac, then?

I don't think so, as different platforms have different window managers and we can't make all of them behave the same.

github-merge-queue bot pushed a commit that referenced this issue Feb 1, 2025
* Added new test: changing window size should not change position

* Removed unnecessary window placement assignment to fix window position shifting due to incorrect workspace coordinate usage

* Fixed test because buttons went off the window on tiny screen resolutions

* Make test Windows specific

* Use `Bounds` instead of `Width` and `Height` properties as they are "requested" size rather than actual size (see #18060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by-design The behavior reported in the issue is actually correct. question
Projects
None yet
Development

No branches or pull requests

6 participants