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

Invert calculation for the "Eights" in portrait mode #1541

Closed
wants to merge 7 commits into from

Conversation

joaquin-splunk
Copy link

When I was configuring my screen that's on portrait mode with the Eights, the only one that fit my expectation was when it was on the Bottom Left corner of my screen. That's when I noticed on the code for Rectangle/WindowCalculation/BottomLeftEighthCalculation.swift the divisors are switched for the height and the width, while on the other calculations for the Eights are equal for both landscape and portrait mode.

This PR just switches them for the portrait mode for the other calculations.

@joaquin-splunk
Copy link
Author

This is in relation to #1540

@rxhanson
Copy link
Owner

Thanks for contributing. I tested this out real quick and it looked like the eighths that go in the center spaces will have to get their origin coordinates adjusted as well?

Copy link
Owner

@rxhanson rxhanson left a comment

Choose a reason for hiding this comment

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

Figured I'd check back in on this. As noted, it looked to me that we would also need the origins of each rectangle adjusted here as well. I don't mind fixing this, but didn't want to step on any toes if you were still looking to fix it.

@joaquin-splunk
Copy link
Author

Thanks for checking in. I haven't had the time yet to look into the issue. I might take a look during this weekend or next weekend, unless you get it before then.

@rxhanson
Copy link
Owner

rxhanson commented Feb 3, 2025

I went ahead and fixed the calculations, and this will go in the next release.

@rxhanson rxhanson closed this Feb 3, 2025
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