Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Pass Node ID string to run(). #152

Merged
merged 7 commits into from
Nov 29, 2012
Merged

Pass Node ID string to run(). #152

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2fdaab0
Pass Node ID string to run(). #109
iamdustan Nov 14, 2012
6411946
A test
iamdustan Nov 14, 2012
cb6ed0f
Tests pass
iamdustan Nov 14, 2012
31b3c35
Move node string check to `run`
iamdustan Nov 15, 2012
6e484d8
Update changelog
iamdustan Nov 15, 2012
05ca105
Refactor to pass node id string through to the renderer
iamdustan Nov 29, 2012
bd22c5f
Fix test
iamdustan Nov 29, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ v0.4.2
DisplayObject
* Added an destroy event for DisplayObjects, which is fired on a displayobject when
destroy is called on it.
* Allow for an ID string to be passed to as the first argument to `run`

v0.4.1 / 2012-10-26
-------------------
Expand Down
3 changes: 1 addition & 2 deletions src/bootstrapper/player.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function(EventEmitter, RendererController, AssetController, tools, URI, version)
/**
* Loads a bonsai movie and embeds it into a HTML document.
*
* @param {HTMLElement} node The html element to replace with the movie
* @param {HTMLElement|String} node The html element or DOM id to inject the movie into
* @param {string} url The URL to the bonsai script to load
* @param {Number} [options.width] The width of the movie
* @param {Number} [options.height] The height of the movie
Expand All @@ -84,7 +84,6 @@ function(EventEmitter, RendererController, AssetController, tools, URI, version)
* @returns {Movie}
*/
run: function(node, url, options) {

if (url && typeof url != 'string') {
options = url;
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/renderer/svg/svg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ define([
* The SvgRenderer constructor
*
* @constructor
* @param {HTMLElement} node The element to append the svg root node to.
* @param {HTMLElement|String} node The element or element id to append the svg root node to.
* @param {number} width The width to apply to the svg root node.
* Falsy means 'no width applied'.
* @param {number} height The height to apply to the svg root node.
Expand All @@ -208,6 +208,11 @@ define([
* with the framerate.
*/
function SvgRenderer(node, width, height, options) {

if (typeof node === 'string') {
node = document.getElementById(node);
}

options = options || {};
this.width = width;
this.height = height;
Expand Down
10 changes: 10 additions & 0 deletions test/player-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ define([
expect(args[3]).toHaveProperties('baseUrl');
});


describe('defaultRunnerOptions', function() {
var originalOptions;
beforeEach(function() {
Expand Down Expand Up @@ -271,6 +272,15 @@ define([
return player.run(node, options);
})();


it('passes a string when given a DOM id to the renderer', function () {
var node = 'node', width = 100, height = 100, options = { width: width, height: height };

player.run(node, options);

expect(MockRendererConstructor).toHaveBeenCalledWith(node, width, height, {});
});

});

});
Expand Down
10 changes: 10 additions & 0 deletions test/renderer/svg-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,16 @@ define([
return new SvgRenderer(createFakeDomNode(), 1, 1);
}

it('should accept a dom id for the node argument', function () {
spyOn(document, 'getElementById').andReturn(createFakeDomNode());
spyOn(SvgRenderer, 'Svg');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was hoping that this line would work for the new Svg line in the SvgRenderer constructor, but it didn’t fly. I got it to work by making the new Svg line say new SvgRenderer.Svg but then there were other integration issues with L227 svg.root.addEventListener

Any tips?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I’d just test whether the append() method of the fake node has been called with the root node of the movie. E.g. like this:

var targetNode = createFakeDomNode();
spyOn(targetNode, 'appendChild');
spyOn(document, 'getElementById').andReturn(targetNode);
var renderer = new SvgRenderer('thing', 1, 1);

expect(document.getElementById).toHaveBeenCalledWith('thing');
expect(targetNode.appendChild.).toHaveBeenCalledWith(renderer.svg.rootContainer);

That should do the trick


new SvgRenderer('thing', 1, 1);

expect( SvgRenderer.Svg ).toHaveBeenCalledWith( createFakeDomNode(), 1, 1);
});


describe('allowEventDefaults', function() {
it('should assign the constructor value as property', function() {
expect(new SvgRenderer(createFakeDomNode(), 1, 1, {
Expand Down