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

Background blur #316

Merged
merged 11 commits into from May 16, 2022
Merged

Background blur #316

merged 11 commits into from May 16, 2022

Conversation

bartreardon
Copy link
Contributor

Adds a background layer that blurs the entire screen behind the main nudge window
Also made nudge (and the blurred screen) persist across virtual desktops.

Needs more work to make it prod ready and trigger the logic that displays the blurred screen as well as remove the blur on command.

Call `viewObserved.bluredBackground?.close()` when clicking the "update device" button. this will make the background blur go away so the user can see the update window
@bartreardon
Copy link
Contributor Author

@erikng added in the command to un-blur the window on clicking "update device". Only thing to consider now is what conditions to initiate the blur and set a variable.

Set the condition to display the blur when `pastRequiredInstallationDate` is true.
@bartreardon
Copy link
Contributor Author

made the blur appear after pastRequiredInstallationDate is true. This could be predicated on some other condition but I think this makes sense logically. WDYT?

@bartreardon
Copy link
Contributor Author

as per the discussing in the macadmins slack, thoughts on having the Nudge window persist across all virtual desktops as a default setting vs only in mega nag mode. Personally I think it's fine to have it be on all screens. The point is, of course, to get people to upgrade sooner rather than later. Open to counter arguments though.

@erikng
Copy link
Member

erikng commented Feb 22, 2022

So the main question I have is there not a way to do this all natively in swiftui? I've stayed away from storyboards entirely for this project and it seems somewhat of a step back to now have a mix of swiftui and "legacy" ui storyboards. I use legacy in quotes, because it's definitely not legacy, but seems like eventually it might be.

@groob
Copy link
Collaborator

groob commented Feb 23, 2022

I think you should be able to create an NSWindow without the storyboard.

@bartreardon
Copy link
Contributor Author

the thing I found about pure swiftui and blur is it only applies when the window is active. So if you create a window and apply transparency, all that goes away when it's not active, thus your background is now just a solid shade of window.

Creating a window with the desired properties without the storyboard should be doable. If you agree that swift and swiftui run congruent then it will keep it all neat. (and I agree with the regression statement. While storyboards are supported, so is interface builder - from a generational point of view IB -> SB -> SUI).

The storyboard is what I have and I know worked. If you agree with the concept then I can massage it (it benefits me to do so).

@bartreardon
Copy link
Contributor Author

ok - just commenting to say I have a solution that avoids the need for a storyboard. Will update my PR.

@erikng
Copy link
Member

erikng commented Feb 24, 2022

