This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 972
'New tab' dashboard images fade in from dark #11817
Merged
Merged
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
Codecov Report
@@ Coverage Diff @@
## master #11817 +/- ##
==========================================
+ Coverage 52.61% 52.65% +0.03%
==========================================
Files 271 271
Lines 25715 25673 -42
Branches 4104 4100 -4
==========================================
- Hits 13530 13518 -12
+ Misses 12185 12155 -30
|
bsclifton
approved these changes
Nov 15, 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is beautiful- works exactly as expected 😄 👍
Great job!
Holding off on merge for this one; we'll likely pull into a future 0.19.x hotfix 😄 |
8 tasks
bsclifton
added a commit
that referenced
this pull request
Nov 16, 2017
'New tab' dashboard images fade in from dark
bsclifton
added a commit
that referenced
this pull request
Nov 16, 2017
'New tab' dashboard images fade in from dark
petemill
pushed a commit
to petemill/browser-laptop
that referenced
this pull request
Nov 17, 2017
'New tab' dashboard images fade in from dark
bsclifton
added a commit
that referenced
this pull request
Nov 17, 2017
'New tab' dashboard images fade in from dark
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.
Fix #5309
Addresses new tab when in 'Dashboard' mode. Sets default background color so that the text is readable without the background having loaded. Waits for image to load completely before fading it in. Also general clean up to remove separate gradient element and double-setting of wallpaper image source.
Note: I noticed after I had done this that this had been attempted before in #6590, but that was abandoned because of the white flash before the tab loads. This minimizes it by further explicitly setting the background color in html before the JS / react components have loaded (although webpack can be configured to output the css as a .css file which can be included in about-newtab.html). The flashes of white that are still present on both tab open and close are from both the background on open, and the window background on close (when no other tab is shown). These will be addressed in #11813.
Here's what it looks like: https://www.dropbox.com/s/qpsk8fk3m5rpx3f/newtab-fade-i5309.mov?dl=0
Here's what it looked like before (with empty profile): https://www.dropbox.com/s/05mz3expvewmcsv/newtab-fade-before-i5309.mov?dl=0
Submitter Checklist:
git rebase -i
to squash commits (if needed).Test Plan:
Automated tests included.
Manual tests include checking that image loads with good network, that fallback image loads with no network and clear cache, and that when settings are set to no images on Dashboard, that the very colorful gradient shows.
Reviewer Checklist:
Tests