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 monitor sorting with mix of portrait and landscape layout. #150

Merged
merged 2 commits into from
Oct 21, 2015

Conversation

neeasade
Copy link
Contributor

This PR changes monitor sorting to keep sorting by x and y coordinates, but also takes into account a height comparison that I believe fixes sorting in currently unexpected monitor situations. This is done in a way that keeps current monitor sorting the same on setups with the y offset aligned across the top, so impact should be minimal.

This includes #118

Notes for how I got here and screenshots along the way are attached to #147 (It's unorganized as I commented as I went along) - this was tested in 3 different monitor layouts with different geometry spawns, as seen there.

@LemonBoy
Copy link
Owner

Looks fine to me and Travis, I didn't double check everything but the = is redundant in one of the two comparisons.

@neeasade
Copy link
Contributor Author

Ah, you're right - I believe it's the latter, but will test when home today to be safe.

@neeasade
Copy link
Contributor Author

Removed second =, retested with the three monitor layouts, seems okay to me. Used --amend then repushed.

LemonBoy added a commit that referenced this pull request Oct 21, 2015
Fix monitor sorting with mix of portrait and landscape layout.
@LemonBoy LemonBoy merged commit 1585d72 into LemonBoy:master Oct 21, 2015
@LemonBoy
Copy link
Owner

Aaand merged! Thank you for the PR :)

@neeasade neeasade mentioned this pull request Oct 21, 2015
@neeasade
Copy link
Contributor Author

np, thank you for the flexible program. :)

@neeasade neeasade deleted the monitor_sort branch November 9, 2015 19:23
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.

2 participants