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

[TIMOB-16436] Integrate appcompat themes to SDK #5348

Merged
merged 8 commits into from
Feb 20, 2014
Merged

[TIMOB-16436] Integrate appcompat themes to SDK #5348

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bbf23c5
timob-16436: Integrate appcompat themes to SDK
hieupham007 Feb 12, 2014
0bf19dd
timob-16436: Integrate ActionBarActivity
hieupham007 Feb 13, 2014
3b5881b
timob-16436: addressed comments
hieupham007 Feb 13, 2014
e4c4c84
timob-16436: hack for actionbar 4.0+
hieupham007 Feb 14, 2014
4e8d5bc
timob-16436: menu fix.
hieupham007 Feb 18, 2014
8619d48
timob-16436: fix showAsAction, invalidateOptionsMenu, setLogo, and se…
hieupham007 Feb 19, 2014
2c06b05
timob-16436: addressed comments
hieupham007 Feb 20, 2014
13c36f4
timob-16436: Added more functionalities to 2.x ActionBar
hieupham007 Feb 20, 2014
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
6 changes: 3 additions & 3 deletions android/cli/commands/_build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3277,7 +3277,7 @@ AndroidBuilder.prototype.generateTheme = function generateTheme(next) {
if (!fs.existsSync(themeFile)) {
this.logger.info(__('Generating %s', themeFile.cyan));

var flags = 'Theme';
var flags = 'Theme.AppCompat';
if ((this.tiapp.fullscreen || this.tiapp['statusbar-hidden']) && this.tiapp['navbar-hidden']) {
flags += '.NoTitleBar.Fullscreen';
} else if (this.tiapp['navbar-hidden']) {
Expand Down Expand Up @@ -3362,15 +3362,15 @@ AndroidBuilder.prototype.generateAndroidManifest = function generateAndroidManif
'activity': {
'name': 'ti.modules.titanium.media.TiVideoActivity',
'configChanges': ['keyboardHidden', 'orientation'],
'theme': '@android:style/Theme.NoTitleBar.Fullscreen',
'theme': '@style/Theme.AppCompat.NoTitleBar.Fullscreen',
'launchMode': 'singleTask'
}
},
'Media.showCamera': {
'activity': {
'name': 'ti.modules.titanium.media.TiCameraActivity',
'configChanges': ['keyboardHidden', 'orientation'],
'theme': '@android:style/Theme.Translucent.NoTitleBar.Fullscreen'
'theme': '@style/Theme.AppCompat.Translucent.NoTitleBar.Fullscreen'
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion android/modules/android/src/java/ti/modules/titanium/android/AndroidModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import android.app.Notification;
import android.app.PendingIntent;
import android.app.Service;
import android.app.ActionBar;
import android.support.v7.app.ActionBar;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
Expand Down
23 changes: 8 additions & 15 deletions android/modules/ui/src/java/ti/modules/titanium/ui/TabGroupProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import ti.modules.titanium.ui.widget.tabgroup.TiUIAbstractTabGroup;
import ti.modules.titanium.ui.widget.tabgroup.TiUIActionBarTabGroup;
import ti.modules.titanium.ui.widget.tabgroup.TiUITabHostGroup;
import android.app.Activity;
import android.support.v7.app.ActionBarActivity;
import android.content.Intent;
import android.os.Build;
import android.os.Message;
Expand All @@ -52,7 +52,7 @@ public class TabGroupProxy extends TiWindowProxy implements TiActivityWindow
protected static final int MSG_LAST_ID = MSG_FIRST_ID + 999;

private ArrayList<TabProxy> tabs = new ArrayList<TabProxy>();
private WeakReference<Activity> tabGroupActivity;
private WeakReference<ActionBarActivity> tabGroupActivity;
private TabProxy selectedTab;
private boolean isFocused;

Expand Down Expand Up @@ -287,7 +287,7 @@ private TabProxy handleGetActiveTab() {
@Override
protected void handleOpen(KrollDict options)
{
Activity topActivity = TiApplication.getAppCurrentActivity();
ActionBarActivity topActivity = (ActionBarActivity) TiApplication.getAppCurrentActivity();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to cast this to an ActionBarActivity? We don't do anything specific with ActionBarActivity. Also not sure if TiApplication.getAppCurrentActivity(); may return some other type of activity or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we don't need to cast it here, but since this entire class is using ActionBarActivity, not casting it would require an additional import of android.app.Activity, which would serve no purpose other than saving a cast.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well you would be risking a class cast exception if TiApplication.getAppCurrentActivity(); doesn't return an ActionBarActivity. It may be worth it to not cast instead of saving an import.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add an instanceof check here. If its not ActiionBarActivity, it will crash at a later time.

Intent intent = new Intent(topActivity, TiActivity.class);
fillIntent(topActivity, intent);

Expand All @@ -300,19 +300,12 @@ protected void handleOpen(KrollDict options)

@Override
public void windowCreated(TiBaseActivity activity) {
tabGroupActivity = new WeakReference<Activity>(activity);
tabGroupActivity = new WeakReference<ActionBarActivity>(activity);
activity.setWindowProxy(this);
activity.setLayoutProxy(this);
setActivity(activity);

// Use the navigation tabs if this platform supports the action bar.
// Otherwise we will fall back to using the TabHost implementation.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.HONEYCOMB && activity.getActionBar() != null) {
view = new TiUIActionBarTabGroup(this, activity);

} else {
view = new TiUITabHostGroup(this, activity);
}
view = new TiUIActionBarTabGroup(this, activity);
Copy link
Contributor

Choose a reason for hiding this comment

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

You still might want to check that the activity.getSupportActionBar() is not null and create TabHost if it is null


setModelListener(view);

Expand Down Expand Up @@ -371,7 +364,7 @@ protected void handleClose(KrollDict options)
releaseViews();
view = null;

Activity activity = tabGroupActivity.get();
ActionBarActivity activity = tabGroupActivity.get();
if (activity != null && !activity.isFinishing()) {
activity.finish();
}
Expand Down Expand Up @@ -446,7 +439,7 @@ public void onTabSelected(TabProxy tabProxy) {
selectedTab.onFocusChanged(true, focusEventData);
}

private void fillIntent(Activity activity, Intent intent)
private void fillIntent(ActionBarActivity activity, Intent intent)
{
if (hasProperty(TiC.PROPERTY_FULLSCREEN)) {
intent.putExtra(TiC.PROPERTY_FULLSCREEN, TiConvert.toBoolean(getProperty(TiC.PROPERTY_FULLSCREEN)));
Expand Down Expand Up @@ -502,7 +495,7 @@ public void releaseViewsForActivityForcedToDestroy()
}

@Override
protected Activity getWindowActivity()
protected ActionBarActivity getWindowActivity()
{
return (tabGroupActivity != null) ? tabGroupActivity.get() : null;
}
Expand Down
5 changes: 2 additions & 3 deletions android/modules/ui/src/java/ti/modules/titanium/ui/widget/tabgroup/TiUIActionBarTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,12 @@
package ti.modules.titanium.ui.widget.tabgroup;

import org.appcelerator.kroll.KrollProxy;
import org.appcelerator.kroll.common.Log;
import org.appcelerator.titanium.TiC;
import org.appcelerator.titanium.util.TiUIHelper;

import ti.modules.titanium.ui.TabProxy;
import android.app.ActionBar;
import android.app.Fragment;
import android.support.v7.app.ActionBar;
import android.support.v4.app.Fragment;
import android.graphics.drawable.Drawable;
import android.os.Bundle;
import android.view.LayoutInflater;
Expand Down
10 changes: 5 additions & 5 deletions ...oid/modules/ui/src/java/ti/modules/titanium/ui/widget/tabgroup/TiUIActionBarTabGroup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@

import ti.modules.titanium.ui.TabGroupProxy;
import ti.modules.titanium.ui.TabProxy;
import android.app.ActionBar;
import android.app.ActionBar.Tab;
import android.app.ActionBar.TabListener;
import android.support.v7.app.ActionBar;
import android.support.v7.app.ActionBar.Tab;
import android.support.v7.app.ActionBar.TabListener;
import android.app.Activity;
import android.app.FragmentTransaction;
import android.support.v4.app.FragmentTransaction;
import android.view.ViewGroup;
import android.widget.FrameLayout;

Expand Down Expand Up @@ -50,7 +50,7 @@ public TiUIActionBarTabGroup(TabGroupProxy proxy, TiBaseActivity activity) {
activity.addOnLifecycleEventListener(this);

// Setup the action bar for navigation tabs.
actionBar = activity.getActionBar();
actionBar = activity.getSupportActionBar();
actionBar.setNavigationMode(ActionBar.NAVIGATION_MODE_TABS);
actionBar.setDisplayShowTitleEnabled(true);

Expand Down
5 changes: 3 additions & 2 deletions android/templates/build/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@

<application android:icon="@drawable/appicon"
android:label="<%- tiapp.name %>" android:name="<%- classname %>Application"
android:debuggable="false">
android:debuggable="false"
android:theme="@style/Theme.AppCompat">
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of applying theme to the application I would instead apply theme to TiActivity.
That way any modules that have their own activities that do not extend TiBaseActivity will continue to work as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

theme to application includes all sub activities that don't have a theme set, so TiActivity will be using the theme set for application as well... It's a good default to have, i.e even new activities from modules will use the application theme as default.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we decide to apply the theme to TiActivity, we will also need to do it for activities like JSActivity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tested and confirmed that AppCompat theme works with non-actionbar activities (i.e activites that do not extend TiBaseActivity), so we should be fine using it as a default theme.


<activity android:name=".<%- classname %>Activity"
android:label="@string/app_name" android:theme="@style/Theme.Titanium"
Expand All @@ -28,7 +29,7 @@
android:configChanges="keyboardHidden|orientation" />
<activity android:name="org.appcelerator.titanium.TiTranslucentActivity"
android:configChanges="keyboardHidden|orientation"
android:theme="@android:style/Theme.Translucent" />
android:theme="@style/Theme.AppCompat.Translucent" />
<activity android:name="ti.modules.titanium.ui.android.TiPreferencesActivity" />

</application>
Expand Down
26 changes: 25 additions & 1 deletion android/templates/build/theme.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,30 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<style name="Theme.Titanium" parent="android:<%- flags %>">
<style name="Theme.AppCompat.NoTitleBar">
<item name="android:windowNoTitle">true</item>
</style>

<style name="Theme.AppCompat.NoTitleBar.Fullscreen">
<item name="android:windowFullscreen">true</item>
<item name="android:windowContentOverlay">@null</item>
</style>

<style name="Theme.Titanium" parent="@style/<%- flags %>">
<item name="android:windowBackground">@drawable/background</item>
</style>

<style name="Theme.AppCompat.Translucent">
<item name="android:windowBackground">@android:color/transparent</item>
<item name="android:colorBackgroundCacheHint">@null</item>
<item name="android:windowIsTranslucent">true</item>
</style>

<style name="Theme.AppCompat.Translucent.NoTitleBar">
<item name="android:windowNoTitle">true</item>
<item name="android:windowContentOverlay">@null</item>
</style>

<style name="Theme.AppCompat.Translucent.NoTitleBar.Fullscreen">
<item name="android:windowFullscreen">true</item>
</style>
</resources>
3 changes: 2 additions & 1 deletion android/titanium/src/java/org/appcelerator/titanium/TiBaseActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import android.os.Messenger;
import android.os.RemoteException;
import android.support.v4.app.FragmentActivity;
import android.support.v7.app.ActionBarActivity;
import android.view.KeyEvent;
import android.view.Menu;
import android.view.MenuItem;
Expand All @@ -61,7 +62,7 @@
* The base class for all non tab Titanium activities. To learn more about Activities, see the
* <a href="http://developer.android.com/reference/android/app/Activity.html">Android Activity documentation</a>.
*/
public abstract class TiBaseActivity extends FragmentActivity
public abstract class TiBaseActivity extends ActionBarActivity
implements TiActivitySupport/*, ITiWindowHandler*/
{
private static final String TAG = "TiBaseActivity";
Expand Down
8 changes: 4 additions & 4 deletions android/titanium/src/java/org/appcelerator/titanium/proxy/ActionBarProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
import org.appcelerator.titanium.util.TiFileHelper;
import org.appcelerator.titanium.util.TiUrl;

import android.app.ActionBar;
import android.app.Activity;
import android.support.v7.app.ActionBar;
import android.support.v7.app.ActionBarActivity;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.os.Message;
Expand Down Expand Up @@ -46,10 +46,10 @@ public class ActionBarProxy extends KrollProxy

private ActionBar actionBar;

public ActionBarProxy(Activity activity)
public ActionBarProxy(ActionBarActivity activity)
{
super();
actionBar = activity.getActionBar();
actionBar = activity.getSupportActionBar();
}

@Kroll.method @Kroll.setProperty
Expand Down
8 changes: 3 additions & 5 deletions android/titanium/src/java/org/appcelerator/titanium/proxy/ActivityProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.appcelerator.titanium.util.TiActivitySupport;
import org.appcelerator.titanium.util.TiActivitySupportHelper;

import android.support.v7.app.ActionBarActivity;
import android.app.Activity;
import android.content.Intent;
import android.os.Build;
Expand Down Expand Up @@ -245,11 +246,8 @@ public TiWindowProxy getWindow()
@Kroll.method @Kroll.getProperty
public ActionBarProxy getActionBar()
{
Activity activity = getWrappedActivity();
if (actionBarProxy == null && activity != null && Build.VERSION.SDK_INT >= TiC.API_LEVEL_HONEYCOMB) {
actionBarProxy = new ActionBarProxy(activity);
}

ActionBarActivity activity = (ActionBarActivity) getWrappedActivity();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to cast this one to an ActionBarActivity ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, getWrappedActivity() returns type Activity. We only cast to ActionBarActivity when an action bar is involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh also because ActionBarProxy constructor requires type ActionBarActivity

actionBarProxy = new ActionBarProxy(activity);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we removing the null check. Won't this create a new proxy everytime the method is called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

return actionBarProxy;
}

Expand Down