Store Unsplash background image not dataturl (#836)

* store background image, not background selection in presets

* move localStorage into save utils

* curry omit util fn

* clean up manual image upload by URL

* refactor image changes

* separate uploadImage from selectImage

* upgrade visual regression testing tests

* add TODOs

components/Editor.js

@@ -38,6 +38,8 @@ const BackgroundSelect = dynamic(() => import('./BackgroundSelect'), {
   loading: () => null
 })
 
+const getConfig = omit(['code'])
+
 class Editor extends React.Component {
   static contextType = ApiContext
 
@@ -326,7 +328,7 @@ class Editor extends React.Component {
       exportSize
     } = this.state
 
-    const config = omit(this.state, ['code'])
+    const config = getConfig(this.state)
 
     const theme = this.getTheme()
 

components/EditorContainer.js

@@ -16,7 +16,7 @@ function EditorContainer(props) {
   }, [])
 
   React.useEffect(() => {
-    saveThemes(localStorage, themes.filter(({ custom }) => custom))
+    saveThemes(themes.filter(({ custom }) => custom))
   }, [themes])
 
   return <Editor {...props} themes={themes} updateThemes={updateThemes} />

components/ImagePicker.js

@@ -40,7 +40,8 @@ const INITIAL_STATE = {
   crop: null,
   imageAspectRatio: null,
   pixelCrop: null,
-  photographer: null
+  photographer: null,
+  dataURL: null
 }
 
 export default class ImagePicker extends React.Component {
@@ -48,13 +49,14 @@ export default class ImagePicker extends React.Component {
   constructor(props) {
     super(props)
     this.state = INITIAL_STATE
+    this.selectMode = this.selectMode.bind(this)
     this.handleURLInput = this.handleURLInput.bind(this)
+    this.uploadImage = this.uploadImage.bind(this)
     this.selectImage = this.selectImage.bind(this)
     this.removeImage = this.removeImage.bind(this)
     this.onImageLoaded = this.onImageLoaded.bind(this)
     this.onCropChange = this.onCropChange.bind(this)
     this.onDragEnd = this.onDragEnd.bind(this)
-    this.selectMode = this.selectMode.bind(this)
   }
 
   static getDerivedStateFromProps(nextProps, state) {
@@ -73,9 +75,13 @@ export default class ImagePicker extends React.Component {
     return null
   }
 
+  selectMode(mode) {
+    this.setState({ mode })
+  }
+
   async onDragEnd() {
     if (this.state.pixelCrop) {
-      const croppedImg = await getCroppedImg(this.props.imageDataURL, this.state.pixelCrop)
+      const croppedImg = await getCroppedImg(this.state.dataURL, this.state.pixelCrop)
       this.props.onChange({ backgroundImageSelection: croppedImg })
     }
   }
@@ -102,18 +108,23 @@ export default class ImagePicker extends React.Component {
     })
   }
 
+  handleImageChange = (url, dataURL, photographer) => {
+    this.setState({ dataURL, photographer }, () => {
+      this.props.onChange({
+        backgroundImage: url,
+        backgroundImageSelection: null,
+        photographer
+      })
+    })
+  }
+
   handleURLInput(e) {
     e.preventDefault()
     const url = e.target[0].value
     return this.context
       .downloadThumbnailImage({ url })
-      .then(({ dataURL }) =>
+      .then(res => res.dataURL)
-        this.props.onChange({
+      .then(dataURL => this.handleImageChange(url, dataURL))
-          backgroundImage: dataURL,
-          backgroundImageSelection: null,
-          photographer: null
-        })
-      )
       .catch(err => {
         if (err.message.indexOf('Network Error') > -1) {
           this.setState({
@@ -124,22 +135,15 @@ export default class ImagePicker extends React.Component {
       })
   }
 
-  selectMode(mode) {
+  async uploadImage(e) {
-    this.setState({ mode })
+    const dataURL = await fileToDataURL(e.target.files[0])
+    return this.handleImageChange(dataURL, dataURL)
   }
 
-  selectImage(e, { photographer } = {}) {
+  async selectImage(url, { photographer } = {}) {
-    const file = e.target ? e.target.files[0] : e
+    // TODO use React suspense for loading this asset
-
+    const { dataURL } = await this.context.downloadThumbnailImage({ url })
-    return fileToDataURL(file).then(dataURL =>
+    return this.handleImageChange(url, dataURL, photographer)
-      this.setState({ photographer }, () => {
-        this.props.onChange({
-          backgroundImage: dataURL,
-          backgroundImageSelection: null,
-          photographer
-        })
-      })
-    )
   }
 
   removeImage() {
@@ -172,7 +176,7 @@ export default class ImagePicker extends React.Component {
             <Input
               type="file"
               accept="image/png,image/x-png,image/jpeg,image/jpg"
-              onChange={this.selectImage}
+              onChange={this.uploadImage}
             />
           ) : (
             <form onSubmit={this.handleURLInput}>
@@ -251,7 +255,7 @@ export default class ImagePicker extends React.Component {
       </div>
     )
 
-    if (this.props.imageDataURL) {
+    if (this.state.dataURL) {
       content = (
         <div className="settings-container">
           <div className="image-container">
@@ -260,7 +264,7 @@ export default class ImagePicker extends React.Component {
               <button onClick={this.removeImage}>&times;</button>
             </div>
             <ReactCrop
-              src={this.props.imageDataURL}
+              src={this.state.dataURL}
               onImageLoaded={this.onImageLoaded}
               crop={this.state.crop}
               onChange={this.onCropChange}

components/RandomImage.js

@@ -13,7 +13,7 @@ function RandomImage(props) {
   const [selectImage, { loading: selecting }] = useAsyncCallback(() => {
     const image = cacheRef.current[cacheIndex]
 
-    return api.unsplash.download(image.id).then(blob => props.onChange(blob, image))
+    return api.unsplash.download(image.id).then(url => props.onChange(url, image))
   })
 
   const [updateCache, { loading: updating, error, data: imgs }] = useAsyncCallback(

components/Settings.js

@@ -347,7 +347,7 @@ class Settings extends React.PureComponent {
     )
   }
 
-  savePresets = () => savePresets(localStorage, this.state.presets.filter(p => p.custom))
+  savePresets = () => savePresets(this.state.presets.filter(p => p.custom))
 
   renderContent = () => {
     switch (this.state.selectedMenu) {

cypress/integration/visual-testing.spec.js

@@ -0,0 +1,115 @@
+/* global cy, before, after */
+import { environment } from '../util'
+
+describe.skip('Visual regression testing', () => {
+  describe('Buttons', () => {
+    before(() => {
+      cy.eyesOpen({
+        appName: 'Carbon',
+        testName: 'Button',
+        browser: environment
+      })
+      cy.visit('/')
+    })
+
+    beforeEach(() => {
+      cy.reload()
+    })
+
+    after(() => {
+      cy.eyesClose()
+    })
+
+    it('test export button', () => {
+      cy.get('[data-cy=export-button]').click()
+      cy.eyesCheckWindow({
+        tag: 'export button',
+        target: 'region',
+        selector: '.page'
+      })
+    })
+
+    it('test display button', () => {
+      cy.get('[data-cy=display]').click()
+      cy.eyesCheckWindow({
+        tag: 'display button',
+        target: 'region',
+        selector: '.page'
+      })
+    })
+
+    it('test color button', () => {
+      cy.get('[data-cy=display]').click()
+      cy.wait(2000)
+      cy.get('[title="#50E3C2"]').click()
+      cy.wait(500)
+      cy.eyesCheckWindow({
+        tag: 'color button',
+        target: 'region',
+        selector: '.page'
+      })
+    })
+  })
+
+  describe('Syntax', () => {
+    before(() => {
+      cy.eyesOpen({
+        appName: 'Carbon',
+        testName: 'Syntax',
+        browser: environment
+      })
+    })
+
+    after(() => {
+      cy.eyesClose()
+    })
+
+    const cases = [
+      ['JSON', "/?code={name:'Andrew',age:30}&l=application%2Fjson"],
+      ['C#', '/?code=class Program { static void Main(){ do }}&l=text%2Fx-csharp'],
+      ['C++', '/?l=text%2Fx-c%2B%2Bsrc&code=for(size_t i=0 ;i<length; i%2B%2B){}']
+    ]
+
+    cases.forEach(([language, example]) => {
+      it(`Syntax test for "${language}"`, () => {
+        cy.visit(example)
+        cy.eyesCheckWindow({
+          tag: language,
+          target: 'region',
+          selector: '.page'
+        })
+      })
+    })
+  })
+
+  describe('Themes', () => {
+    before(() => {
+      cy.eyesOpen({
+        appName: 'Carbon',
+        testName: 'Syntax',
+        browser: environment
+      })
+    })
+
+    after(() => {
+      cy.eyesClose()
+    })
+
+    const cases = [
+      ['JSON', "/?code={name:'Andrew',age:30}&l=application%2Fjson"],
+      ['C#', '/?code=class Program { static void Main(){ do }}&l=text%2Fx-csharp'],
+      ['C++', '/?l=text%2Fx-c%2B%2Bsrc&code=for(size_t i=0 ;i<length; i%2B%2B){}']
+    ]
+
+    cases.forEach(([language, example]) => {
+      it(`Syntax test for "${language}"`, () => {
+        cy.visit(example)
+        cy.eyesCheckWindow({
+          tag: language,
+          target: 'region',
+          selector: '.page'
+        })
+      })
+    })
+  })
+})

cypress/integration/visual-testing/button-test.spec.js

@@ -1,51 +0,0 @@
-/* global cy, before, after */
-import { environment } from '../../util'
-
-describe('Visual Regression Testing', () => {
-  before(() => {
-    cy.eyesOpen({
-      appName: 'Carbon',
-      testName: 'Button',
-      browser: environment
-    })
-    cy.visit('/')
-  })
-
-  beforeEach(() => {
-    cy.reload()
-  })
-
-  after(() => {
-    cy.eyesClose()
-  })
-
-  it('test export button', () => {
-    cy.get('[data-cy=export-button]').click()
-    cy.eyesCheckWindow({
-      tag: 'export button',
-      target: 'region',
-      selector: '.page'
-    })
-  })
-
-  it('test display button', () => {
-    cy.get('[data-cy=display]').click()
-    cy.eyesCheckWindow({
-      tag: 'display button',
-      target: 'region',
-      selector: '.page'
-    })
-  })
-
-  it('test color button', () => {
-    cy.get('[data-cy=display]').click()
-    cy.wait(2000)
-    cy.get('[title="#50E3C2"]').click()
-    cy.wait(500)
-    cy.eyesCheckWindow({
-      tag: 'color button',
-      target: 'region',
-      selector: '.page'
-    })
-  })
-})

cypress/integration/visual-testing/syntax-test.spec.js

@@ -1,33 +0,0 @@
-/* global cy, before, after */
-import { environment } from '../../util'
-
-describe('Visual Regression Testing', () => {
-  before(() => {
-    cy.eyesOpen({
-      appName: 'Carbon',
-      testName: 'Syntax',
-      browser: environment
-    })
-  })
-
-  after(() => {
-    cy.eyesClose()
-  })
-
-  const cases = [
-    ['JSON', "/?code={name:'Andrew',age:30}&l=application%2Fjson"],
-    ['C#', '/?code=class Program { static void Main(){ do }}&l=text%2Fx-csharp'],
-    ['C++', '/?l=text%2Fx-c%2B%2Bsrc&code=for(size_t i=0 ;i<length; i%2B%2B){}']
-  ]
-
-  cases.forEach(([language, example]) => {
-    it(`Syntax test for "${language}"`, () => {
-      cy.visit(example)
-      cy.eyesCheckWindow({
-        tag: language,
-        target: 'region',
-        selector: '.page'
-      })
-    })
-  })
-})

cypress/integration/visual-testing/theme-test.spec.js

@@ -1,29 +0,0 @@
-/* global cy, before ,after */
-import { environment } from '../../util'
-import { THEMES } from '../../../lib/constants'
-
-describe('Visual Regression Testing', () => {
-  before(() => {
-    cy.eyesOpen({
-      appName: 'Carbon',
-      testName: 'Themes',
-      browser: environment
-    })
-    cy.visit('/')
-  })
-
-  after(() => {
-    cy.eyesClose()
-  })
-
-  THEMES.forEach(t => {
-    it(`Test theme: "${t.name}"`, () => {
-      cy.get('[data-cy="themes-container"] [data-cy="theme-selector-button"]').click()
-      cy.contains(t.name).click({ force: true })
-      cy.eyesCheckWindow({
-        target: 'region',
-        selector: '.page'
-      })
-    })
-  })
-})

lib/api.js

@@ -107,11 +107,7 @@ const downloadThumbnailImage = img => {
 
 const unsplash = {
   download(id) {
-    return client
+    return client.get(`/unsplash/download/${id}`).then(res => res.data.url)
-      .get(`/unsplash/download/${id}`)
-      .then(res => res.data.url)
-      .then(url => client.get(url, { responseType: 'blob' }))
-      .then(res => res.data)
   },
   async random() {
     const imageUrls = await client.get('/unsplash/random')

lib/util.js

@@ -9,11 +9,20 @@ const THEMES_KEY = 'CARBON_THEMES'
 const createAssigner = key => {
   const assign = morph.assign(key)
 
-  return (window, v) => assign(window, JSON.stringify(v))
+  return v => assign(localStorage, JSON.stringify(v))
 }
 
-export const saveSettings = createAssigner(SETTINGS_KEY)
+const map = fn => obj => obj.map(fn)
-export const savePresets = createAssigner(PRESETS_KEY)
+export const omit = keys => object => omitBy(object, (_, k) => keys.indexOf(k) > -1)
+
+export const saveSettings = morph.compose(
+  createAssigner(SETTINGS_KEY),
+  omit(['code', 'backgroundImage', 'backgroundImageSelection', 'themes', 'highlights', 'fontUrl'])
+)
+export const savePresets = morph.compose(
+  createAssigner(PRESETS_KEY),
+  map(omit(['backgroundImageSelection']))
+)
 export const saveThemes = createAssigner(THEMES_KEY)
 
 const parse = v => {
@@ -87,8 +96,6 @@ export const formatCode = async code => {
   })
 }
 
-export const omit = (object, keys) => omitBy(object, (_, k) => keys.indexOf(k) > -1)
-
 export const stringifyRGBA = obj => `rgba(${obj.r},${obj.g},${obj.b},${obj.a})`
 
 export const capitalize = s => s.charAt(0).toUpperCase() + s.slice(1)

pages/index.js

@@ -10,7 +10,7 @@ import EditorContainer from '../components/EditorContainer'
 import Page from '../components/Page'
 import { MetaLinks } from '../components/Meta'
 import { updateRouteState } from '../lib/routing'
-import { saveSettings, clearSettings, omit } from '../lib/util'
+import { saveSettings, clearSettings } from '../lib/util'
 
 class Index extends React.Component {
   componentDidMount() {
@@ -25,17 +25,7 @@ class Index extends React.Component {
   onEditorUpdate = debounce(
     state => {
       updateRouteState(this.props.router, state)
-      saveSettings(
+      saveSettings(state)
-        localStorage,
-        omit(state, [
-          'code',
-          'backgroundImage',
-          'backgroundImageSelection',
-          'themes',
-          'highlights',
-          'fontUrl'
-        ])
-      )
     },
     750,
     { trailing: true, leading: true }