Merge pull request #524 from ethereum/fileExplorerCleanup

src/app.js

@@ -63,19 +63,12 @@ window.addEventListener('message', function (ev) {
   }
 }, false)
 
-/*
-  trigger tabChanged
-*/
 var run = function () {
   var self = this
   this.event = new EventManager()
   var fileStorage = new Storage('sol:')
   var files = new Files(fileStorage)
   var config = new Config(fileStorage)
-  var uiStorage = new Storage('sol-ui:')
-  var ui = new Files(uiStorage)
-
-  ui.set('currentFile', '')
 
   // return all the files, except the temporary/readonly ones
   function packageFiles () {
@@ -91,7 +84,7 @@ var run = function () {
     while (files.exists(path + counter)) {
       counter = (counter | 0) + 1
     }
-    return path + counter
+    return path + counter + '.sol'
   }
 
   // Add files received from remote instance (i.e. another browser-solidity)
@@ -184,56 +177,57 @@ var run = function () {
   var editor = new Editor(document.getElementById('input'))
 
   // ---------------- FilePanel --------------------
+  // TODO: All FilePanel related CSS should move into file-panel.js
+  // app.js provides file-panel.js with a css selector or a DOM element
+  // and file-panel.js adds its elements (including css), see "Editor" above
   var css = csjs`
-    .filepanel    {
+    .filepanel-container    {
       display     : flex;
       width       : 200px;
     }
   `
-  var filepanel = document.querySelector('#filepanel')
-  filepanel.className = css.filepanel
+  var filepanelContainer = document.querySelector('#filepanel')
+  filepanelContainer.className = css['filepanel-container']
   var FilePanelAPI = {
     createName: createNonClashingName,
     switchToFile: switchToFile,
     event: this.event
   }
-  var el = new FilePanel(FilePanelAPI, files)
-  filepanel.appendChild(el)
-  var api = el.api
+  var filePanel = new FilePanel(FilePanelAPI, files)
+  // TODO this should happen inside file-panel.js
+  filepanelContainer.appendChild(filePanel)
 
-  api.register('ui', function changeLayout (data) {
+  filePanel.events.register('ui-hidden', function changeLayout (isHidden) {
     var value
-    if (data.type === 'minimize') {
+    if (isHidden) {
       value = -parseInt(window['filepanel'].style.width)
       value = (isNaN(value) ? -window['filepanel'].getBoundingClientRect().width : value)
       window['filepanel'].style.position = 'absolute'
       window['filepanel'].style.left = (value - 5) + 'px'
       window['filepanel'].style.width = -value + 'px'
       window['tabs-bar'].style.left = '45px'
-    } else if (data.type === 'maximize') {
+    } else {
       value = -parseInt(window['filepanel'].style.left) + 'px'
       window['filepanel'].style.position = 'static'
       window['filepanel'].style.width = value
       window['filepanel'].style.left = ''
       window['tabs-bar'].style.left = value
-    } else {
-      window['filepanel'].style.width = data.width + 'px'
-      window['tabs-bar'].style.left = data.width + 'px'
     }
   })
-  api.register('focus', function (path) {
-    [...window.files.querySelectorAll('.file .name')].forEach(function (span) {
-      if (span.innerText === path) switchToFile(path)
-    })
+  filePanel.events.register('ui-resize', function changeLayout (width) {
+    window['filepanel'].style.width = width + 'px'
+    window['tabs-bar'].style.left = width + 'px'
   })
   files.event.register('fileRenamed', function (oldName, newName) {
+    // TODO please never use 'window' when it is possible to use a variable
+    // that references the DOM node
     [...window.files.querySelectorAll('.file .name')].forEach(function (span) {
       if (span.innerText === oldName) span.innerText = newName
     })
   })
   files.event.register('fileRemoved', function (path) {
-    if (path === ui.get('currentFile')) {
-      ui.set('currentFile', '')
+    if (path === config.get('currentFile')) {
+      config.set('currentFile', '')
       switchToNextFile()
     }
     editor.discard(path)
@@ -338,7 +332,7 @@ var run = function () {
         if (!files.rename(originalName, newName)) {
           alert('Error while renaming file')
         } else {
-          ui.set('currentFile', '')
+          config.set('currentFile', '')
           switchToFile(newName)
           editor.discard(originalName)
         }
@@ -368,7 +362,7 @@ var run = function () {
   function switchToFile (file) {
     editorSyncFile()
 
-    ui.set('currentFile', file)
+    config.set('currentFile', file)
 
     if (files.isReadOnly(file)) {
       editor.openReadOnly(file, files.get(file))
@@ -385,7 +379,12 @@ var run = function () {
     }
   }
 
-  switchToNextFile()
+  var previouslyOpenedFile = config.get('currentFile')
+  if (previouslyOpenedFile && files.get(previouslyOpenedFile)) {
+    switchToFile(previouslyOpenedFile)
+  } else {
+    switchToNextFile()
+  }
 
   // Synchronise tab list with file names known to the editor
   function refreshTabs () {
@@ -399,10 +398,10 @@ var run = function () {
       $filesEl.append($('<li class="file"><span class="name">' + name + '</span><span class="remove"><i class="fa fa-close"></i></span></li>'))
     }
 
-    var currentFileOpen = !!ui.get('currentFile')
+    var currentFileOpen = !!config.get('currentFile')
 
     if (currentFileOpen) {
-      var active = $('#files .file').filter(function () { return $(this).find('.name').text() === ui.get('currentFile') })
+      var active = $('#files .file').filter(function () { return $(this).find('.name').text() === config.get('currentFile') })
       active.addClass('active')
     }
     $('#input').toggle(currentFileOpen)
@@ -640,7 +639,7 @@ var run = function () {
       this.fullLineMarker = null
       if (lineColumnPos) {
         var source = compiler.lastCompilationResult.data.sourceList[location.file] // auto switch to that tab
-        if (ui.get('currentFile') !== source) {
+        if (config.get('currentFile') !== source) {
           switchToFile(source)
         }
         this.statementMarker = editor.addMarker(lineColumnPos, 'highlightcode')
@@ -764,12 +763,12 @@ var run = function () {
 
   var rendererAPI = {
     error: (file, error) => {
-      if (file === ui.get('currentFile')) {
+      if (file === config.get('currentFile')) {
         editor.addAnnotation(error)
       }
     },
     errorClick: (errFile, errLine, errCol) => {
-      if (errFile !== ui.get('currentFile') && files.exists(errFile)) {
+      if (errFile !== config.get('currentFile') && files.exists(errFile)) {
         switchToFile(errFile)
       }
       editor.gotoLine(errLine, errCol)
@@ -821,7 +820,7 @@ var run = function () {
     if (transactionDebugger.isActive) return
 
     editorSyncFile()
-    var currentFile = ui.get('currentFile')
+    var currentFile = config.get('currentFile')
     if (currentFile) {
       var target = currentFile
       var sources = {}
@@ -831,8 +830,8 @@ var run = function () {
   }
 
   function editorSyncFile () {
-    var currentFile = ui.get('currentFile')
-    if (currentFile) {
+    var currentFile = config.get('currentFile')
+    if (currentFile && editor.current()) {
       var input = editor.get(currentFile)
       files.set(currentFile, input)
     }
@@ -843,7 +842,7 @@ var run = function () {
   var saveTimeout = null
 
   function editorOnChange () {
-    var currentFile = ui.get('currentFile')
+    var currentFile = config.get('currentFile')
     if (!currentFile) {
       return
     }

src/app/file-explorer.js

@@ -20,12 +20,9 @@ var css = csjs`
     background-color  : white;
   }
   .remove             {
-    align-self        : center;
-    padding-left      : 10px;
+    float             : right;
   }
   .activeMode         {
-    display           : flex;
-    justify-content   : space-between;
     margin-right      : 10px;
     padding-right     : 19px;
   }
@@ -37,7 +34,7 @@ module.exports = fileExplorer
 
 function fileExplorer (appAPI, files) {
   var fileEvents = files.event
-  var tv = new Treeview({
+  var treeView = new Treeview({
     extractData: function (value, tree, key) {
       var newValue = {}
       // var isReadOnly = false
@@ -86,10 +83,11 @@ function fileExplorer (appAPI, files) {
   var focusElement = null
   var textUnderEdit = null
 
-  var element = tv.render(files.listAsTree())
+  var element = treeView.render(files.listAsTree())
   element.className = css.fileexplorer
 
-  var api = new EventManager()
+  var events = new EventManager()
+  var api = {}
   api.addFile = function addFile (file) {
     var name = file.name
     if (!files.exists(name) || confirm('The file ' + name + ' already exists! Would you like to overwrite it?')) {
@@ -97,7 +95,7 @@ function fileExplorer (appAPI, files) {
       fileReader.onload = function (event) {
         var success = files.set(name, event.target.result)
         if (!success) alert('Failed to create file ' + name)
-        else api.trigger('focus', [name])
+        else events.trigger('focus', [name])
       }
       fileReader.readAsText(file)
     }
@@ -113,7 +111,7 @@ function fileExplorer (appAPI, files) {
     var label = getLabelFrom(li)
     var filepath = label.dataset.path
     var isFile = label.className.indexOf('file') === 0
-    if (isFile) api.trigger('focus', [filepath])
+    if (isFile) events.trigger('focus', [filepath])
   }
 
   function hover (event) {
@@ -247,12 +245,13 @@ function fileExplorer (appAPI, files) {
   }
 
   function fileAdded (filepath) {
-    var el = tv.render(files.listAsTree())
+    var el = treeView.render(files.listAsTree())
     el.className = css.fileexplorer
     element.parentElement.replaceChild(el, element)
     element = el
   }
 
+  element.events = events
   element.api = api
   return element
 }

src/app/file-panel.js

@@ -106,11 +106,13 @@ function filepanel (appAPI, files) {
     `
   }
 
-  var api = new EventManager()
+  var events = new EventManager()
   var element = template()
-  element.api = api
-  fileExplorer.api.register('focus', function (path) {
-    api.trigger('focus', [path])
+  // TODO please do not add custom javascript objects, which have no
+  // relation to the DOM to DOM nodes
+  element.events = events
+  fileExplorer.events.register('focus', function (path) {
+    appAPI.switchToFile(path)
   })
 
   return element
@@ -120,10 +122,14 @@ function filepanel (appAPI, files) {
     this.classList.toggle(css.isVisible)
     this.children[0].classList.toggle('fa-angle-double-right')
     this.children[0].classList.toggle('fa-angle-double-left')
-    api.trigger('ui', [{ type: isHidden ? 'minimize' : 'maximize' }])
+    events.trigger('ui-hidden', [isHidden])
   }
 
   function uploadFile (event) {
+    // TODO The file explorer is merely a view on the current state of
+    // the files module. Please ask the user here if they want to overwrite
+    // a file and then just use `files.add`. The file explorer will
+    // pick that up via the 'fileAdded' event from the files module.
     ;[...this.files].forEach(fileExplorer.api.addFile)
   }
 
@@ -159,7 +165,7 @@ function filepanel (appAPI, files) {
     document.removeEventListener('keydown', cancelGhostbar)
     var width = (event.pageX < limit) ? limit : event.pageX
     element.style.width = width + 'px'
-    api.trigger('ui', [{ type: 'resize', width: width }])
+    events.trigger('ui-resize', [width])
   }
 
   function createNewFile () {

test-browser/tests/ballot.js

@@ -6,7 +6,7 @@ var sauce = require('./sauce')
 
 var sources = {
   'sources': {
-    'Untitled': examples.ballot.content
+    'Untitled.sol': examples.ballot.content
   }
 }
 
@@ -27,7 +27,7 @@ function runTests (browser, testData) {
   browser
     .waitForElementVisible('.newFile', 10000)
     .click('.envView')
-  contractHelper.testContracts(browser, sources.sources.Untitled, ['Untitled:Ballot'], function () {
+  contractHelper.testContracts(browser, sources.sources['Untitled.sol'], ['Untitled.sol:Ballot'], function () {
     browser.end()
   })
 }

test-browser/tests/compiling.js

@@ -5,7 +5,7 @@ var sauce = require('./sauce')
 
 var sources = {
   'sources': {
-    'Untitled': `pragma solidity ^0.4.0;
+    'Untitled.sol': `pragma solidity ^0.4.0;
       contract TestContract { function f() returns (uint) { return 8; } }`
   }
 }
@@ -27,7 +27,7 @@ function runTests (browser) {
   browser
     .waitForElementVisible('.newFile', 10000)
     .click('.envView')
-  contractHelper.testContracts(browser, sources.sources.Untitled, ['Untitled:TestContract'], function () {
+  contractHelper.testContracts(browser, sources.sources['Untitled.sol'], ['Untitled.sol:TestContract'], function () {
     browser.click('.create .constructor .call')
       .waitForElementPresent('.instance .call[title="f"]')
       .click('.instance .call[title="f"]')

test-browser/tests/simpleContract.js

@@ -5,7 +5,7 @@ var sauce = require('./sauce')
 
 var sources = {
   'sources': {
-    'Untitled': 'contract test1 {} contract test2 {}'
+    'Untitled.sol': 'contract test1 {} contract test2 {}'
   }
 }
 
@@ -26,7 +26,7 @@ function runTests (browser) {
   browser
     .waitForElementVisible('.newFile', 10000)
     .click('.envView')
-  contractHelper.testContracts(browser, sources.sources.Untitled, ['Untitled:test1', 'Untitled:test2'], function () {
+  contractHelper.testContracts(browser, sources.sources['Untitled.sol'], ['Untitled.sol:test1', 'Untitled.sol:test2'], function () {
     browser.end()
   })
 }

test-browser/tests/staticanalysis.js

@@ -6,7 +6,7 @@ var dom = require('../helpers/dom')
 
 var sources = {
   'sources': {
-    'Untitled': `
+    'Untitled.sol': `
 contract test1 { address test = tx.origin; }
 contract test2 {}
 contract TooMuchGas {
@@ -33,13 +33,13 @@ function runTests (browser) {
   browser
     .waitForElementVisible('.newFile', 10000)
     .click('.envView')
-  contractHelper.testContracts(browser, sources.sources.Untitled, ['Untitled:TooMuchGas', 'Untitled:test1', 'Untitled:test2'], function () {
+  contractHelper.testContracts(browser, sources.sources['Untitled.sol'], ['Untitled.sol:TooMuchGas', 'Untitled.sol:test1', 'Untitled.sol:test2'], function () {
     browser
       .click('.staticanalysisView')
       .click('#staticanalysisView button')
       .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () {
-        dom.listSelectorContains(['Untitled:2:33: use of tx.origin',
-          'Fallback function of contract Untitled:TooMuchGas requires too much gas'],
+        dom.listSelectorContains(['Untitled.sol:2:33: use of tx.origin',
+          'Fallback function of contract Untitled.sol:TooMuchGas requires too much gas'],
           '#staticanalysisresult .warning span',
           browser, function () {
             browser.end()