This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
windows restore in the correct order to ensure correct focused window #13161
+52
−19
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
10 tasks
Codecov Report
@@ Coverage Diff @@
## master #13161 +/- ##
==========================================
- Coverage 56.18% 56.15% -0.03%
==========================================
Files 279 279
Lines 27873 27895 +22
Branches 4560 4564 +4
==========================================
+ Hits 15660 15665 +5
- Misses 12213 12230 +17
|
petemill
force-pushed
the
fix/window-restore-focus
branch
from
5eda49a to
8d2a42e
Compare
petemill
commented
Feb 16, 2018
| @@ -77,6 +77,9 @@ ipc.on(messages.INITIALIZE_WINDOW, (e, mem) => { | |||
| const windowValue = message.windowValue | |||
|
|
|||
| currentWindow.setWindowId(windowValue.id) | |||
| if (process.env.NODE_ENV === 'development') { | |||
| console.debug(`This Window's ID is:`, windowValue.id) | |||
There was a problem hiding this comment.
Just a little something in each window's console.log to aid window debugging (when in dev mode, and set to show debug messages).
petemill
commented
Feb 16, 2018
| @@ -752,21 +750,18 @@ const doAction = (action) => { | |||
| } | |||
| break | |||
| } | |||
| case windowConstants.WINDOW_ON_WINDOW_UPDATE: | |||
There was a problem hiding this comment.
This constant does not exist
petemill
commented
Feb 16, 2018
| let windowValue = makeImmutable(action.windowValue) | ||
|
|
||
| if (windowValue.get('focused')) { | ||
| windowValue = windowValue.set('focusTime', performance.timing.navigationStart + performance.now()) |
There was a problem hiding this comment.
This should be set whenever window is updated (not only when it's ready) as we want to save when each window last had focus. Also we can use new Date().getTime() as milliseconds are perfectly fine, we don't need to resolve to a microsecond for this.
…dex is restored and the previously-focused window gets focus Fix #13158
bsclifton
force-pushed
the
fix/window-restore-focus
branch
from
8d2a42e to
a0133fe
Compare
bsclifton
approved these changes
Feb 19, 2018
bsclifton
added a commit
that referenced
this pull request
Feb 19, 2018
windows restore in the correct order to ensure correct focused window
bsclifton
added a commit
that referenced
this pull request
Feb 19, 2018
windows restore in the correct order to ensure correct focused window
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Do not merge before #12438 since it's best to test this fix with those changes too as that PR changes a lot of window logic. This PR should only show 1 commit to merge.
Fix #13158
Note that this test plan may never be 100% robust, but this gets us closer.
There will be additional cases that cannot reproduce success (i.e. if a window is macOS fullscreen), but the code to 'fix' those scenarios would likely introduce new issues and/or not be maintainable, for arguably small benefit.
Submitter Checklist:
git rebase -ito squash commits (if needed).Test Plan:
On #13158
Reviewer Checklist:
Tests