SDA-3990: Hide all windows and restore with previously focused window #1659
Conversation
NguyenTranHoangSym
force-pushed
the
SDA-3990
branch
from
02bfe7e
to
e5bac8b
Compare
src/app/screen-snippet-handler.ts
Outdated
| (window as ICustomBrowserWindow).winName !== currentWindow || | ||
| (window as ICustomBrowserWindow).winName !== 'main' | ||
| ) { | ||
| arr.push((window as ICustomBrowserWindow).winName); |
There was a problem hiding this comment.
I don't think every window has a winName no? Why not relying on the window ID here?
There was a problem hiding this comment.
I only exclude main not to push it in, already got main outside
The Idea here is I will consecutively push the element into array. Then with each of element in array I take it out to process consequently
src/app/screen-snippet-handler.ts
Outdated
| const currentWindows = BrowserWindow.getAllWindows(); | ||
|
|
||
| currentWindows.forEach((currentWindow) => { | ||
| currentWindow?.minimize(); |
There was a problem hiding this comment.
How does this behave on Windows with a window in fullscreen mode?
There was a problem hiding this comment.
minmize and focus will switch back to previously working windows.
focus will bring it back instead of doing anything else. (No config modified)
There was a problem hiding this comment.
I don't think this approach will cover all use-cases.
One of them which covers 2 use-cases actually is:
- having 5 windows popped: 2 windows minimised and one in full screen
if hideOnCapture === true:
- all windows will be minimised
- we take a screenshot
- all windows will be restored
Consequence: 5 windows on the screen rather than 3 with one in fullscreen
Maybe having a structure like this would help restoring all windows?
interface WindowState {
minimized: boolean;
focused: boolean;
fullscreen: boolean
}
const windowsState: WindowState[] = [];
we populate windowsState before hiding all windows and we restore all windows based on windowsState
As we only have one focused window, this one should be the last one to be "restored" and focused.
WDYT @NguyenTranHoangSym ?
There was a problem hiding this comment.
@sbenmoussati
You are right. The status of currently minimized (The fullscreen thing i guess it is unaffected, since I will only focus back on the windows without changing anything). Im going to add an additional case to not restore the minimized ones
src/app/screen-snippet-handler.ts
Outdated
| @@ -180,6 +199,25 @@ class ScreenSnippet { | |||
|
|
|||
| if (dimensions.width === 0 && dimensions.height === 0) { | |||
| logger.info('screen-snippet-handler: no screen capture picture'); | |||
|
|
|||
| if (hideOnCapture) { | |||
| const currentFocusedWindow = winStore.getWindowStore(); | |||
There was a problem hiding this comment.
I believe this doesn't correspond to currentFocusedWindow, would currentWindows be more appropriate?
There was a problem hiding this comment.
Yes totally agree let me change
NguyenTranHoangSym
force-pushed
the
SDA-3990
branch
from
e5bac8b
to
a8e474d
Compare
NguyenTranHoangSym
force-pushed
the
SDA-3990
branch
from
a8e474d
to
16f1897
Compare
src/app/screen-snippet-handler.ts
Outdated
| { | ||
| id: 'main', | ||
| focused: mainWindow?.isFocused(), | ||
| minimized: mainWindow?.isMinimized(), |
There was a problem hiding this comment.
[Question]: IWindowState has a fullscreen property that is missing here, any reason for that?
There was a problem hiding this comment.
We don't actually need it now. I forgot to remove it.
Since rightnow you will show the old windows without changing anything relating to the aspect of windows
src/app/screen-snippet-handler.ts
Outdated
| if (currentWindow !== 'main') { | ||
| curWindow?.minimize(); | ||
|
|
||
| if (windowObj.windows.length < 1) { |
There was a problem hiding this comment.
I guess it would be clearer if we have dedicated functions like hideWindows() and restoreWindows(), what do you think?
There was a problem hiding this comment.
Done,. I have made it some common methods. Can be shared between components
NguyenTranHoangSym
force-pushed
the
SDA-3990
branch
2 times, most recently
from
64ce119
to
988b40c
Compare
NguyenTranHoangSym
force-pushed
the
SDA-3990
branch
from
988b40c
to
7c8402b
Compare
Description
Related PRs