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-26168] Android: Fix initializing of scrollbars #10142

Merged
merged 4 commits into from Aug 13, 2018

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Jun 28, 2018

  • Scrollbars do not show in Titanium.UI.ScrollView
  • Works around a Google bug where scrollbars are not initialized
TEST CASE
const win = Ti.UI.createWindow({ backgroundColor: 'gray' }),
      scrollView = Ti.UI.createScrollView({
          showVerticalScrollIndicator: true,
          width: Ti.UI.FILL,
          height: Ti.UI.FILL
      }),
      view = Ti.UI.createLabel({
          backgroundColor: 'blue',
          width: Ti.UI.FILL,
          height: 1000,
          text: Array(16).join('SCROLL ')
      });

scrollView.add(view);

win.add(scrollView);
win.open();
  • Scrollbar should be visible when scrolling

JIRA Ticket

@ComputerTinker
Copy link

Gary, thanks for this fix. I attempted to cherrypick it, apply it to the 7.2.x branch, and test it out. The SDK I compiled works great on Android 6, however when I run in Android 4.1 it crashes with the following errors:

[ERROR] : dalvikvm: Could not find class 'android.graphics.drawable.RippleDrawable', referenced from method org.appcelerator.titanium.view.TiUIView.applyTouchFeedback
[WARN] : dalvikvm: VFY: unable to resolve new-instance 207 (Landroid/graphics/drawable/RippleDrawable;) in Lorg/appcelerator/titanium/view/TiUIView;
[WARN] : dalvikvm: threadid=1: thread exiting with uncaught exception (group=0xb116c228)
[ERROR] : TiApplication: (main) [38,38] Sending event: exception on thread: main msg:java.lang.RuntimeException: Unable to start activity ComponentInfo{APP_NAME/org.appcelerator.titanium.TiActivity}: java.lang.NullPointerException; Titanium 7.2.1,2018/07/02 16:49,undefined
[ERROR] : TiApplication: java.lang.RuntimeException: Unable to start activity ComponentInfo{edu.bju.logia/org.appcelerator.titanium.TiActivity}: java.lang.NullPointerException

Perhaps when I compile locally the results differ from the official build process? Is there a way I could get access to the SDK build you generated for this patch?

@garymathews
Copy link
Contributor Author

@ComputerTinker Nice catch! Although it looks to be unrelated to this change. I've added the fix anyway.

@jquick-axway
Copy link
Contributor

@ComputerTinker, perhaps you had a bad merge? Where it was crashing was already guarded with canApplyTouchFeedback() which checked the API Level.

Also, are you setting the touchFeedback property on a ScrollView? I don't think that would normally work with ScrollViews since they're designed to use "intercepted" touch events.

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@ComputerTinker
Copy link

@garymathews for some reason your fix didn't work. I don't know why there's a difference between "min target android version" and "min android version" in the compatibility matrix, but to my untrained eyes it seems as though we're forced to compile with >= v23 even though the code will run with >= v16, so perhaps that's why the (Build.VERSION.SDK_INT >= 21) check isn't catching it?

@jquick-axway I made sure to do a fresh checkout of 7.2.X and cherrypicked just Gary's two patches (which only update two files) so the code seems fine to me. I'm not setting touchFeedback on the ScrollView that I know of. The XML looks like this:

@ComputerTinker
Copy link

ComputerTinker commented Jul 5, 2018

I thought I would be smart and just comment out the RippleDrawable bits in TiUIView.java, however after doing that the app still blows up, although this time in a slightly different way. The following error jumps out at me:

Could not find class 'android.graphics.drawable.RippleDrawable', referenced from method android.support.v7.widget.AppCompatImageHelper.hasOverlappingRendering

@ComputerTinker
Copy link

One last thing I'll mention is that my app works fine when I compile it against 7.2.0.GA (it's just that there are no scrollbars), so it seems as though a recent patch has broken Android 4.x support in the 7.2.X branch.

@jquick-axway
Copy link
Contributor

jquick-axway commented Jul 9, 2018

I still think this is a bad merge. Instead of cherrypicking the changes over. Just copy and paste the TiUIScrollView.java file over to 7.2.x. That's all that's needed.

Edit:
And make sure to do a clean build too. I suspect you must have switched from "master" to "7_2_X" and it wasn't a clean switch over. You may have remnants of the newest Google support libraries/resources from master mixed in with 7.2.x, because the error you are running into is related to resources included with Google's support libraries and has nothing to do with this PR's code change.

@garymathews
Copy link
Contributor Author

garymathews commented Jul 9, 2018

@ComputerTinker You could replace titanium-ui.jar in your 7.2.0 SDK with titanium-ui.jar

@ComputerTinker
Copy link

@garymathews thanks for the jar file, and sorry about the delayed response.

I tried replacing the existing titanium-ui.jar file in /Users/USER_NAME/Library/Application Support/Titanium/mobilesdk/osx/7.2.0.GA/android/modules/ with the one you provided. Then I uninstalled the old version of my app, did a Project->Clean and built again in Appcelerator Studio, but unfortunately my program blew up immediately upon launch. Parsing through the log I see a bunch of errors related to transitions, and then:

[ERROR] : dalvikvm: Could not find class 'android.graphics.drawable.RippleDrawable', referenced from method org.appcelerator.titanium.view.TiUIView.applyTouchFeedback

[ERROR] : dalvikvm: Could not find class 'android.graphics.drawable.RippleDrawable', referenced from method android.support.v7.widget.AppCompatImageHelper.hasOverlappingRendering

[ERROR] : TiApplication: (main) [358,358] Sending event: exception on thread: main msg:java.lang.NullPointerException; Titanium 7.2.0,2018/06/07 05:21,undefined
[ERROR] : TiApplication: java.lang.NullPointerException
[ERROR] : TiApplication: at android.widget.ScrollBarDrawable.setAlpha(ScrollBarDrawable.java:221)
[ERROR] : TiApplication: at android.view.View.onDrawScrollBars(View.java:11268)
[ERROR] : TiApplication: at android.view.View.draw(View.java:13464)
[ERROR] : TiApplication: at android.widget.FrameLayout.draw(FrameLayout.java:467)
[ERROR] : TiApplication: at android.support.v4.widget.NestedScrollView.draw(NestedScrollView.java:1815)
[ERROR] : TiApplication: at android.view.View.getDisplayList(View.java:12409)
[...]

I can post the entire log if you like, but I think it's largely similar to the one I posted earlier.

If I put the old .jar file back and repeat the clean and recompile process, my app works again.

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Jul 20, 2018

@garymathews , I am seeing the same issue as @ComputerTinker , on android 4.1 app is stuck at splash

@garymathews
Copy link
Contributor Author

@lokeshchdhry Updated PR

@ComputerTinker
Copy link

@garymathews, it works!! I did a fresh checkout of 7_2_X and applied your updated PR and now my app no longer longer crashes and I have scrollbars. Thank you!

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@build
Copy link
Contributor

build commented Aug 13, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@hansemannn hansemannn merged commit 1b08f8c into tidev:master Aug 13, 2018
@hansemannn hansemannn modified the milestones: 7.4.0, 7.5.0 Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants