Skip to content

Also remember to persist window positions - #17066

Merged
Carlos Zamora (carlos-zamora) merged 1 commit into
mainfrom
dev/migrie/b/17010-restore-positions-too
Apr 17, 2024
Merged

Also remember to persist window positions#17066
Carlos Zamora (carlos-zamora) merged 1 commit into
mainfrom
dev/migrie/b/17010-restore-positions-too

Conversation

@zadjii-msft

Copy link
Copy Markdown
Member

This got lost in #16598. TerminalPage needs to ask the window where the it actually is, so it can persist it. More details in #16598 (comment)

Closes #17010

This got lost in #16598. `TerminalPage` needs to ask the window where the it
actually is, so it can persist it.

Closes #17010
Comment on lines +1958 to +1960
const auto launchPosRequest{ winrt::make<LaunchPositionRequest>() };
RequestLaunchPosition.raise(*this, launchPosRequest);
layout.InitialPosition(launchPosRequest.Position());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BTW as I mentioned briefly in #12633 we should probably use GetWindowPlacement here. It only needs the HWND which the TerminalPage already knows about. The struct could then be serialized to a JSON object. It even got WPF_ASYNCWINDOWPLACEMENT which may be useful for us: https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-windowplacement

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, though, we may also want to be careful about how that plays with focus mode too. Since forever, session restore + fomo has restored at the wrong size, I think wrongly including the size of the tab row. (I cannot for the life of me find that issue though)

@DHowett

Copy link
Copy Markdown
Member

Hey, does this apply to #17049 as well?

@zadjii-msft

Copy link
Copy Markdown
Member Author

Hey, does this apply to #17049 as well?

I'd guess that it does. The simple check is if anyone is still calling AppHost::_GetWindowLaunchPosition

@carlos-zamora
Carlos Zamora (carlos-zamora) added this pull request to the merge queue Apr 17, 2024
Merged via the queue into main with commit 643f716 Apr 17, 2024
@carlos-zamora
Carlos Zamora (carlos-zamora) deleted the dev/migrie/b/17010-restore-positions-too branch April 17, 2024 18:33
Dustin L. Howett (DHowett) pushed a commit that referenced this pull request Apr 23, 2024
This got lost in #16598. `TerminalPage` needs to ask the window where
the it actually is, so it can persist it. More details in
#16598 (comment)

Closes #17010

(cherry picked from commit 643f716)
Service-Card-Id: 92360686
Service-Version: 1.20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

[1.21 canary] Session restore is restoring to the wrong screen coordinates and window size

4 participants