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.

Sign up for GitHub

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

Jump to bottom

Simplify non-client hit testing in NCIslandWindow to fix doubleclick #3024

Merged
merged 1 commit into from
Oct 2, 2019
Merged

Simplify non-client hit testing in NCIslandWindow to fix doubleclick #3024

merged 1 commit into from
Oct 2, 2019

Conversation

DHowett-MSFT
Copy link
Contributor

It turns out that our WM_LBUTTONDOWN handler wasn't even necessary, as
our NCHITTEST tells win32 that all of the titlebar is actually
non-client area. This brings the code in line with
NonNonClientIslandWindow.

Fixes #2513

Validation Steps Performed

Went wild with the clicking and dragging.

@DHowett
Simplify non-client hit testing in NCIslandWindow to fix doubleclick
45460be 
It turns out that our WM_LBUTTONDOWN handler wasn't even necessary, as
our NCHITTEST tells win32 that all of the titlebar is actually
non-client area. This brings the code in line with
NonNonClientIslandWindow.

Fixes #2513
@zadjii-msft
Copy link
Member

Does this regress #2028/#2438?

zadjii-msft
zadjii-msft approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

seems good to me..., but definitely worried about that other issue regressing. Considering you added the _DragRegionClickedHandlers call w/in the WM_NC*BUTTON* handlers, this looks like it'll work though.

@zadjii-msft zadjii-msft added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Oct 2, 2019
@DHowett-MSFT
Copy link
Contributor Author

Nah, I tested :)

@DHowett-MSFT DHowett-MSFT added the Needs-Second It's a PR that needs another sign-off label Oct 2, 2019
@ghost ghost requested review from miniksa and carlos-zamora October 2, 2019 16:50
carlos-zamora
carlos-zamora approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

I love it when we mostly delete code and get more functionality haha

@carlos-zamora carlos-zamora merged commit 621d841 into master Oct 2, 2019
@carlos-zamora carlos-zamora deleted the dev/duhowett/minmax-2 branch October 2, 2019 17:25
DHowett-MSFT pushed a commit that referenced this pull request Oct 3, 2019
@DHowett
Simplify non-client hit testing in NCIslandWindow to fix doubleclick (#…
42e281d 
…3024)

It turns out that our WM_LBUTTONDOWN handler wasn't even necessary, as
our NCHITTEST tells win32 that all of the titlebar is actually
non-client area. This brings the code in line with
NonNonClientIslandWindow.

Fixes #2513
@ghost
Copy link

ghost commented Oct 4, 2019

🎉Windows Terminal Preview v0.5.2762.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Oct 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zadjii-msft zadjii-msft zadjii-msft approved these changes

@carlos-zamora carlos-zamora carlos-zamora approved these changes

@miniksa miniksa Awaiting requested review from miniksa

Assignees
No one assigned
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NonClientIslandWindow -- titlebar doubleclick no longer minmaxes
4 participants
@DHowett-MSFT @zadjii-msft @carlos-zamora @DHowett