Skip to content

fix: Window opening on dual screen #1183 #1213

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

Closed
wants to merge 5 commits into from

Conversation

LinuxHeki
Copy link

I have no idea what I changed, but now is working perfectly!

@LinuxHeki LinuxHeki changed the title Fix for "Window opening on dual screen" #1183 fix: Window opening on dual screen #1183 Oct 14, 2021
@leviport leviport requested review from a team October 14, 2021 14:25
jackpot51
jackpot51 previously approved these changes Oct 14, 2021
@jackpot51
Copy link
Member

It looks reasonable to me. @mmstick what do you think?

@jackpot51 jackpot51 requested a review from mmstick October 14, 2021 14:29
@jackpot51
Copy link
Member

I would recommend squashing this into one commit, though

@LinuxHeki
Copy link
Author

How to squash this into one commit?

@LinuxHeki
Copy link
Author

Do I need to delete repo, fork this repo and make changes in one commit?

@mmstick
Copy link
Member

mmstick commented Oct 14, 2021

git reset --soft 130ac4f
git commit --amend

As long as QA is okay with the window attaching to the display that the mouse is on, regardless of which window is actively focused. Because GNOME always opens the window on the display where the mouse is.

@LinuxHeki
Copy link
Author

LinuxHeki commented Oct 14, 2021

Is this what I needed to do?

@LinuxHeki
Copy link
Author

Or do I need to do this with every commit?

@mmstick
Copy link
Member

mmstick commented Oct 14, 2021

Now you have more commits rather than less

@LinuxHeki
Copy link
Author

Sorry

@LinuxHeki
Copy link
Author

So what do I need to do? More precisely please. I'm 14.

@isantop
Copy link
Contributor

isantop commented Oct 14, 2021

I believe you can get what is needed with the following:

git fetch
git reset --soft origin/master
git add .
git commit --amend
git push -f

@LinuxHeki
Copy link
Author

LinuxHeki commented Oct 14, 2021

After git add . I typed git status and nothing shows up. Is this right?

@isantop
Copy link
Contributor

isantop commented Oct 14, 2021

Nope, you should be seeing the files you modified. What are your remotes looking like?

git remote -v

@LinuxHeki
Copy link
Author

origin https://github.com/LinuxHeki/shell (fetch) origin https://github.com/LinuxHeki/shell (push) upstream https://github.com/pop-os/shell.git (fetch) upstream https://github.com/pop-os/shell.git (push)

@isantop
Copy link
Contributor

isantop commented Oct 14, 2021

I see the issue. You need to do as follows:

git fetch upstream
git reset --soft upstream/master
git add .
git commit --amend
git push -f -u origin master

Spotify Flatpak returns `null` for `meta.get_title()`. Use window name instead.
@LinuxHeki
Copy link
Author

Why are in the commit changes that I didn't do?

@isantop
Copy link
Contributor

isantop commented Oct 14, 2021

It looks like those are the changes to our local master branch which you had previously merged in as a merge commit. It shouldn't be a problem, provided the Engineering review still checks out.

@jacobgkau
Copy link
Member

The description for the commit is talking about the Flatpak fix and not what this commit's actually doing.

@jacobgkau
Copy link
Member

I squashed the commits from earlier into the correct starting commit and force-pushed. The metadata should be correct now, will test soon.

@LinuxHeki
Copy link
Author

So, that's it?

@jacobgkau
Copy link
Member

jacobgkau commented Oct 14, 2021

After building this locally to test, this doesn't seem to fix #1183:

simplescreenrecorder-2021-10-14_15.34.02.mp4

At first, I thought it was because of the mouse position behavior that @mmstick described, but I also recreated it where the window gets fullscreened onto a display the mouse isn't on:

simplescreenrecorder-2021-10-14_15.38.54.mp4

@LinuxHeki
Copy link
Author

LinuxHeki commented Oct 15, 2021

Is working, except if there are more than two windows on second display and zero on main, then Minecraft 1.17 moves to main display. Minecraft 1.8 moves to main display every single time.

Fix is not perfect but it's better than nothing (for me).

How are your videos long 30s and less than 10MB and mine 15s and 15MB?

@LinuxHeki
Copy link
Author

Now should work with Minecraft 1.17 but not with Minecraft 1.8

@LinuxHeki LinuxHeki force-pushed the master branch 2 times, most recently from 3a520dc to ee60b64 Compare October 15, 2021 10:51
@jacobgkau
Copy link
Member

Now should work with Minecraft 1.17 but not with Minecraft 1.8

I'm still not seeing any change in behavior, still testing with Minecraft 1.17 since it's the latest version:

Screenshot from 2021-10-15 13-15-44

The window and cursor were both on the primary display, then Minecraft full-screened itself to the external display. We're wanting it to full-screen onto the same display it's currently on, correct?

@LinuxHeki
Copy link
Author

If you press F11 three times is working...

@mmstick
Copy link
Member

mmstick commented Oct 22, 2021

The only change this makes is reverting 49aba22

@LinuxHeki
Copy link
Author

Yes...

@mmstick
Copy link
Member

mmstick commented Oct 26, 2021

Commit was reverted, so we can close this

@mmstick mmstick closed this Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants