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

Rejig notifications to work with layer-shell positioning #656

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

fossfreedom
Copy link
Contributor

Description

This PR changes from absolute positioning of windows (aka X11) to layershell positioning

Submitter Checklist

  • Squashed commits with git rebase -i (if needed)
  • Built budgie-desktop and verified that the patch worked (if needed)

@EbonJaeger
Copy link
Member

Could we use anchors to set the corner of the screen selected in Budgie Desktop Settings?

@fossfreedom
Copy link
Contributor Author

@EbonJaeger hopefully the last commit does this. Needs proper testing rather than just "notify-send" stuff that I have been using.

@EbonJaeger
Copy link
Member

Using a margin for y-axis positioning is really clever; I like it. In the calculate position function, do we still have switch cases for left and right? If so, it could likely be further simplified, since we only care about the y-axis. Left side or right side doesn't matter anymore.

@fossfreedom
Copy link
Contributor Author

Using a margin for y-axis positioning is really clever; I like it. In the calculate position function, do we still have switch cases for left and right? If so, it could likely be further simplified, since we only care about the y-axis. Left side or right side doesn't matter anymore.

indeed - latest commit does this.

@EbonJaeger
Copy link
Member

Since the calculate function only has to return one value now, should we make the return type int and remove the out parameter variable?

@fossfreedom
Copy link
Contributor Author

Definitely down to the bare essentials in the latest commit :)

Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

I love seeing that function gutted 😃

It appears to work great for me; I sent some notifications with different position settings, and it just works. No more notifications in the middle of the screen!

Copy link
Member

@JoshStrobl JoshStrobl left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@fossfreedom fossfreedom merged commit 2cdcbd9 into main Jan 22, 2025
1 check passed
@fossfreedom fossfreedom deleted the notifications branch January 22, 2025 22:10
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.

3 participants