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

When use the block mode, not all workspaces are blocked in Mac OS X #256

Closed
1 task done
mhewedy opened this issue Jun 26, 2018 · 29 comments · Fixed by #306
Closed
1 task done

When use the block mode, not all workspaces are blocked in Mac OS X #256

mhewedy opened this issue Jun 26, 2018 · 29 comments · Fixed by #306

Comments

@mhewedy
Copy link

mhewedy commented Jun 26, 2018

Prerequisites

  • I'm using latest version: v installed using homebrew

Description

[Description of the issue]

Operating system: Mac OS X

Steps to Reproduce

  1. Change the setting to "Breaks are fullscreen" to on
  2. Wait for the break to come

Expected behavior: Block happened on all workspaces

Actual behavior: Block happened on single workspace which make it easy to skip the break and continue working

Reproduces how often: alaways

Additional Information

Any additional information, configuration or data that might be necessary to reproduce the issue.

@hovancik
Copy link
Owner

Hi, on my mac 10.13.2 and one external monitor it works fine and I see break window on all screens.

How many external monitors do you have?
Do you have option "breaks are show on all screens" checked? It's on last settings page.

screen shot 2018-06-26 at 12 59 02

@mhewedy
Copy link
Author

mhewedy commented Jun 26, 2018

Yes, I have this option "breaks are shown on all screens" checked. I think it comes checked by default.

Note, I am not talking about all monitors, I am talking about all workspaces within my mac book bro monitor. ( I am not attaching external monitor in this case)

@hovancik
Copy link
Owner

Gotcha. I have found API for this so I will take a look whether it works.

https://github.com/electron/electron/blob/master/docs/api/browser-window.md#winsetvisibleonallworkspacesvisible

@mhewedy
Copy link
Author

mhewedy commented Jun 26, 2018

Note, the same function works fine on windows 10

