wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Themes search #3302

Closed kwonye closed 8 years ago

kwonye commented 8 years ago

Addresses #3157

This allows you to search for a theme and activate. Uses new v1.2 endpoints.

Needs review: @tonyr59h

tonyr59h commented 8 years ago
kwonye commented 8 years ago
  • The background on the search icon is not transparent on lower API's (tested on 4.1.1)

Fixed in afdf0b0

  • The layout on the Support page is messy, not sure if that's fixable in the app though

Seems to be a web rendering issue. Will forward to relevant team.

* Rotation causes the Search view style to change; the white background turns blue
* It's relatively easy to get into a [weird state](https://cldup.com/cbheJKurma.png) where the search view is showing when not in search mode
* When in the above state selecting the Search icon will crash the app `java.lang.IllegalStateException: Fragment already added: ThemeSearchFragment`

Addressed

tonyr59h commented 8 years ago

It's relatively easy to get into a weird state where the search view is showing when not in search mode

I'm still seeing this issue, not sure how to repro it exactly but it happens fairly frequently when I tinker around.

kwonye commented 8 years ago

I'm still seeing this issue, not sure how to repro it exactly but it happens fairly frequently when I tinker around.

I'll keep trying it out. What device are you using?

sendhil commented 8 years ago

@kwonye any update on this?

tonyr59h commented 8 years ago

@sendhil I'm in the middle of reviewing again, trying to figure out how that search state bug occurs so we can fix it.

kwonye commented 8 years ago

@sendhil Finishing it up right now. To do's are:

  1. Hide masterbar (changing URL request)
  2. Hiding premium themes
  3. Customizer is currently "closable", may have to work w/ flow to disable it.
tonyr59h commented 8 years ago

Now that the spinner is out of the picture I think we can make that whole bar clickable to search.

tonyr59h commented 8 years ago

Can we intercept the X button press in the theme WebView and redirect back to the Theme browser activity?

See video

tonyr59h commented 8 years ago

There's this really weird scrolling behavior that happens in the browser ListView. See the video here. Keep an eye on the scrollbar, I never scrolled down, that bounce happens independent of user input.

tonyr59h commented 8 years ago

App crashes when entering the theme browser with no connection.

