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

Fix fullscreen after maximize #3721

Merged
2 commits merged into from
Dec 16, 2019
Merged

Fix fullscreen after maximize #3721

2 commits merged into from
Dec 16, 2019

Conversation

beviu
Copy link
Contributor

@beviu beviu commented Nov 26, 2019

Summary of the Pull Request

Fixes the sides disappearing when entering full screen mode when the window is maximized.
However, now, a Vista-style frame briefly appears when entering/exiting full screen.

References

PR Checklist

  • Closes Entering Fullscreen from Maximized doesn't work right #3709
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Requires documentation to be updated (no)
  • 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

When the non-client island window is maximized and has the WS_OVERLAPPEDWINDOW style, SetWindowPos is "lying": the position ends up being offset compared to the one we gave it (found by debugging). So I changed it to use WS_POPUP like the client island window was already doing. But now it has the Vista frame that appears briefly when entering/exiting full screen like the client island window.

Validation Steps Performed

@zadjii-msft
Copy link
Member

@greg904 Is there any way to not get that vista-style window frame to appear for a brief second, and fix this bug?

Overall, I'm happy that this works better, but that frame felt really jarring in my own testing :/

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. labels Nov 26, 2019
@beviu
Copy link
Contributor Author

beviu commented Nov 27, 2019

I don't know how to do it. Chromium's code https://github.com/chromium/chromium/blob/master/ui/views/win/fullscreen_handler.cc is pretty much the same as here and yet Chromium doesn't have the issue. I'm missing something somewhere.

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.

Okay, I don't know if this is the right solution, but I was able to get the vista frame to hide itself when entering fullscreen if I hide the window near the top of IslandWindow::_SetIsFullscreen, and show it again at the bottom.

The vista titlebar is visible for a frame or so when leaving fullscreen, but idk how we fix that. @greg904 if you want to experiment with that, I can let you try your hand at it more. If not, that's fine too - I can submit my own PR once this is merged to fix it when entering. Whatever you prefer 😄

@beviu
Copy link
Contributor Author

beviu commented Dec 13, 2019

I don't really have time now unfortunately so I would prefer that you submit your own PR.

@zadjii-msft
Copy link
Member

@greg904 No problem. Thanks for the initial fix 😄

@zadjii-msft zadjii-msft added AutoMerge Marked for automatic merge by the bot when requirements are met Needs-Second It's a PR that needs another sign-off labels Dec 13, 2019
@ghost
Copy link

ghost commented Dec 13, 2019

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit abb2df1 into microsoft:master Dec 16, 2019
@ghost
Copy link

ghost commented Jan 14, 2020

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

Handy links:

@beviu beviu deleted the fix-maximize-fullscreen branch March 1, 2020 19:24
This pull request was closed.
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 Needs-Second It's a PR that needs another sign-off Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entering Fullscreen from Maximized doesn't work right
3 participants