This is what I mean by workspace (but since I don't have mac in my work, I screenshotted my windows workspaces)

stretchly

@mhewedy
Copy link
Author

mhewedy commented Jun 26, 2018

@hovancik , it is working as expected on windows, the issue only occurs on mac. Shall I open this issue on electron instead?

@hovancik
Copy link
Owner

@mhewedy The API I mentioned is not used right now in stretchly, so no need to open issue with electron, yet. I will test turning it on and we shall see. Electron's docs don't mention whether it's on or off by default.

@mhewedy
Copy link
Author

mhewedy commented Jun 26, 2018

@hovancik I tried the function you mentioned but didn't manage to get the issue solved.

@hovancik
Copy link
Owner

how did you do it? I haven't had time yet...

@mhewedy
Copy link
Author

mhewedy commented Jun 27, 2018

I called the method setVisibleOnAllWorkspaces(true) in two places,

And in both cases, the issue still exists.

@hovancik
Copy link
Owner

I guess if you would like to dig deeper you could try trying the api call from this repo:

https://github.com/electron/electron-quick-start

If this does not work there, then it is electron bug and you could report it. If it works then we probably need to call it in another place or some another stretchly code is interfering with it.

I've already did few reports like this (https://github.com/hovancik/stretchly#known-issues)

@mhewedy
Copy link
Author

mhewedy commented Jun 28, 2018

It seems we have a complex issue:

Call to BrowserWindow.setFullScreen should be avoided, because it sends stretchly to a new workspace itself (i.e. as if you click the mac maximize button without click on the alt button). Instead we can resize the BrowserWindow to fill the screen.

Still One issue remaining is to hide mac menubar and dockbar

here's diffs

Index: app/main.js
<+>UTF-8
===================================================================
--- app/main.js	(revision c9d960eeb801921820ad36435717e7c4cc204936)
+++ app/main.js	(date 1530212152000)
@@ -302,9 +302,11 @@
   if (settings.get('ideas')) {
     idea = breakIdeas.randomElement
   }
+  const electron = require('electron');
 
   for (let displayIdx = 0; displayIdx < numberOfDisplays(); displayIdx++) {
-    let breakWinLocal = new BrowserWindow({
+
+    const options = {
       icon: `${__dirname}/images/stretchly_18x18.png`,
       x: displaysX(displayIdx),
       y: displaysY(displayIdx),
@@ -314,17 +316,26 @@
       skipTaskbar: true,
       focusable: false,
       title: 'stretchly'
-    })
+    }
+
+    if (settings.get('fullscreen')) {
+      const displaySize = electron.screen.getAllDisplays()[displayIdx].size
+      options.width = displaySize.width
+      options.height = displaySize.height
+    }
+
+    let breakWinLocal = new BrowserWindow(options)
+
     // breakWinLocal.webContents.openDevTools()
     breakWinLocal.once('ready-to-show', () => {
       breakWinLocal.show()
-      breakWinLocal.setFullScreen(settings.get('fullscreen'))
       if (displayIdx === 0) {
         breakPlanner.emit('breakStarted', true)
       }
       breakWinLocal.webContents.send('breakIdea', idea, settings.get('breakStrictMode'))
       breakWinLocal.webContents.send('progress', Date.now(), settings.get('breakDuration'))
       breakWinLocal.setAlwaysOnTop(true)
+      breakWinLocal.setVisibleOnAllWorkspaces(true)
     })
     breakWinLocal.loadURL(modalPath)
     breakWins.push(breakWinLocal)

@hovancik
Copy link
Owner

I see. Could you push your code and make a pr so I can play with it as well?

And have you tried it on electron-quick-start? Does it behave the same?

@mhewedy
Copy link
Author

mhewedy commented Jun 29, 2018

Mac contains two types of workspaces:

  • Application-created:
    application-created
  • vs User-created:
    user-created

The following table illustrates the diff between stretchly and electron-quick-start regarding specific features:

Feature stretchly electron-quick-start
setFullScreen create a application-created ws yes yes
setVisibleOnAllWorkspaces make the app visiable on user-created workspaces yes yes
setVisibleOnAllWorkspaces make the app visiable on other application-created workspaces yes no

@mhewedy
Copy link
Author

mhewedy commented Jun 30, 2018

Any updates on this from your side?

@hovancik
Copy link
Owner

hovancik commented Jul 1, 2018

Hi, I haven't had time yet ;[ There are some old PRs haunting me and my time is limited ;[ I need to do some code challenge for job interview tomorrow but after that I hope to find at least hour of time.

@mhewedy
Copy link
Author

mhewedy commented Jul 1, 2018

Good luck with your interview 👍 👍

@hovancik
Copy link
Owner

hovancik commented Jul 2, 2018

Thanks, I have moved interview for tomorrow, probably :D

Anyway, I have tried your PR and it seems that we can not put window over whole screen in that way ;[ At least I haven't found one ;[

there is also

      breakWinLocal.setSimpleFullScreen(true)
      breakWinLocal.setVisibleOnAllWorkspaces(true)

that is a bit better but the titlebar is still visible and skipping break text often gets hidden by bar ;[

So I guess what we can try is to open issue in electron and ask them to make setVisibleOnAllWorkspaces and setFullscreen both turned on behave in such a way that there is no escape from app that is fullscreen.

@hovancik
Copy link
Owner

hovancik commented Jul 2, 2018

Or maybe there is hope

I'm just trying breakWinLocal.setKiosk(true) and it seems to work.

I remember it not doing anything long time ago, so maybe they fixed it?

@mhewedy
Copy link
Author

mhewedy commented Jul 2, 2018

Woooow!
setKiosk(true) looks like it is the one!

@hovancik
Copy link
Owner

hovancik commented Jul 2, 2018

Would you like to update PR so that we only update setFullscreen to setKiosk ?

I've tested on linux and it's broken, but the fullscreen is as well (electron/electron#11632)

I have to test on Windows

@mhewedy
Copy link
Author

mhewedy commented Jul 2, 2018

@hovancik I noticed that the setKiosk(true) works as expected only in the electron-starter, not in stretchly, I am trying to find out why!

@hovancik
Copy link
Owner

hovancik commented Jul 2, 2018

It seemed to work from me. What I did was to replace setFullscreen with setKiosk. But not with your changes, I did it in master.

@mhewedy
Copy link
Author

mhewedy commented Jul 2, 2018

Have you tried to press ALT+TAB?

@hovancik
Copy link
Owner

hovancik commented Jul 3, 2018

Yes, that is able to workaround fullscreen break ;[ Unless electron devs come up with some mode where fullscreen is always fullscreen and on top, I don't think we can fix that ;/

@hovancik
Copy link
Owner

hovancik commented Jul 9, 2018

@mhewedy will you update your PR to use kiosk mode as I mentioned so we can have you as contributor?

I think the most we can do now is to keep behavior as it was before but use Kiosk, not Fullscreen. Will still need to check on Windows and Linux...

@mhewedy
Copy link
Author

mhewedy commented Jul 9, 2018

@hovancik It's my honor. but let me double check it because I am pretty sure the kiosk only working as expected in the electron-quick-stater.

@mhewedy
Copy link
Author

mhewedy commented Jul 14, 2018

@hovancik Sorry for being late, currently I am too busy, in one or two days I should be resume working on this issue.

@hovancik
Copy link
Owner

No problem ;] take your time

@pjebs
Copy link

pjebs commented Sep 15, 2018

I think there is a setting in the app mac app bundle plist that helps but I can't remember what it was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants