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.

Sign up for GitHub

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

Jump to bottom

Fix macOS bug where tray window causes spaces to switch #2677

Merged
merged 1 commit into from
Dec 2, 2020
Merged

Fix macOS bug where tray window causes spaces to switch #2677

merged 1 commit into from
Dec 2, 2020

Conversation

briankendall
Copy link
Contributor

Closes #2332

Let me know if there's any changes you need to the PR and I'll be happy to update my commit

@briankendall briankendall force-pushed the fix-tray-window-spaces-bug branch 2 times, most recently from 2adeab5 to b5b4def Compare November 30, 2020 18:25
@er-vin
Copy link
Member

er-vin commented Dec 1, 2020

/rebase

@github-actions github-actions bot force-pushed the fix-tray-window-spaces-bug branch from b5b4def to a6b9621 Compare December 1, 2020 05:54
er-vin
er-vin requested changes Dec 1, 2020
View reviewed changes
src/gui/systray.h Outdated
@@ -29,6 +29,7 @@ namespace OCC {
#ifdef Q_OS_OSX
bool canOsXSendUserNotification();
void sendOsXUserNotification(const QString &title, const QString &message);
void setTrayWindowLevelAndVisibleOnAllSpaces(QQuickWindow *qtWindow);
Copy link
Member

Choose a reason for hiding this comment

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

I'd rename the parameter from qtWindow to window. Also seeing how it's used please change the type to QWindow.

src/gui/systray.mm Outdated
@@ -1,4 +1,5 @@
#include <QString>
#include <QQuickWindow>
Copy link
Member

Choose a reason for hiding this comment

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

Please include QWindow instead.

src/gui/systray.mm Outdated
@@ -41,4 +42,13 @@ void sendOsXUserNotification(const QString &title, const QString &message)
[notification release];
}

void setTrayWindowLevelAndVisibleOnAllSpaces(QQuickWindow *qtWindow)
Copy link
Member

Choose a reason for hiding this comment

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

Rename the parameter and change its type please.

src/gui/systray.mm Outdated
void setTrayWindowLevelAndVisibleOnAllSpaces(QQuickWindow *qtWindow)
{
NSView *view = (NSView *)qtWindow->winId();
NSPanel *window = (NSPanel *)[view window];
Copy link
Member

Choose a reason for hiding this comment

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

Please change window here to nativeWindow.

@briankendall briankendall force-pushed the fix-tray-window-spaces-bug branch from a6b9621 to 9fef8b4 Compare December 1, 2020 16:49
@briankendall
Copy link
Contributor Author

Okay I made the requested changes. (And fixed my usage of NSPanel instead of NSWindow which wasn't technically correct.) Let me know if there's anything else you'd like me to do.

@er-vin
Copy link
Member

er-vin commented Dec 2, 2020

/rebase

@github-actions github-actions bot force-pushed the fix-tray-window-spaces-bug branch from 9fef8b4 to f9991e1 Compare December 2, 2020 06:54
@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-2677-f9991e180ca1cb7d88d773e61c1fe914f31d1791-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@er-vin er-vin merged commit 7721832 into nextcloud:master Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@er-vin er-vin er-vin approved these changes

@allexzander allexzander Awaiting requested review from allexzander

@camilasan camilasan Awaiting requested review from camilasan

@DominiqueFuchs DominiqueFuchs Awaiting requested review from DominiqueFuchs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Client 3.0 on the Mac causes desktop/space switch
3 participants
@briankendall @er-vin @nextcloud-desktop-bot