about:newtab dashboard page shows text on a dark background, and fade…

…s in the image when loaded Fix #5309 Note: the flashes of white that are still present on both tab open and close are from both the <webview> background on open, and the window background on close (when no other tab is shown). These will be addressed in #11813
brave · Nov 6, 2017 · 5fbf1af · 5fbf1af
1 parent 88974f2
commit 5fbf1af
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 72 deletions.
diff --git a/app/extensions/brave/about-newtab.html b/app/extensions/brave/about-newtab.html
@@ -4,6 +4,11 @@
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 <html>
   <head>
+    <style>
+      html, body {
+        background: #222;
+      }
+    </style>
     <meta charset="utf-8">
     <meta name="availableLanguages" content="">
     <meta name="defaultLanguage" content="en-US">

diff --git a/js/about/newtab.js b/js/about/newtab.js
@@ -30,6 +30,7 @@ const backgrounds = require('../data/backgrounds')
 // Utils
 const urlutils = require('../lib/urlutil')
 const {random} = require('../../app/common/lib/randomUtil')
+const cx = require('../lib/classSet')
 const ipc = window.chrome.ipcRenderer
 
 // Styles
@@ -42,6 +43,7 @@ class NewTabPage extends React.Component {
     this.state = {
       showNotification: false,
       imageLoadFailed: false,
+      imageLoadComplete: false,
       updatedStamp: undefined,
       showEmptyPage: true,
       showImages: false,
@@ -79,14 +81,13 @@ class NewTabPage extends React.Component {
 
   get randomBackgroundImage () {
     const image = Object.assign({}, backgrounds[Math.floor(random() * backgrounds.length)])
-    image.style = {backgroundImage: 'url(' + image.source + ')'}
     return image
   }
 
   get fallbackImage () {
     const image = Object.assign({}, config.newtab.fallbackImage)
     const pathToImage = path.join(__dirname, '..', '..', image.source)
-    image.style = {backgroundImage: 'url(' + `${pathToImage}` + ')'}
+    image.source = pathToImage
     return image
   }
 
@@ -229,6 +230,12 @@ class NewTabPage extends React.Component {
     })
   }
 
+  onImageLoadCompleted () {
+    this.setState({
+      imageLoadComplete: true
+    })
+  }
+
   getLetterFromUrl (url) {
     const hostname = urlutils.getHostname(url.get('location'), true)
     const name = url.get('title') || hostname || '?'
@@ -250,20 +257,27 @@ class NewTabPage extends React.Component {
       return null
     }
     const gridLayout = this.gridLayout
-    const backgroundProps = {}
-    let gradientClassName = 'gradient'
-    if (this.showImages) {
-      backgroundProps.style = this.state.backgroundImage.style
-      gradientClassName = 'bgGradient'
-    }
-    return <div data-test-id='dynamicBackground' className='dynamicBackground' {...backgroundProps}>
+    return <div data-test-id='dynamicBackground' className='dynamicBackground'>
       {
-        this.showImages
-          ? <img src={this.state.backgroundImage.source} onError={this.onImageLoadFailed.bind(this)} data-test-id='backgroundImage' />
-          : null
+        this.showImages &&
+        <div
+          className={cx({
+            imageBackground: true,
+            hasLoaded: this.state.imageLoadComplete
+          }
+        )}>
+          <img
+            src={this.state.backgroundImage.source}
+            onLoad={this.onImageLoadCompleted.bind(this)}
+            onError={this.onImageLoadFailed.bind(this)}
+            data-test-id='backgroundImage' />
+        </div>
       }
-      <div data-test-id={this.showImages ? 'bgGradient' : 'gradient'} className={gradientClassName} />
-      <div className='content'>
+      <div className={cx({
+        content: true,
+        backgroundLoaded: this.state.imageLoadComplete,
+        showImages: this.showImages
+      })}>
         <main>
           <div className='statsBar'>
             <Stats newTabData={this.state.newTabData} />

diff --git a/less/about/newtab.less b/less/about/newtab.less
@@ -9,23 +9,12 @@
   box-sizing: border-box;
 }
 
-
 strong, div, span, li, em, p, a {
   font-family: "Helvetica Neue", Arial, sans-serif;
   font-weight: 400;
   -webkit-font-smoothing: antialiased;
 }
 
-body {
-  background: #fff;
-}
-
-body, .dynamicBackground, bgGradient {
-  opacity: 0;
-  animation: fadeIn 200ms;
-  animation-fill-mode: forwards;
-}
-
 .copyrightNotice {
   user-select: none;
   cursor: default;
@@ -45,33 +34,27 @@ ul {
 }
 
 .dynamicBackground {
-  background-position: top center;
-  background-repeat: no-repeat;
-  background-size: cover;
   display: flex;
   flex: 1;
-
-  > img {
-    display: none;
-  }
 }
 
-.gradient {
-  background: radial-gradient(circle farthest-corner at right bottom,#ff4444 0,rgb(9, 14, 160) 100%);
-  height: 100%;
-  left: 0;
-  position: absolute;
-  top: 0;
-  width: 100%;
-}
-
-.bgGradient {
+.imageBackground {
   position: absolute;
   top: 0;
+  bottom: 0;
   left: 0;
-  width: 100%;
-  background: linear-gradient(rgba(0, 0, 0, 1), rgba(0, 0, 0, 0));
-  height: 400px;
+  right: 0;
+  z-index: 1;
+  img {
+    width: 100%;
+    height: 100%;
+    object-fit: cover;
+    opacity: 0;
+    transition: opacity .5s ease-in-out;
+  }
+  &.hasLoaded img {
+    opacity: 1;
+  }
 }
 
 .content {
@@ -86,6 +69,11 @@ ul {
   height: 100%;
   min-height: 100vh;
   padding: 40px 60px;
+  background: transparent;
+  background: radial-gradient(circle farthest-corner at right bottom,#ff4444 0,rgb(9, 14, 160) 100%);
+  &.showImages {
+    background: linear-gradient(to bottom, rgba(34, 34, 34, 1) 0%, rgba(0, 0, 0, 0) 400px);
+  }
 
   .topSitesContainer {
     width: 100%;

diff --git a/test/unit/about/newTabPageTest.js b/test/unit/about/newTabPageTest.js
@@ -46,9 +46,7 @@ describe('NewTab component unit tests', function () {
 
   let wrapper, incognitoWrapper
   const backgroundImage = {
-    style: {
-      backgroundImage: 'url(testing123.jpg)'
-    }
+    source: 'testing123.jpg'
   }
   const TIME_UNIT = {
     SECOND: 'S',
@@ -116,23 +114,19 @@ describe('NewTab component unit tests', function () {
         assert.equal(randomSpy.calledOnce, true)
       })
 
-      it('returns an object which has a value set for `style.backgroundImage`', function () {
+      it('returns an object which has a value set for `source`', function () {
         const instance = wrapper.instance()
         const result = instance.randomBackgroundImage
         assert.notEqual(result, undefined)
-        assert.notEqual(result.style, undefined)
-        assert.notEqual(result.style.backgroundImage, undefined)
-        assert.equal(!!result.style.backgroundImage.match(/^url\(/), true)
+        assert.notEqual(result.source, undefined)
       })
     })
     describe('fallbackImage', function () {
-      it('returns an object which has a value set for `style.backgroundImage`', function () {
+      it('returns an object which has a value set for `source`', function () {
         const instance = wrapper.instance()
         const result = instance.fallbackImage
         assert.notEqual(result, undefined)
-        assert.notEqual(result.style, undefined)
-        assert.notEqual(result.style.backgroundImage, undefined)
-        assert.equal(!!result.style.backgroundImage.match(/^url\(/), true)
+        assert.notEqual(result.source, undefined)
       })
     })
     describe('topSites', function () {
@@ -192,18 +186,14 @@ describe('NewTab component unit tests', function () {
           })
         })
 
-        it('sets backgroundImage for root element to the URL of the image', function () {
-          const node = wrapper.find('[data-test-id="dynamicBackground"]').node
-          assert.notEqual(node, undefined)
-          assert.deepEqual(node.props.style, backgroundImage.style)
-        })
-
-        it('includes div element with class bgGradient', function () {
-          assert.equal(wrapper.find('div[data-test-id="bgGradient"]').length, 1)
+        it('includes wallpaper background element', function () {
+          assert.equal(wrapper.find('[data-test-id="backgroundImage"]').length, 1)
         })
 
-        it('includes img element (used to detect onError)', function () {
-          assert.equal(wrapper.find('img[data-test-id="backgroundImage"]').length, 1)
+        it('sets backgroundImage for root element to the URL of the image', function () {
+          const node = wrapper.find('[data-test-id="backgroundImage"]').node
+          assert.notEqual(node, undefined)
+          assert.deepEqual(node.props.src, backgroundImage.source)
         })
       })
 
@@ -217,12 +207,8 @@ describe('NewTab component unit tests', function () {
           })
         })
 
-        it('includes element with class gradient', function () {
-          assert.equal(wrapper.find('div[data-test-id="gradient"]').length, 1)
-        })
-
-        it('does NOT include img element (used to detect onError)', function () {
-          assert.equal(wrapper.find('img[data-test-id="backgroundImage"]').length, 0)
+        it('does NOT include wallpaper background', function () {
+          assert.equal(wrapper.find('[data-test-id="backgroundImage"]').length, 0)
         })
       })
     })