chromelyapps / Chromely

Build Cross Platform HTML Desktop Apps on .NET using native GUI, HTML5, JavaScript, CSS, Owin, AspNetCore (MVC, RazorPages, Blazor)
MIT License
2.98k stars 279 forks source link

Feature request: Access accurate window dimensions and position to enable saving and restoring window position #275

Closed wwarby closed 3 years ago

wwarby commented 3 years ago

I've just submitted a pull request (#274) to demonstrate the solution I've come up with for something I don't think is quite possible with Chromely in it's current form: capturing the window size and position when it changes and restoring on the next run.

The APIs Chromely provides already makes this nearly possible, but there are two issues with the current implementation:

  1. Although the Window class provides a HostMoved event, it's event args don't provide access to the X and Y co-ordinates the window was moved to
  2. The Width and Height properties provided in the event arguments for the HostSizeChanged event don't accurately describe the window's width and height. *(that is, if you record their values unaltered and provide them back to the config.WindowOptions.Size startup option, the window will start smaller than it was on the last resize event, because the values written to SizeChangedEventArgs are not adjusted to account for the title bar and the absence of the default window border.

Essentially what I've done in #274 is fix those two problems (and provide an API to directly read the window bounds in GetWindowClientBounds rather than force the user to subscribe to events to gain access).

My use case looks something like this:

public class CustomWindow : Window {
    public CustomWindow(
        IChromelyNativeHost nativeHost,
        IChromelyConfiguration config,
        ChromelyHandlersResolver handlersResolver,
        IStateService stateService
    ) : base(nativeHost, config, handlersResolver) {

        NativeHost.HostSizeChanged += (_, e) => {
            stateService.State.WindowWidth = e.OuterWidth;
            stateService.State.WindowHeight = e.OuterHeight;
            stateService.SaveAsync();
        };

        NativeHost.HostMoved += (_, e) => {
            stateService.State.WindowLeft = e.X;
            stateService.State.WindowTop = e.Y;
            stateService.SaveAsync();
        };
    }
}

// Program.cs
var config = DefaultConfiguration.CreateForRuntimePlatform();
...
config.WindowOptions.StartCentered = false;
config.WindowOptions.Size = new WindowSize(stateService.State.WindowWidth, stateServiceState.WindowHeight);
config.WindowOptions.Position = new WindowPosition(stateService.State.WindowLeft, stateService.State.WindowTop);

AppBuilder
    .Create()
    ...
    .UseWindow<CustomWindow>()
    .Build()
    .Run(args);

I introduced OuterWidth and OuterHeight in addition to Width and Height because replacing the values for Width and Height caused side effects (they seem to be used internally and replacing them sizes the browser incorrectly within the window). The window dimensions are corrected by the use of the Win32 AdjustWindowRectEx API.

Anyway, for your consideration. If I were wishing for features, my ideal feature would be a config.WindowOptions.PreserveState = true config parameter that just did all this for me. If you think the idea has no merit for the general use case I'm happy to just maintain my own fork, or if you think the idea has merit but needs work I'm happy to collaborate if that would help. I've only done the Windows implementation and just stubbed the Mac / Linux implementations because I don't need them personally.

mattkol commented 3 years ago

@wwarby first, I personally do not see an idea as having merit or not. I take all propositions seriously, the question is always whether it is already provided for a core feature or nice to have - we want to keep it as lean as possible.

The 2 bulleted items you referred to are actually by design.

  1. Although the Window class provides a HostMoved event, it's event args don't provide access to the X and Y co-ordinates the window was moved to

This was added to allow for CEF NotifyMoveOrResizeStarted, sure it can be enhanced to do other things, if need be.

the window will start smaller than it was on the last resize event, because the values written to SizeChangedEventArgs are not adjusted to account for the title bar and the absence of the default window border.

This is by design, the event is to facilitate the resize of the client - the browser. This is not meant for the window rect, the outer.

But I know what you are trying to achieve and is already provided for.. in a way. This is usually done with WindowsPlacement ... https://github.com/chromelyapps/Chromely/blob/a05e50ba130a417c053114dbd958db5487e6de3e/src/Chromely/NativeHosts/WinHost/WinBase/Fullscreen.cs#L20

This was done for when there is a switch from normal to fullscreen... But can also be used when windows is closed and reopened..

My suggestion will be to use ChromelyAppUser.

https://github.com/chromelyapps/Chromely/blob/edbf50b307b18ee330ad93fc8fe17c3f9f8e4751/src/Chromely.Core/ChromelyApp.cs#L107

  1. Register a custom ChromelyAppUser
    services.AddSingleton<IChromelyAppSettings, CustomChromelyAppSettings>();
  2. Save on window close

https://github.com/chromelyapps/Chromely/blob/193f74fa34f6a22ad400df36bc5d9df4fb4f0a2a/src_5.0/Chromely/Windows/Window.cs#L126

You do not have to do anything here, if the the save takes care of Windows placement parameters.

    public class CustomAppSettings : IChromelyAppSettings
    {

        public overried void Save(IChromelyConfiguration config)
        {
           // Save X, Y, Width, Height from WindowsPlacement
        }
    }
  1. In a custom configuration
    
    public class CustomConfiguration : IChromelyConfiguration
    {
       public CustomConfiguration (IChromelyAppSettings chromelyAppSettings)
       {
     }
         Read back what is saved from 
        // Save X, Y, Width, Height from WindowsPlacement
    }


It does not have to be in the exact, same way. You do not have to save into a file, you can use registry, as long as there is a way to save and retrieve.

But if I get it wrong, or I missed the point.. 
wwarby commented 3 years ago

Hi @mattkol, thanks very much for the detailed reply. In principle I'd much rather use existing functionality if it's already available. I don't think I'd ever have arrived at the solution you proposed on my own so there's an argument to be made that exposing access to the window placement data could be made a little more visible in the API, but we can circle back to that. For now though I'm struggling to follow your proposed solution - specifically, // Save X, Y, Width, Height from WindowsPlacement. I've created myself a custom implementation of IChromelyAppSettings (inheriting DefaultAppSettings) which gives me access to IChromelyConfiguration as you described, but hunting through the properties of that object I see no exposed window placement data. I've obviously missed something simple here - can you shed a bit of light on where you intended me to get the window placement data from?

mattkol commented 3 years ago

@wwarby I meant something like - https://github.com/chromelyapps/Chromely/issues/279

Looks like it needs to be enhanced for Max window state and other options I did not capture.

so there's an argument to be made that exposing access to the window placement data could be made a little more visible in the API, but we can circle back to that.

Will be nice to see this proposition too. Is it possible you create a demo for that?

wwarby commented 3 years ago

Apologies for the delay on this - I do intend to put together a full demo, just currently got my head down trying to finish the app I'm using Chromely for. It's on my todo list, I'll circle back to it :)