-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removing (most) Closure from BlockFactory #1925
Changes from all commits
bee387d
fa2ebdb
2790f8e
d7b418c
412bc46
81c3641
a3a701f
bf12e66
44b97bd
4fc83f2
66c4fe0
1817ea1
1a1228f
5d6e247
01578ff
e8c50a7
e443970
7d19f2f
4cd4f3b
99fb78d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,25 +28,15 @@ | |
*/ | ||
'use strict'; | ||
|
||
goog.provide('BlockExporterTools'); | ||
|
||
goog.require('FactoryUtils'); | ||
goog.require('BlockOption'); | ||
goog.require('goog.dom'); | ||
goog.require('goog.dom.xml'); | ||
|
||
|
||
/** | ||
* Block Exporter Tools Class | ||
* @constructor | ||
*/ | ||
BlockExporterTools = function() { | ||
function BlockExporterTools() { | ||
// Create container for hidden workspace. | ||
this.container = goog.dom.createDom('div', { | ||
'id': 'blockExporterTools_hiddenWorkspace' | ||
}, ''); // Empty quotes for empty div. | ||
// Hide hidden workspace. | ||
this.container.style.display = 'none'; | ||
this.container = document.createElement('div'); | ||
this.container.id = 'blockExporterTools_hiddenWorkspace'; | ||
this.container.style.display = 'none'; // Hide the hidden workspace. | ||
document.body.appendChild(this.container); | ||
/** | ||
* Hidden workspace for the Block Exporter that holds pieces that make | ||
|
@@ -167,45 +157,6 @@ BlockExporterTools.prototype.addBlockDefinitions = function(blockXmlMap) { | |
eval(blockDefs); | ||
}; | ||
|
||
/** | ||
* Pulls information about all blocks in the block library to generate XML | ||
* for the selector workpace's toolbox. | ||
* @param {!BlockLibraryStorage} blockLibStorage Block Library Storage object. | ||
* @return {!Element} XML representation of the toolbox. | ||
*/ | ||
BlockExporterTools.prototype.generateToolboxFromLibrary | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. was this unused? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
= function(blockLibStorage) { | ||
// Create DOM for XML. | ||
var xmlDom = goog.dom.createDom('xml', { | ||
'id' : 'blockExporterTools_toolbox', | ||
'style' : 'display:none' | ||
}); | ||
|
||
var allBlockTypes = blockLibStorage.getBlockTypes(); | ||
// Object mapping block type to XML. | ||
var blockXmlMap = blockLibStorage.getBlockXmlMap(allBlockTypes); | ||
|
||
// Define the custom blocks in order to be able to create instances of | ||
// them in the exporter workspace. | ||
this.addBlockDefinitions(blockXmlMap); | ||
|
||
for (var blockType in blockXmlMap) { | ||
// Get block. | ||
var block = FactoryUtils.getDefinedBlock(blockType, this.hiddenWorkspace); | ||
var category = FactoryUtils.generateCategoryXml([block], blockType); | ||
xmlDom.appendChild(category); | ||
} | ||
|
||
// If there are no blocks in library and the map is empty, append dummy | ||
// category. | ||
if (Object.keys(blockXmlMap).length == 0) { | ||
var category = goog.dom.createDom('category'); | ||
category.setAttribute('name','Next Saved Block'); | ||
xmlDom.appendChild(category); | ||
} | ||
return xmlDom; | ||
}; | ||
|
||
/** | ||
* Generate XML for the workspace factory's category from imported block | ||
* definitions. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,17 +27,14 @@ | |
|
||
'use strict'; | ||
|
||
goog.provide('BlockLibraryStorage'); | ||
|
||
|
||
/** | ||
* Represents a block library's storage. | ||
* @param {string} blockLibraryName Desired name of Block Library, also used | ||
* to create the key for where it's stored in local storage. | ||
* @param {Object} opt_blocks Object mapping block type to XML. | ||
* @constructor | ||
*/ | ||
BlockLibraryStorage = function(blockLibraryName, opt_blocks) { | ||
function BlockLibraryStorage(blockLibraryName, opt_blocks) { | ||
// Add prefix to this.name to avoid collisions in local storage. | ||
this.name = 'BlockLibraryStorage.' + blockLibraryName; | ||
if (!opt_blocks) { | ||
|
@@ -60,17 +57,15 @@ BlockLibraryStorage = function(blockLibraryName, opt_blocks) { | |
* Reads the named block library from local storage and saves it in this.blocks. | ||
*/ | ||
BlockLibraryStorage.prototype.loadFromLocalStorage = function() { | ||
// goog.global is synonymous to window, and allows for flexibility | ||
// between browsers. | ||
var object = goog.global.localStorage[this.name]; | ||
var object = localStorage[this.name]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. none of the browsers require this to be window.? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. window is the global scope in a browser context: https://stackoverflow.com/a/12660102/152543 |
||
this.blocks = object ? JSON.parse(object) : null; | ||
}; | ||
|
||
/** | ||
* Writes the current block library (this.blocks) to local storage. | ||
*/ | ||
BlockLibraryStorage.prototype.saveToLocalStorage = function() { | ||
goog.global.localStorage[this.name] = JSON.stringify(this.blocks); | ||
localStorage[this.name] = JSON.stringify(this.blocks); | ||
}; | ||
|
||
/** | ||
|
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.
Are these TODOs actually done now?
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.
#1247 was solved by #1293, which added the
.getHue()
method.