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 positioning on multiple screens #35

Merged
merged 2 commits into from
Mar 26, 2022
Merged

Conversation

guillaumeboehm
Copy link
Contributor

The change to browser.py should fix issues on multiple screens such as in issue #29.

I also added a cursor resize to the Xgreeter which seems reasonable to me, but I guess you can remove it if you think it shouldn't be here.

@JezerM JezerM linked an issue Mar 25, 2022 that may be closed by this pull request
Copy link
Owner

@JezerM JezerM left a comment

Choose a reason for hiding this comment

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

Looks nice! I didn't know about QGuiApplication existence and that it's a superclass of QApplication. So, Application.desktop property could be removed to use QGuiApplication methods instead. Also, this could even make possible a multi-monitor support in a future~

Removed the cursor size export

Co-authored-by: Jezer Mejía <[email protected]>
@guillaumeboehm
Copy link
Contributor Author

I'm not sure what you mean by multiple monitor support If it's not what I added in browser.py? So I searched for the use of Application.desktop in the code and couldn't find any, I would be happy to work on this a little bit if it's in my capabilities. Could you detail quickly what you mean ?

@guillaumeboehm guillaumeboehm requested a review from JezerM March 26, 2022 15:38
@JezerM
Copy link
Owner

JezerM commented Mar 26, 2022

What this PR does is to set the primary window in the primary screen/display, which should fix the #29 issue by setting web-greeter's window in the main monitor. However, this is not a multi-monitor support, as the rest of monitors would still be blank without any window.

By Multi-Monitor support I mean something like JezerM/nody-greeter#11 does, which creates multiple IPC-interconnected windows for each monitor, and allows to set a primary and secondary HTML files whether the monitor is primary or not with a index.yml configuration inside the theme directory.

The Application.desktop property, a QDesktopWidget, was previously used to get the screen properties (like geometry), which is now unused with this PR. Also, the PyQt5 documentation declares most of its methods as deprecated, so I was just saying that this property could be removed~

desktop: QDesktopWidget

Copy link
Owner

@JezerM JezerM left a comment

Choose a reason for hiding this comment

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

However, if this PR is just meant to fix the positioning on multiple screens, this should be done~

@guillaumeboehm
Copy link
Contributor Author

I see! Well that's interesting, I think I'll try to look into multi-monitor support then. Not sure I'll get somewhere but I'll create a PR if I manage something!

@JezerM
Copy link
Owner

JezerM commented Mar 26, 2022

Nice~ Then, this PR will be merged. Thanks!

@JezerM JezerM merged commit bc7a4b6 into JezerM:master Mar 26, 2022
@JezerM JezerM mentioned this pull request Mar 26, 2022
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.

Adding active-monitor
2 participants