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

feat: Center Window Preview According to Current Dock Icon #326

Merged
merged 5 commits into from
Dec 22, 2024

Conversation

chrisharper22
Copy link
Contributor

Calculate the window preview frame according to the hovered icon frame, attempting to replace #182 and update to modern codebase.

Describe your changes

[SharedWindowPreviewCoordinator] - calculate x- and y-positions for the preview frame from the currently hovered Dock icon.

Related issue number(s) and link(s)

Checklist before requesting a review

  • I have performed a self-review of my code
  • If this change affects core functionality, I have added a description highlighting the changes
  • I have followed conventional commit guidelines

Core Functionality Changes

If this PR modifies core functionality, please provide a brief description of the changes and their impact below:
N/A

I am leaving this as a draft currently, as I do not have a reliable way to test for multi-screen setups. If someone is able to test this, I would appreciate it.

Calculate the window preview frame according to the hovered icon frame
@chrisharper22
Copy link
Contributor Author

chrisharper22 commented Sep 16, 2024

Alright, so I've noticed an issue that occurs with multiple screens. When the main screen does not have the default screen resolution, the positioning of the preview will be calculated incorrectly on non-main screens. This is possibly related to #182 (comment)

@ejbills
Copy link
Owner

ejbills commented Sep 17, 2024

If someone is able to test this, I would appreciate it.

I can help, just ping me.

chrisharper22 and others added 2 commits September 22, 2024 09:21
- Use helper method from `DockObserver` (`cgPointFromNSPoint(_:forScreen:) to update from appropriate coordinate space
- Update calculated y values to vertically center icon
@chrisharper22
Copy link
Contributor Author

chrisharper22 commented Sep 22, 2024

I updated the code to use the helper methods in DockObserver (e.g., cgPointFromNSPoint(_:forScreen:) to calculate the position for the appropriate coordinate space. I also updated the calculations for yPosition and xPosition due to this coordinate flipping (this needs to be double checked); all accounting for the 3 dock positions.

I tested by using my iPad and secondary MacBook to extend my screen using AirPlay, and from my testing the hover previews are in the correct locations per hovered icon, correct distances from screen bounds/dock, and correct screens.

@ejbills if you can test these changes, I would appreciate it.

@ejbills
Copy link
Owner

ejbills commented Nov 29, 2024

@chrisharper22 this has been on the back-burner while I prioritized some other bugs. I will be taking a look at this this weekend! Apologies for the delay.

@ejbills ejbills marked this pull request as ready for review December 22, 2024 05:18
@ejbills
Copy link
Owner

ejbills commented Dec 22, 2024

@chrisharper22 - apologies for the delay once again, I have finally found some time to test this thoroughly and I fixed a small issue. Besides that, it appears to be working as expected. Thank you for your continued additions!

@ejbills ejbills merged commit c213579 into ejbills:main Dec 22, 2024
2 checks passed
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