android.database.sqlite.SQLiteException: no such column: true (code 1): , while compiling: SELECT id FROM themes WHERE blogId=? and isCurrent=true
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteConnection.nativePrepareStatement(Native Method)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteConnection.acquirePreparedStatement(SQLiteConnection.java:889)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteConnection.prepare(SQLiteConnection.java:500)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteSession.prepare(SQLiteSession.java:588)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteProgram.<init>(SQLiteProgram.java:58)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteStatement.<init>(SQLiteStatement.java:31)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.sqlite.SQLiteDatabase.compileStatement(SQLiteDatabase.java:1052)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.database.DatabaseUtils.stringForQuery(DatabaseUtils.java:833)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at org.wordpress.android.WordPressDB.getCurrentThemeId(WordPressDB.java:1843)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at org.wordpress.android.ui.themes.ThemeBrowserActivity$6.onErrorResponse(ThemeBrowserActivity.java:253)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at com.android.volley.Request.deliverError(Request.java:590)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at com.wordpress.rest.RestRequest.deliverError(RestRequest.java:85)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at com.android.volley.ExecutorDelivery$ResponseDeliveryRunnable.run(ExecutorDelivery.java:101)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.os.Handler.handleCallback(Handler.java:739)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.os.Handler.dispatchMessage(Handler.java:95)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.os.Looper.loop(Looper.java:135)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at android.app.ActivityThread.main(ActivityThread.java:5499)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at java.lang.reflect.Method.invoke(Native Method)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at java.lang.reflect.Method.invoke(Method.java:372)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:942)
10-22 16:33:47.151 19585-19585/org.wordpress.android E/AndroidRuntime:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:737)
10-22 16:33:49.867 273-712/? E/ALSAModule: s_standby handle h 0xb8f98748```
tonyr59h commented 8 years ago

PTR shows a line instead of an arrow (no pointer). Also, after rotation the PTR doesn't work, the animation will hang indefinitely.

kwonye commented 8 years ago

There's this really weird scrolling behavior that happens in the browser ListView. See the video here. Keep an eye on the scrollbar, I never scrolled down, that bounce happens independent of user input.

Hmm, unable to repro unfortunately. Does this happen every time? How about with other activities?

PTR shows a line instead of an arrow (no pointer). Also, after rotation the PTR doesn't work, the animation will hang indefinitely.

It shows it while it's being pulled down, but not while refreshing, which I believe is correct.

App crashes when entering the theme browser with no connection.

Fixed offline usage overall, including this

Can we intercept the X button press in the theme WebView and redirect back to the Theme browser activity?

X is now hidden by using the parameter hide_close=true

Now that the spinner is out of the picture I think we can make that whole bar clickable to search.

Done

tonyr59h commented 8 years ago

Does this happen every time? How about with other activities?

Happens about 80% of the time. I tested in other activities and it doesn't reproduce. Would be nice to get to the bottom of this, but it's not a blocking issue.

All the other stuff is looking good :+1:

I found another big issue, after rotation searching sort of breaks down, I imagine it's state loss somewhere. Lots of weirdness happens; it can crash, searching doesn't work, title bar gets all messed up.

kwonye commented 8 years ago

I found another big issue, after rotation searching sort of breaks down, I imagine it's state loss somewhere. Lots of weirdness happens; it can crash, searching doesn't work, title bar gets all messed up.

@tonyr59h is this on a certain device? Possibly with low memory? I have been trying to repro this on my Nexus 9 w/ 6.0 and it isn't happening.

tonyr59h commented 8 years ago

Happening with Nexus 9 running M. See video.

kwonye commented 8 years ago

Happening with Nexus 9 running M. See video.

Thanks, that uncovered me not handling the fragment lifecycle correctly! Addressed. That should remove all the little quirks that I wasn't able to repro before as well!

kwonye commented 8 years ago

@tonyr59h currently activated theme is now highlight like the web. Also, previously the selected theme was hidden from the list, but it is now shown per @folletto

tonyr59h commented 8 years ago

Nice. I finished testing earlier, looks like you've addressed the fragment state issues. Going through the code now.

tonyr59h commented 8 years ago

Crashing when I select the Search bar

java.lang.IllegalStateException: Couldn't read row 0, col -1 from CursorWindow.  Make sure the Cursor is initialized correctly before accessing data from it.
                                                                         at android.database.CursorWindow.nativeGetLong(Native Method)
                                                                         at android.database.CursorWindow.getLong(CursorWindow.java:511)
                                                                         at android.database.CursorWindow.getInt(CursorWindow.java:578)
                                                                         at android.database.AbstractWindowedCursor.getInt(AbstractWindowedCursor.java:69)
                                                                         at org.wordpress.android.ui.themes.ThemeBrowserAdapter.bindView(ThemeBrowserAdapter.java:77)
                                                                         at android.widget.CursorAdapter.getView(CursorAdapter.java:289)
                                                                         at android.widget.AbsListView.obtainView(AbsListView.java:2346)
                                                                         at android.widget.GridView.onMeasure(GridView.java:1065)
                                                                         at org.wordpress.android.widgets.HeaderGridView.onMeasure(HeaderGridView.java:81)
tonyr59h commented 8 years ago

Without a connection the Failed to fetch themes toast hangs around for around 30s.

tonyr59h commented 8 years ago

After I re-enable connection, when I click on a theme I get a Could not load theme toast. Rotation resolves the issue.

tonyr59h commented 8 years ago

Action Bar title is cleared when rotated.

kwonye commented 8 years ago

Great catches, @tonyr59h!

I was able to address all the issues except for the toast. Is that on clean install, initial load only?

tonyr59h commented 8 years ago

It happens every time I scroll to the bottom of the themes list with no connection. Tested on My Nexus 9 and a Genymotion 4.4 emu

tonyr59h commented 8 years ago

Nice, looks good. Let's test the feature/themes-browser branch thoroughly for any merge errors.

sendhil commented 8 years ago

:dancer: