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-7227] Complete Ti.UI.Window implementation #1270

Merged
merged 6 commits into from Jan 26, 2012
Merged

[TIMOB-7227] Complete Ti.UI.Window implementation #1270

merged 6 commits into from Jan 26, 2012

Conversation

cb1kenobi
Copy link
Contributor

No description provided.

…n. Added mobileweb support to project creation scripts. Added support for favicons. Commented out old stuff from compiler.py including i18n support. Some code cleanup. Updated credits.
…p images. Fixed more back button issues. Dropped support for id from Ti.Platform.
@billdawson
Copy link
Contributor

titanium.py & project.py review: CR and FR accepted. I tested new titanium.py (which calls project.py) and successfully created: a) a mobileweb-only project; and b) a project with iphone+android+mobileweb.

To reiterate, scope of this review was just those two files.

@@ -40,7 +40,7 @@ methods:
type: String
- name: stopLoading
summary: stop loading a currently loading page
note: For mobile web, due to browser permissions, this will issue a stop on all windows include the parent and all other webviews.
note: For Mobile Web, due to browser permissions, this will issue a stop on all windows include the parent and all other webviews.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be description:

description: |
For Mobile Web, due to browser permissions, this will issue a stop on all windows including the parent and all other web views.

Cap Mobile Web, s/include/including/
Um, should we note that this would be a Really Bad Thing to do? Because it sounds like a Really Bad Thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed typo.

It's not a really bad thing to do. Generally the stop button just stops the page loading. It doesn't halt the JavaScript already running in the page. Because of the way mobile web apps work, by the time a WebView is even created, the page is loaded and stop would effectively do nothing.

@arthurevans
Copy link
Contributor

Reviewed docs. 1 doc problem needs fixing.

@billdawson
Copy link
Contributor

Chris I forgot to mention: the mode of titanium.py got changed to 644 from 755. It should go back, unless you did it on purpose?

@arthurevans
Copy link
Contributor

Hi Chris--

The more important part of that comment was that the line started with note: which is not a valid TDoc tag. What you want here is a description.

@cb1kenobi
Copy link
Contributor Author

Arthur, fixed typo.

@cb1kenobi
Copy link
Contributor Author

Bill, you are correct. I had a bit of a problem when I was working on it and never meant to change the mode. I have changed it back to executable.

@billdawson
Copy link
Contributor

Titanium.py, re-approved (in case that was necessary ;) )

@arthurevans
Copy link
Contributor

Ran docgen and validate, checked output. Documentation approved.

mobileweb_resources = os.path.join(resources_dir,'mobileweb')
if not os.path.exists(mobileweb_resources): os.makedirs(mobileweb_resources)
mobileweb_gen = os.path.join(template_dir,'mobileweb','mobileweb.py')
run([sys.executable, mobileweb_gen, name, appid, directory])
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is how the other platforms do things, but why are we running these in a separate process? mobileweb.py can just be imported and run by calling it's methods directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When in Rome...

@nebrius
Copy link
Contributor

nebrius commented Jan 26, 2012

Code reviewed and tested. Request accepted.

nebrius added a commit that referenced this pull request Jan 26, 2012
[TIMOB-7227] Complete Ti.UI.Window implementation
@nebrius nebrius merged commit b3c9ee5 into tidev:master Jan 26, 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants