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

Apply a GDI region to the top level Island window to allow dragging with a single Xaml Island #929

Merged
merged 49 commits into from
Jun 25, 2019

Conversation

ocalvo
Copy link
Contributor

@ocalvo ocalvo commented May 21, 2019

Summary of the Pull Request

When Terminal is drawing in the non client area we no longer create 2 islands. Instead we create a single island and create a region that "pokes" the drag-able title bar area in the top right corner of the main window of terminal.

Capture

This change also fixes:

  • A crash at shutdown where the Application object is released early.
  • DestroyWindow is not getting called at shutdown for the all HWND (This have the effect of showing the window for a brief period after closing the app)
  • WindowsDesktopXamlSource.Close was not getting called at shutdown.

References

PR Checklist

  • Closes 872, 1085
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

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

Is there a visual difference when changing this? Like does it remove the pixel gap between the islands or the frame? Or is it just the drag area?

Can you MSPaint on a screenshot what the difference is so we can visualize it more easily?

src/cascadia/WindowsTerminal/AppHost.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
@miniksa miniksa added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels May 22, 2019
@miniksa miniksa added this to the Terminal v1.0 milestone May 22, 2019
ocalvo and others added 2 commits May 22, 2019 11:19
Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
src/cascadia/TerminalApp/App.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/App.h Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/App.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/App.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/App.idl Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/AppHost.cpp Outdated Show resolved Hide resolved
@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels May 30, 2019
@DHowett-MSFT
Copy link
Contributor

/azp run

1 similar comment
@DHowett-MSFT
Copy link
Contributor

/azp run

@microsoft microsoft deleted a comment from azure-pipelines bot May 31, 2019
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

I'm comfortable with this. I just need a second signoff to merge it.

src/cascadia/TerminalApp/App.xaml Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/BaseWindow.h Outdated Show resolved Hide resolved
src/cascadia/WindowsTerminal/pch.h Outdated Show resolved Hide resolved
@Poopooracoocoo
Copy link

kinda odd how the borders at the top are rounded. Very few apps have rounded borders on Windows 10. One is Math Input Panel.

@Stanzilla
Copy link
Contributor

They already said this will change in the future, Windows is getting rounded borders everywhere

@Poopooracoocoo
Copy link

Introducing rounded corners after years of persistent sharp corners is hard and we don't have rounded corners right now anyway. I got a little off topic as it's more of a topic for the winui repo

@DHowett-MSFT
Copy link
Contributor

The real technical reason this is happening is that we're suppressing window decorations and drawing over them, and the kernel component in charge of that (don't ask) gives us a region with rounded corners.
If we let the kernel do the painting, it looks like Windows Vista's Aero Basic.
Hence, the rounded corners.

mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 2, 2019
…ith a single Island (microsoft#929)

* Use a region to cut off the dragable region
* Use proper measurements for the draggable area
* Working better, paint works most of the time
* Fix a bug where paint is incomplete when double clicking the dragbar
* Remove old fork on XamlApplication
* Upgrade to XamlApp preview6.2
* Add Microsoft.VCRTForwarders to make it easy to dogfood

Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
Co-Authored-By: Mike Griese <migrie@microsoft.com>
mcpiroman pushed a commit to mcpiroman/terminal that referenced this pull request Jul 23, 2019
…ith a single Island (microsoft#929)

* Use a region to cut off the dragable region
* Use proper measurements for the draggable area
* Working better, paint works most of the time
* Fix a bug where paint is incomplete when double clicking the dragbar
* Remove old fork on XamlApplication
* Upgrade to XamlApp preview6.2
* Add Microsoft.VCRTForwarders to make it easy to dogfood

Co-Authored-By: Michael Niksa <miniksa@microsoft.com>
Co-Authored-By: Mike Griese <migrie@microsoft.com>
@ghost
Copy link

ghost commented Aug 3, 2019

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

Handy links:

@beviu beviu mentioned this pull request Oct 30, 2019
5 tasks
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 Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.