so we have a git conflict (which isn't a big problem, but I think a bigger issue.

I was playing with window?.collectionBehavior = [.canJoinAllSpaces] to fix the full screen bug reported on slack and when I was using this, it made everything horrible for nudge behavior. :/

If you pull the latest development code and uncomment L72 on ContentView.swift you can see the behavior for yourself.

  1. Open xcode not in full screen mode, run the code (in -demo-mode scheme) with the line uncommented. Nudge will not open in the foreground.
  2. Put Xcode in full screen mode and watch the bad behavior.

@erikng
Copy link
Member

erikng commented Feb 24, 2022

I've fixed the git conflict for you as well.

@bartreardon
Copy link
Contributor Author

I'm not sure what I'm looking for. I get the same behaviour whether window?.collectionBehavior = [.canJoinAllSpaces] is commented or not. with xcode fullscreened nudge runs but never appears.

@bartreardon
Copy link
Contributor Author

There was a conflict when I did my last commit but I thought eb3edcf fixed that up (wasn't complaining on my end anyway, unless I was missing some upstream changes in between then and now)

@erikng
Copy link
Member

erikng commented Feb 24, 2022

Weird. What behavior do you see?

@bartreardon
Copy link
Contributor Author

If the active screen is a full-screened app then nudge never shows. Im also running dual screens though so there is a screen with nothing fullscreened on it. Will disconnect and try with just one screen and see if i get different behaviour. I wonder if this isn’t a quirk of the window manager in macos making decisions about what to do.

@bartreardon
Copy link
Contributor Author

ok - I let it run for a bit. when fullscreened, nothing happens, if I leave that screen and go so something else, nudge eventually appears, but is then persistent on all screens (including the fullscreen app). Is this not what you want nudge to do?

saying all this, as canJoinAllSpaces is new behaviour, leaving it out for now if it's causing problems would not break any existing behaviour.

@erikng
Copy link
Member

erikng commented Feb 24, 2022

I may have confused you. Don't try the behavior on this PR but straight from dev branch with just the line change

you might also have to change one other line because I had reversed its logic when testing this.

https://github.com/macadmins/nudge/blob/development/Nudge/Preferences/DefaultPreferencesNudge.swift#L86 Comment this out.

@erikng
Copy link
Member

erikng commented Feb 24, 2022

For your question though, we need to dynamically change that property if you see the behavior I saw and only enable that part of the window properties when in this mode.

@bartreardon
Copy link
Contributor Author

I may have confused you. Don't try the behavior on this PR but straight from dev branch with just the line change

you might also have to change one other line because I had reversed its logic when testing this.

https://github.com/macadmins/nudge/blob/development/Nudge/Preferences/DefaultPreferencesNudge.swift#L86 Comment this out.

ah, gotcha

@bartreardon
Copy link
Contributor Author

Still not seeing any changed/unexpected behaviour between having canJoinAllSpaces on or not other than nudge being available on all screens. commenting out DefaultPreferencesNudge.swift#L86 makes nudge appear over the fullscreen xcode. I'm still not sure what behaviour I'm looking for (or if I'm seeing it and not recognising it as the issue)

@erikng
Copy link
Member

erikng commented Feb 25, 2022

The issue I saw is that when you go back to a full screen app, nudge doesn't go away and continues to be front and center. That doesn't happen when you're not full screen.

@bartreardon
Copy link
Contributor Author

right - in that case, yes I saw that as well.

@erikng
Copy link
Member

erikng commented Feb 25, 2022

Seems to only happen when that key is set :/

@bartreardon
Copy link
Contributor Author

running the demo-mode scheme I don't see nudge go to the background on a fullscreen app. If I launch while xcode is fullscreen then nudge is front and centre regardless if canJoinAllSpaces is active or not. When not fullscreen nudge will sit behind windows and with canJoinAllSpaces enabled will pop front and centre when entering fullscreen.

Will be one of two things then, 1 - the layering of fullscreen apps is not expected. macOS could be doing something weird with fullscreen apps and setting the z-layer priority as if to make the app the background layer or something or 2 - something odd when honouring canJoinAllSpaces that keeps the window in the foreground.

either way, whether canJoinAllSpaces is in there or not, if nudge appears with a fullscreen app open it will be in the forground.

@bartreardon
Copy link
Contributor Author

that's what I'm seeing here anyway

@bartreardon
Copy link
Contributor Author

xcode 13.2.1 and macos 12.2.1 btw

@erikng
Copy link
Member

erikng commented Feb 25, 2022

I spent like 8 hours on this so I'm hoping you're wrong when I test this again tomorrow.

Also on 13.2.1/12.2.1

and yeah macOS does really dumb things that I've had to work around

bb5c752

this only accounted for the first launch, so you might be seeing this on subsequent things, which is not ideal but may be unavoidable. You can see all the NSApp things I tried and all essentially failed.

@bartreardon
Copy link
Contributor Author

hmm - also weird things happen if xcode is fullscreen but not on the primary display, nudge will appear on the primary display instead of the active display. leads me to believe that macos fullscreen+spaces shenanigans are at play.

@erikng
Copy link
Member

erikng commented Feb 25, 2022

Yeah it all sucks lol. I tried googling for quite some time to force an app to the "desktop" space rather than the current active space, but could never find anything like that so I just hacked together the NSApp.hide() but I'm not the biggest fan of it because Nudge can't be selected by the user until the first activation from the timer controller.

@bartreardon
Copy link
Contributor Author

Where did you want to go with this one? The window behaviour seems to be independent of the blur effect or being present on all displays and is just how macos treats popover windows on full-screened apps. Optional is to perhaps see if we can do a test to see if NSScreen.main is a fullscreened app and not launch nudge (no idea if that's possible at this stage if the info is in the main properties). Then when the user eventually moves to another virtual desktop then nudge should pop up.

@erikng
Copy link
Member

erikng commented Mar 12, 2022

Once I cut 1.1.6 I'll start looking at this PR. Probably should cut the release next week (I've been too busy to cut one last week and a half)

@erikng erikng changed the base branch from development to dev-blur May 16, 2022 16:37
@erikng
Copy link
Member

erikng commented May 16, 2022

I'm going to merge this into a dev-blur branch and play around with it, then merge it into the real development branch if I like it. Sorry this took so long.

@erikng erikng merged commit 15f5951 into macadmins:dev-blur May 16, 2022
@bartreardon
Copy link
Contributor Author

All good my friend. 🙂

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.

None yet

3 participants