activate fix #1782
Merged
activate fix #1782
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Mason Fish <[email protected]>
Signed-off-by: Mason Fish <[email protected]>
mason-fish
force-pushed
the
1770-activate-bug
branch
from
a8f8e3b
to
fd24e51
Compare
jameskerr
requested changes
Aug 19, 2021
src/js/electron/brim.ts
Outdated
Comment on lines
73
to
75
| log.info("activate fired! window count: ", this.windows.count()) | ||
| if (this.windows.count() === 0 || this.windows.count() === 1) | ||
| this.windows.init() |
There was a problem hiding this comment.
Let's add a method called this.windows.getVisible().length I think that would help this read more clearly.
Comment on lines
212
to
213
| if (this.getWindows().find((w) => w.name === "hidden")) return | ||
| this.openWindow("hidden") |
There was a problem hiding this comment.
Let's make methods called
getAll()
getVisible()
getHidden()
Then we could use them in all the places that make sense.
jameskerr
reviewed
Aug 19, 2021
| @@ -47,7 +47,7 @@ export default function windowManager( | |||
| return { | |||
| init() { | |||
| // hidden renderer/window is never persisted, so always open it with rest | |||
| this.openWindow("hidden") | |||
| this.openHidden() | |||
There was a problem hiding this comment.
Maybe this could be called ensureHiddenRenderer?
jameskerr
reviewed
Aug 19, 2021
| manager.openHidden() | ||
| windows = manager.getWindows() | ||
| expect(windows).toHaveLength(1) | ||
| expect(windows[0].name).toEqual("hidden") |
jameskerr
reviewed
Aug 19, 2021
src/js/electron/brim.ts
Outdated
| @@ -69,7 +70,9 @@ export class BrimMain { | |||
| } | |||
|
|
|||
| activate() { | |||
| if (this.windows.count() === 0) this.windows.init() | |||
| log.info("activate fired! window count: ", this.windows.count()) | |||
mason-fish
force-pushed
the
1770-activate-bug
branch
from
5f93612
to
f0675a4
Compare
Signed-off-by: Mason Fish <[email protected]>
mason-fish
force-pushed
the
1770-activate-bug
branch
from
f0675a4
to
7e8ca00
Compare
Signed-off-by: Mason Fish <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1770
Part of the activation handler was still conditional to there being zero windows rendered, which should ideally never exist since there should always be at least a hidden window. This adjusts the logic accordingly. I also found a way to render multiple hidden windows using this activation flow, so I've included some protection around that as well.