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.

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

TIMOB-9539: BlackBerry: Implement include() #91

Merged

Conversation

alexandergalstyan
Copy link

Reviewers: DavidC, DavidL

ChangeLog:

  • Added include functionality to Ti namespace.

Test Cases:

  • Run tibbtest.
  • Make sure include_test.js included in bar-descriptor.xml
  • Make sure Ti.include() works correctly.
  • Use the following app.js snippet:

////////////////////////////////////////
:app.js
var win1 = Titanium.UI.createWindow({
backgroundColor:'#F00'
});

var label1 = Ti.UI.createLabel({
text:'Hello, world!',
color:'green',
top: 200
});

win1.add(label1);

Ti.include('include_test.js');

// open window
win1.open();
////////////////////////////////////////
:include_test.js
var mybutton=Ti.UI.createButton
(
{top: 100,title: 'Push Me'}
);

mybutton.addEventListener
(
'click',
function(e)
{
e.source.title='Pushed!';
slider1.backgroundColor='#00F';
progress1.backgroundColor='#0F0';
}
);

win1.add(mybutton);

Reviewers: DavidC, DavidL

ChangeLog:
- Added include functionality to Ti namespace.

Test Cases:
- Run tibbtest.
- Make sure include_test.js included in bar-descriptor.xml
- Make sure Ti.include(<filename>) works correctly.
- Use the following app.js snippet:

////////////////////////////////////////
:app.js
var win1 = Titanium.UI.createWindow({
    backgroundColor:'#F00'
});

var label1 = Ti.UI.createLabel({
	text:'Hello, world!',
	color:'green',
	top: 200
});

win1.add(label1);

Ti.include('include_test.js');

// open window
win1.open();
////////////////////////////////////////
:include_test.js
var mybutton=Ti.UI.createButton
(
	{top: 100,title: 'Push Me'}
);

mybutton.addEventListener
(
	'click',
	function(e)
	{
		e.source.title='Pushed!';
		slider1.backgroundColor='#00F';
		progress1.backgroundColor='#0F0';
	}
);

win1.add(mybutton);
}

string fileName = "app/native/assets/";
fileName += *String::Utf8Value(javaScript);

Choose a reason for hiding this comment

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

This does not fulfil the contract of being "interpreted local to the current file" as specified in the docs.

Choose a reason for hiding this comment

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

We also need to make sure this gets run in the proper context.

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@dlifshitz-maca
Copy link

Reviewed

Reviewers: DavidC, DavidL

ChangeLog:
- Address comments.
- Making include filename interpreted local to the current file.

Test Cases:
 •Run tibbtest.
 •Make sure include_test.js included in bar-descriptor.xml
 •Make sure Ti.include() works correctly.
 •Use the following app.js snippet (including include_test.js and include_test1.js):

////////////////////////////////////////
 :app.js
 var win1 = Titanium.UI.createWindow({
 backgroundColor:'#F00'
 });

var label1 = Ti.UI.createLabel({
 text:'Hello, world!',
 color:'green',
 top: 200
 });

win1.add(label1);

Ti.include('include_test.js');

// open window
 win1.open();
 ////////////////////////////////////////
 :include_test.js
var mybutton=Ti.UI.createButton
(
	{top: 100,title: 'Push Me'}
);

mybutton.addEventListener
(
	'click',
	function(e)
	{
		e.source.title='Pushed!';
		slider1.backgroundColor='#00F';
		progress1.backgroundColor='#0F0';
	}
);

Ti.include('include_test1.js');

win1.add(mybutton);
 ////////////////////////////////////////
 :include_test1.js
var slider1=Ti.UI.createSlider
(
	{
		top: 200,
		min: 0,
		max: 100,
		value: 50,
	}
);

var sliderListener=function(e)
{
	progress1.value=100-e.value;
	label1.text='Slider value: '+e.value;
	label1.top=e.value;
	mybutton.opacity=e.value/100.0;
};

slider1.addEventListener('change',sliderListener);

win1.add(slider1);
@alexandergalstyan
Copy link
Author

Updated.

{
String::Utf8Value error(tryCatch.Exception());
TiLogger::getInstance().log(string(*error) + "\n");
return ThrowException(tryCatch.Exception());

Choose a reason for hiding this comment

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

This seems like duplicated effort. Won't ThrowException display the error in the log?

Choose a reason for hiding this comment

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

You should add the name of the file that can't be found to the exception.

Copy link
Author

Choose a reason for hiding this comment

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

Remove 1st logging.

@dlifshitz-maca
Copy link

Reviewed


ifstream ifs(fullPath.c_str());

if (ifs.bad())

Choose a reason for hiding this comment

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

I just realized this won't be true if the file does not exist. Use fail() instead, which also checks the badbit.

Copy link
Author

Choose a reason for hiding this comment

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

Done with !ifs.

@dcampbell-macadamian
Copy link

Reviewed

Reviewers: DavidC, DavidL

ChangeLog:
- Address comments.

Test Cases:
- Run tibbtest.
- Make sure include_test.js included in bar-descriptor.xml
- Make sure Ti.include() works correctly.
- Use the following app.js snippet:

////////////////////////////////////////
 :app.js
 var win1 = Titanium.UI.createWindow({
 backgroundColor:'#F00'
 });

var label1 = Ti.UI.createLabel({
 text:'Hello, world!',
 color:'green',
 top: 200
 });

win1.add(label1);

Ti.include('include_test.js');

// open window
 win1.open();
 ////////////////////////////////////////
 :include_test.js
 var mybutton=Ti.UI.createButton
 (
 {top: 100,title: 'Push Me'}
 );

mybutton.addEventListener
 (
 'click',
 function(e)
 {
 e.source.title='Pushed!';
 slider1.backgroundColor='#00F';
 progress1.backgroundColor='#0F0';
 }
 );

win1.add(mybutton);
@alexandergalstyan
Copy link
Author

Updated.

@dlifshitz-maca
Copy link

Approved

1 similar comment
@dcampbell-macadamian
Copy link

Approved

…erry' into includeBranch

Conflicts:
	blackberry/tibb/TiMessageStrings.h
alexandergalstyan added a commit that referenced this pull request Jul 9, 2012
TIMOB-9539: BlackBerry: Implement include()
@alexandergalstyan alexandergalstyan merged commit 4ea1b23 into Macadamian:blackberry Jul 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants