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

Don't auto-dismiss the warning dialog on launch #15273

Merged

Conversation

zadjii-msft
Copy link
Member

Apparently, ShowWindow also sends a WM_MOVE, which we then turn around and use to dismiss open dialogs.

Closes #15170

Regressed in #13811

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels May 1, 2023
@github-actions

This comment has been minimized.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

This seems like us working around implicit magic. Is there no way to suppress the WM_MOVE?

@zadjii-msft zadjii-msft added this to the Terminal v1.18 milestone May 1, 2023
@zadjii-msft
Copy link
Member Author

This seems like us working around implicit magic. Is there no way to suppress the WM_MOVE?

Didn't seem like it. I might be able to filter based off if the old window rect and the new one are the same, but that might not work for the initial show

Comment on lines +1261 to +1268
// Don't set our state to Initialized until after the call to ShowWindow.
// When we call ShowWindow, the OS will also send us a WM_MOVE, which we'll
// then use to try and dismiss an open dialog. This creates the unintended
// side effect of immediately dismissing the initial warning dialog, if
// there were settings load warnings.
//
// In AppHost::_WindowMoved, we'll make sure we're at least initialized
// before dismissing open dialogs.
Copy link
Member

Choose a reason for hiding this comment

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

BODGY?

Copy link
Member Author

Choose a reason for hiding this comment

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

i mean, at this point:

// BODGY

@carlos-zamora carlos-zamora added Needs-Second It's a PR that needs another sign-off AutoMerge Marked for automatic merge by the bot when requirements are met labels May 2, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot deleted the dev/migrie/b/15170-auto-dismiss-warnings branch May 2, 2023 18:22
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Second It's a PR that needs another sign-off label May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. zBugBash-Consider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings error/warning dialog seems to self-dismiss on startup
4 participants