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

Improved monitor ordering #1

Merged
merged 8 commits into from
Mar 24, 2020

Conversation

ivan100sic
Copy link
Collaborator

Summary of the Pull Request

Here's my part of the code, take a look at it, then squash it and add it to your branch.

References

PR Checklist

  • Closes #xxx
  • 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

Validation Steps Performed


TEST_METHOD(TestMonitorOrdering08)
{
// AABBCC
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can do something like this 😃

------------------
|    ||    ||    |
|    ||    ||    |
------------------
|       ||       |
|       ||       |
------------------

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, why not!

Copy link
Owner

@vldmr11080 vldmr11080 left a comment

Choose a reason for hiding this comment

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

Look awesome to me! I could catch logic behind main part quite quickly, comments are really important here. Not sure if we should ask someone else to check, or You can merge to move-window-arrow-key-imp and continue there.

@ivan100sic
Copy link
Collaborator Author

We can ask 2 more people to look at this code and then merge this first and then the main PR.

@ivan100sic ivan100sic merged commit 8564b56 into move-window-arrow-key-impr Mar 24, 2020
vldmr11080 added a commit that referenced this pull request Mar 25, 2020
* When moving window into zones using arrow keys, support multi-monitor scenario

* Minor coding style adjustments

* Split implementation into separate functions because of readability

* Rename certain arguments

* Modify unit tests after API changes

* Address PR comments and add unit tests

* Return true from MoveWindowIntoZoneByDirection only if window is successfully added to new zone

* Improved monitor ordering (#1)

* Implemented improved monitor ordering v1

* Fixed some embarrassing bugs, added some tests

* Added one more test

* Extracted a value to a variable

* ASCII art in unit test comments describing monitor layouts

* Removed empty line for consistency

* Update comment to match the code

* Refactored tests, added tests for X,Y offsets

Co-authored-by: Ivan Stošić <ivan100sic@gmail.com>
vldmr11080 pushed a commit that referenced this pull request Nov 10, 2020
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