Skip to content
This repository has been archived by the owner on Nov 15, 2024. It is now read-only.

WIP: Enable being above fullscreen #17

Conversation

dignite
Copy link
Contributor

@dignite dignite commented Nov 3, 2018

Discussion in #11

Why?

In order to be able to develop in fullscreen mode on Mac OS
And still see the mob timer and be interrupted by the fullscreen window to switch mobber
We want the fullscreen window to open on top of the fullscreen display or interrupt it by switching the active desktop

How?

  • Hide dock on window creation, which allows windows to float above/interrupt other fullscreen windows.
  • Set alwaysOnTop-level to screen-saver for the fullscreen window to be on top of everything else.

Tests

  • Single screen Mac OSX Mojave
  • Single screen Windows 10
  • Multiple connected screens

@allan-stewart
Copy link
Contributor

allan-stewart commented Nov 4, 2018

This looks good to me from just a brief code review. So if it solves Travis' issue from #11 then I'm good to merge this (once we get confirmation).

Also, thanks for digging into this!

@TravisBumgarner
Copy link
Contributor

Some Notes. I can try and peak later in to the code and give more feedback. Thanks for working on this!

  • If the mob timer has focus, you can't three finger scroll sideways. Perhaps a better indicator of the mob timer besides a drop shadow?
  • Can't open settings menu in full screen
  • The mob timer isn't always visible when in full screen.

@allan-stewart
Copy link
Contributor

I played around with this today too and found some problems. The kiosk mode locks you into the mob timer window, even after you start a new turn (thus dismissing the full-screen window). When the timer has focus (and it retains focus even after starting the turn) then you can't Command+Tab between apps, nor Control+Arrow between spaces/full-screen apps.

The kiosk mode also seems to cause the timer window (the small one) to hover above the current full screen app you have focus on after the full-screen timer goes off. This makes it harder to know where the small window went and find it again.

The solution seems to be to remove the kiosk mode. Doing that made it work much better for me, and the full-screen window still showed up over the full screen app (though there was a gap for the non-visible menubar in the bounds, but that seems minor).

@dignite
Copy link
Contributor Author

dignite commented Nov 12, 2018

Great input! I did indeed add kiosk to avoid the gap, we can live with the gap. I'll remove it.

@@ -10,7 +11,7 @@ exports.createTimerWindow = () => {
}

let {width, height} = electron.screen.getPrimaryDisplay().workAreaSize
timerWindow = new electron.BrowserWindow({
timerWindow = createBrowserWindowOnCurrentScreen({
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like calling the helper method here does anything significant. I can't seem to get the small timer window to be above full-screen apps. And if it did launch in a full screen space then it would follow that app around which seems a bit odd to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have access to my mac but when I do I will look into this. Since it was a while since I did the changes I have a hard time remembering which tests I performed.

@dignite
Copy link
Contributor Author

dignite commented Nov 12, 2018

Some Notes. I can try and peak later in to the code and give more feedback. Thanks for working on this!

Thanks for the feedback @TravisBumgarner!

  • If the mob timer has focus, you can't three finger scroll sideways. Perhaps a better indicator of the mob timer besides a drop shadow?

I think this is good feedback and an improvement to this could be made independently of this PR, I won't look into that in this PR.

  • Can't open settings menu in full screen

I'll have a look, added it as a "Todo" in the description here so I won't forget

  • The mob timer isn't always visible when in full screen.

Can you provide some more information on this? Steps to reproduce? Any pattern on when it occurs?

@TravisBumgarner
Copy link
Contributor

Getting this bug. As far as your question I can't recreate it. 🤷‍♂️

screen shot 2018-11-12 at 2 20 15 pm

@dignite
Copy link
Contributor Author

dignite commented Nov 13, 2018

Getting this bug. As far as your question I can't recreate it. 🤷‍♂️

screen shot 2018-11-12 at 2 20 15 pm

Yeah my bad that issue has been fixed.

I am having some success with the app being on top of fullscreen apps but it is far from perfect.

With npm start with one fullscreen app:

  • When launching the app from a fullscreen terminal it appears on top of that terminal but as soon as you drag the timer window simply appears on desktop 1 and focuses desktop 1, not possible to get it on top of a fullscreen app again.
  • The fullscreen window does appear above the current desktop, even if it is fullscreen.
  • I am able to open settings above the fullscreen app, when clicking "Configure" on the fullscreen window.

With npm start with two fullscreen apps:

  • When launching the app from a fullscreen terminal sharing the fullscreen with another app the timer window simply appears on desktop 1 and focuses desktop 1, not possible to get it on top of a fullscreen app again.
  • The fullscreen window does appear above the current desktop, but takes you to the desktop.
  • Since you get moved to Desktop, opening settings is no problem, when clicking "Configure" on the fullscreen window.

With npm run build-mac and then start with either one or two fullscreen apps (using Alfred):

  • When launching the app from a fullscreen terminal sharing the fullscreen with another app the timer window simply appears on desktop 1 and focuses desktop 1, but when you swap between your desktops/fullscreens it re-appears in the same spot. If dragged, then it goes back to Desktop 1 with your focus.
  • The fullscreen window does appear above the current desktop, but takes you to the desktop if it was a "two fullscreen apps"-window.
  • If you get moved to Desktop, opening settings is no problem, when clicking "Configure" on the fullscreen window. If you stay above a fullscreen window it is also no problem.

@dignite
Copy link
Contributor Author

dignite commented Nov 14, 2018

One option could be to focus only on letting the fullscreen mob timer window interrupt fullscreen apps and not touch timer window or settings window

@allan-stewart
Copy link
Contributor

I think that's probably the best option -- just have the fullscreen window interrupt fullscreen apps.

The common use case for the mob timer is that the users are not in fullscreen mode (at least I assume that, based on the usages I know about directly and the feedback we've had thus far). The small timer window is designed to float over other windows so you always see it. But that's kind of the opposite of using an app in fullscreen mode; when you go fullscreen on a Mac it's all about focusing on one single application. And it interacts with the Spaces virtual screen stuff which makes things more complicated (especially if there are multiple monitors).

So my gut feeling is that we should avoid getting too complicated in the fullscreen case. If the "fullscreen" timer window can interrupt fullscreen usage, that's great. Users who want fullscreen code editors (for example) probably don't want the small timer floating around on them. But they will want to know when the time is up (otherwise why are they using the timer?) so let's just cover that case.

@dignite
Copy link
Contributor Author

dignite commented Nov 17, 2018

I have rolled back as much as possible in this PR while still solving the main obstacle: Interrupt fullscreen apps on mac

I am not sure if there is any downside to using bounds instead of workAreaSize (should cover menu bars/start menus) but I think we can tackle that change separately!

I am unable to test this on Mac yet since that laptop was left at my workplace over the weekend.

@dignite dignite force-pushed the pluralsight-mob-timer-interrupt-fullscreen branch from 9bfde22 to 49e4e31 Compare November 17, 2018 11:03
@allan-stewart
Copy link
Contributor

I just tested your latest changes on my Mac and I think it's working great per the discussion above. So I'm going to merge it and we can cover any further changes in a separate PR (if needed).

@allan-stewart allan-stewart merged commit 5b239f3 into pluralsight:master Nov 19, 2018
@dignite
Copy link
Contributor Author

dignite commented Nov 19, 2018

Great, thanks

@dignite dignite deleted the pluralsight-mob-timer-interrupt-fullscreen branch November 27, 2018 21:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants