Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Avoid 'flashes of white' when tabs open or close #11818

Merged
merged 1 commit into from
Nov 18, 2017
Merged

Conversation

petemill
Copy link
Member

@petemill petemill commented Nov 7, 2017

Remove white flash for new tab pages which use about:newtab.
Sets the frame background to the same background as the newtab page, for a smooth transition between frame creation and the tab beginning to load content.
Keeps a closed frame around until the next frame is ready, to avoid flash when closing tabs.

Fix #11813
Addresses #5309

When opening a tab, there is a flash of the background of the component, which is explicitly set to white in window.less.
This is avoided by setting the background to the 'new tab dashboard' background color (only when that is the Url for the frame), and by hiding the frame if it's about:blank and for the first 150ms since when a frame is first created, it starts as about:blank and then sets its location to the specified Url.

When closing a tab, there is a flash of the background of the whole window (or at least div.tabContainer) as the active frame is removed, but the next active frame is not set active yet. This is avoided by keeping the Element for the frame around for 100ms, with a low z-index, so that the new / next active tab will show on top when it is ready.

Closing tabs - Before:
https://www.dropbox.com/s/aon11cov3nqcau2/p11813-close-tabs-before.mov?dl=0

Closing tabs - After:
https://www.dropbox.com/s/0x1n8msobxssihh/p11813-close-tabs-after.mov?dl=0

Test Plan:

  • When creating a new tab there is no flash between the frames (difficult to see until 'New tab' dashboard images fade in from dark #11817 lands since all new frames have white background, and the window has a white background).
  • When closing a tab with a loaded previous and next tab, there is no white flash between leaving the old tab and showing the next tab.

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.

Security

Introduces new package: https://github.com/reactjs/react-transition-group

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

package.json Outdated
"react-select": "^0.9.1",
"react-transition-group": "^2.2.1",
Copy link
Member

Choose a reason for hiding this comment

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

@petemill
Copy link
Member Author

I've rebased this branch and specified a test plan. Would appreciate a review and merge to master so I can land open a PR for some other features based on this.

getTransitionStateClassName (stateName) {
if (!stateName) {
return null
} else {
Copy link
Member

Choose a reason for hiding this comment

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

nit: no else needed here

@bsclifton
Copy link
Member

bsclifton commented Nov 17, 2017

Although we default to dashboard, users can pick others (including an empty tab). I noticed if I picked empty tab, the tab stays as the gray color
screen shot 2017-11-17 at 12 45 38 am

screen shot 2017-11-17 at 12 48 32 am

There might be a flash of gray when opening a new tab when set to search engine / home page, but I can't tell for sure (my eyes are horrible! it's too fast 👀 )

This appears to have been introduced w/ #11817 (missed when reviewing originally)

@codecov-io
Copy link

codecov-io commented Nov 17, 2017

Codecov Report

Merging #11818 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #11818      +/-   ##
==========================================
- Coverage   53.38%   53.38%   -0.01%     
==========================================
  Files         274      274              
  Lines       26018    26020       +2     
  Branches     4170     4170              
==========================================
+ Hits        13890    13891       +1     
- Misses      12128    12129       +1
Flag Coverage Δ
#unittest 53.38% <100%> (-0.01%) ⬇️
Impacted Files Coverage Δ
js/about/newtab.js 49.38% <ø> (-0.31%) ⬇️
app/renderer/components/styles/theme.js 100% <ø> (ø) ⬆️
js/about/newprivatetab.js 79.16% <100%> (+0.9%) ⬆️

Remove white flash for new tab pages which use about:newtab.
Sets the frame background to the same background as the newtab page, for a smooth transition between frame creation and the tab beginning to load content.
Keeps the frame around until the next frame is ready, to avoid flash when closing tabs.
Fix flash of 'dark' when opening new private tab by fading in private tab background.

Addresses #5309
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Works as expected and looks great! Really nice polish in here 😄 👌

Thanks for following up with @bradleyrichter and others on this one

@bsclifton bsclifton merged commit 93d7a77 into master Nov 18, 2017
@bsclifton bsclifton deleted the issue-11813 branch November 18, 2017 07:52
bsclifton added a commit that referenced this pull request Nov 18, 2017
Avoid 'flashes of white' when tabs open or close
bsclifton added a commit that referenced this pull request Nov 18, 2017
Avoid 'flashes of white' when tabs open or close
@bsclifton
Copy link
Member

bsclifton commented Nov 18, 2017

master 93d7a77
0.21.x 225c213
0.20.x b5e7e43
0.19.x 8f12cdf (by petemill)

petemill pushed a commit that referenced this pull request Nov 18, 2017
Avoid 'flashes of white' when tabs open or close
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

White flash after tab open or close
5 participants