1628 mediapicker

[tonyr59h]

Woo, time to get some eyes on this branch! I've been fiddling with it and have some notes provided below for things to work on. I'm not too sure about the look and feel of the icons, would definitely like some design feedback.

Added:

• Post editor image icon launches MediaPickerActivity with two tabs.
• Blog images take some time to load as they are added to the adapter after they are verified (to handle deleted media)
• Clicking an item will select only that item and return to the Post editor
• Long pressing an item will enable Action Mode for multi-select
• A gallery is created when the gallery icon is selected; local images are first uploaded then their ID is added to the gallery
• Simply confirming selection will add each item individually to the Post
• Pressing the back arrow will destroy Action Mode but stay in the MediaPickerActivity, back again will return to the editor
• VideoPress thumbnails!
• Parallelized background thumbnail generation
• Progress indicator when uploading media
• Placeholder image to fallback on when a thumbnail can't be retrieved
• WPCollectionSpan for condensing multi-selected media in the Post editor; can be modified when tapped
• If the user removes all but one item from a collection in the post it replaces the WPCollectionSpan with a WPImageSpan
• ^^ Likewise if the user removes all of the items from a collection it removes the span

  In-progress

• @maxme pointed out that inserting images can sometimes remove previous images; investigating
• If the user removes the first item from a WPCollectionSpan it should generate a new thumbnail from the available content

[roundhill]

I'm getting an exception when I tap on the add media button in the post editor:

java.lang.NullPointerException: Attempt to invoke interface method 'void org.wordpress.mediapicker.source.MediaSource.setListener(org.wordpress.mediapicker.source.MediaSource$OnMediaChange)' on a null object reference
            at org.wordpress.mediapicker.MediaSourceAdapter.<init>(MediaSourceAdapter.java:42)
            at org.wordpress.mediapicker.MediaPickerFragment.layoutGridView(MediaPickerFragment.java:251)
            at org.wordpress.mediapicker.MediaPickerFragment.onCreateView(MediaPickerFragment.java:89)
            at android.app.Fragment.performCreateView(Fragment.java:2053)
            at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:894)
            at android.app.FragmentManagerImpl.moveToState(FragmentManager.java:1067)
            at android.app.BackStackRecord.run(BackStackRecord.java:833)
            at android.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1452)
            at android.app.FragmentManagerImpl.executePendingTransactions(FragmentManager.java:483)
            at android.support.v13.app.FragmentPagerAdapter.finishUpdate(FragmentPagerAdapter.java:145)
            at android.support.v4.view.ViewPager.populate(ViewPager.java:1073)
            at android.support.v4.view.ViewPager.populate(ViewPager.java:919)
            at android.support.v4.view.ViewPager.onMeasure(ViewPager.java:1441)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.widget.LinearLayout.measureChildBeforeLayout(LinearLayout.java:1436)
            at android.widget.LinearLayout.measureVertical(LinearLayout.java:722)
            at android.widget.LinearLayout.onMeasure(LinearLayout.java:613)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.widget.FrameLayout.onMeasure(FrameLayout.java:430)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.support.v7.internal.widget.ActionBarOverlayLayout.onMeasure(ActionBarOverlayLayout.java:453)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.widget.FrameLayout.onMeasure(FrameLayout.java:430)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.widget.LinearLayout.measureChildBeforeLayout(LinearLayout.java:1436)
            at android.widget.LinearLayout.measureVertical(LinearLayout.java:722)
            at android.widget.LinearLayout.onMeasure(LinearLayout.java:613)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewGroup.measureChildWithMargins(ViewGroup.java:5463)
            at android.widget.FrameLayout.onMeasure(FrameLayout.java:430)
            at com.android.internal.policy.impl.PhoneWindow$DecorView.onMeasure(PhoneWindow.java:2560)
            at android.view.View.measure(View.java:17430)
            at android.view.ViewRootImpl.performMeasure(ViewRootImpl.java:2001)
            at android.view.ViewRootImpl.measureHierarchy(ViewRootImpl.java:1166)
            at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1372)
            at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1054)
            at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:5779)
            at android.view.Choreographer$CallbackRecord.run(Choreographer.java:767)
            at android.view.Choreographer.doCallbacks(Choreographer.java:580)
            at android.view.Choreographer.doFrame(Choreographer.java:550)
            at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:753)
            at android.os.Handler.handleCallback(Handler.java:739)
            at android.os.Handler.dispatchMessage(Handler.java:95)
            at android.os.Looper.loop(Looper.java:135)
            at android.app.ActivityThread.main(ActivityThread.java:5221)
            at java.lang.reflect.Method.invoke(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:372)
            at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:899)
            at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:694)

[tonyr59h]

I fixed a different NPE with that last commit, but I've also pushed a fix for the one posted on the MediaPicker-Android library. I'll release a 1.1.1 and integrate it with the PR after some more testing.

[maxme]

I'll co-review this PR with you @roundhill (I'll have to merge it with the editor modularization branch)

[roundhill]

Here's a few UX related observations:

• Some of the images in the list are grey, never seem to load. They appear to be legit pictures that I captured with my camera. When I add them to the editor, they load fine. Happy to send you copies of the images if you think it'd help:

• I long-pressed on some images, then selected the gallery button. I was returned to the editor and nothing seemed to have happened.
• I selected 80 images and selected the check mark. It took 20 seconds for the app to return to the editor. Maybe there's a background task needed to be done here?
• Is rotation not allowed in the media selector?
• I think you were already working on it, but the shadow should be removed from the action bar, and the camera and video icons should be white.

[tonyr59h]

I long-pressed on some images, then selected the gallery button. I was returned to a editor and nothing seemed to have happened.

Have you tried this with only a few items selected? It should give you a toast that tells you uploads are pending and the image previews should be added to the post.

I selected 80 images and selected the check mark. It took 20 seconds for the app to return to the editor. Maybe there's a background task needed to be done here?

Hadn't tested with so many images before but yeah, it's incredibly slow. Seems to be loading all the image previews on the main thread, I'll offload it.

Is rotation not allowed in the media selector?

I disabled it when I was working on the live preview item, it was causing issues. Might be possible to enable it again since removing live preview from the scope.

[roundhill]

Have you tried this with only a few items selected? It should give you a toast that tells you uploads are pending and the image previews should be added to the post.

Yeah, I had only selected two images. Just tried on a 5.0 device and it worked fine. My regular device is running 4.4, so maybe that's a clue.

On the gallery, is there a way that we could have it insert the gallery icon like it used to instead of the shortcode? Otherwise you wouldn't be able to change the gallery type, number of columns, etc.

[roundhill]

I'm also seeing these 'W' logo images in the list.

[roundhill]

I think the gallery upload process could be better. Instead of showing the images in the post and showing a toast message, perhaps we could show a progress bar that shows that the images are being uploaded, but the user could still continue to write their post. That way it wouldn't be as jarring when the gallery creation is complete as well.

[roundhill]

I'm also seeing these 'W' logo images in the list.

Update: I think these aren't loading because the site is marked as private.

[tonyr59h]

Agreed on the upload process, I'll try to add a visual progress indicator in the editor for it. I'll also be removing all the image previews when the user confirms selection, it gets too laggy if many are selected. Maybe replace it with a folder icon to represent 'multiple images here' in the editor.

[tonyr59h]

@roundhill

Some of the images in the list are grey, never seem to load. They appear to be legit pictures that I captured with my camera. When I add them to the editor, they load fine.

I noticed that as well on my 5.0 device. For some reason the thumbnails for those images are loading fine now, maybe they get generated if the device is rebooted? There's gonna be a fallback image for any thumbnail that doesn't load so this should at least be handled gracefully.

[maxme]

1. We should show a back arrow in the action bar.

2. Tap the "Take picture" or "Take video" action bar button and then tap the back button results in a crash

   java.lang.NullPointerException
   at java.io.File.fixSlashes(File.java:185)
   at java.io.File.<init>(File.java:134)
   at org.wordpress.android.ui.media.MediaPickerActivity.onActivityResult(MediaPickerActivity.java:136)
   at android.app.Activity.dispatchActivityResult(Activity.java:5192)

3. We should refresh the media library when opening this screen (if the user never goes to the MediaLibrary activity, he won't see any of his blog library pictures).
4. Adding new pictures sometime remove previous insertions, see: https://cloudup.com/cfWqqCTyH8x (Note: I can repro this only on an emulator).
5. When pictures are loading from the blog library, we should show a loading indicator (like when you scroll down the Reader post list and it loads new posts). Can we load the full (downloaded) list and show placeholders?
6. Sometimes media library pictures never show up (or only a few of them). see: https://cloudup.com/c0mzT78xngt
7. Update to org.wordpress:mediapicker:1.1.1 (1.1.0 crash on my LG G3 but works on my Nexus 5).

[tonyr59h]

When pictures are loading from the blog library, we should show a loading indicator (like when you scroll down the Reader post list and it loads new posts). Can we load the full (downloaded) list and show placeholders?

Unfortunately we can't show N placeholder images while they are loading due to issues with deleted media. After requesting all the images for a blog we actually have to perform an HTTP GET to verify that we don't receive a 404 on a particular image. That's the main reason for the delay you see when loading blog images.

[tonyr59h]

@maxme I can't reproduce your 4th bullet point. Which version of Android are you emulating?

[maxme]

I can't reproduce your 4th bullet point. Which version of Android are you emulating?

It was an emulated Nexus S running Android 4.1.1 (Genymotion).

When I tap the media icon, I now get this crash:

03-02 08:18:42.140    1625-1670/org.wordpress.android.beta E/AndroidRuntime﹕ FATAL EXCEPTION: AsyncTask #3
    java.lang.RuntimeException: An error occured while executing doInBackground()
            at android.os.AsyncTask$3.done(AsyncTask.java:299)
            at java.util.concurrent.FutureTask$Sync.innerSetException(FutureTask.java:273)
            at java.util.concurrent.FutureTask.setException(FutureTask.java:124)
            at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:307)
            at java.util.concurrent.FutureTask.run(FutureTask.java:137)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
            at java.lang.Thread.run(Thread.java:856)
     Caused by: java.lang.NullPointerException
            at org.wordpress.mediapicker.MediaUtils$BackgroundFetchThumbnail.doInBackground(MediaUtils.java:80)
            at org.wordpress.mediapicker.MediaUtils$BackgroundFetchThumbnail.doInBackground(MediaUtils.java:50)
            at android.os.AsyncTask$2.call(AsyncTask.java:287)
            at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:305)
            at java.util.concurrent.FutureTask.run(FutureTask.java:137)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1076)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:569)
            at java.lang.Thread.run(Thread.java:856)

[tonyr59h]

@maxme should be all patched up with the latest MediaPicker library. Looks like I was being too optimistic with the MediaStore query, the emulator returns null.

[maxme]

Unfortunately we can't show N placeholder images while they are loading due to issues with deleted media. After requesting all the images for a blog we actually have to perform an HTTP GET to verify that we don't receive a 404 on a particular image. That's the main reason for the delay you see when loading blog images.

IMHO, that's really a weird user experience. When on a slow network, I have the impression I don't have any images in my media library. How the media library is handling this 404 issue?

Another comment, the more I test it, the more I think we shouldn't mix local and media library pictures. The main reason: I have a huge list of local pics and searching for a media library items make me scroll a loooong way to see them. Second argument: even with the "W" logo, it's not super explicit. I'm not against it, but I would like to hear from other devs/designers/users about this.

[tonyr59h]

When on a slow network, I have the impression I don't have any images in my media library. How the media library is handling this 404 issue?

MediaSourceWPImages queries each of the images and verifies a 200 return code. After all the checks are complete it updates the adapter with all the verified images via notifyDataSetChanged.

the more I test it, the more I think we shouldn't mix local and media library pictures. The main reason: I have a huge list of local pics and searching for a media library items make me scroll a loooong way to see them.

I've noticed this as well. I have ~100 images to scroll through when I want to get to my library images, it's a hassle. I'm gonna switch it to four tabs for now; Device Images, Device Video, Blog Images, Blog Video.

[maxme]

Before the merge:

• [x] move tab titles to strings.xml
• [x] loading indicator for "site image/video"
• [x] reverse sort order
• [x] adding a "site image" doesn't work
• [x] create new post, add 2 (or more) images from the device, back, back (local draft saved), then edit the post again: it's empty.
• [x] create new post, add 2 (or more) images, publish: the published post is empty (no images).

Next iteration:

• empty views
• work on pre-loading / caching / updating the media library

[tonyr59h]

Here's a video of adding two local and two blog images to a new post then publishing it. Local images still need to be queued for upload then the last bullet point will be satisfied.

[tonyr59h]

Removing the WPCollectionSpan on your branch fixes all the issues. There's still the problem (https://github.com/wordpress-mobile/WordPress-Android/issues/2406) of too many items lagging the UI but as discussed on Slack that is likely an edge case.

EDIT: We should merge in your branch, @maxme then all the issues listed above will be resolved.

[maxme]

Tested again, and looks ok!

Encountered a crash, but I'm pretty sure it wasn't introduced in this pr:

03-10 11:30:34.651    8847-8948/org.wordpress.android.beta E/AndroidRuntime﹕ FATAL EXCEPTION: AsyncTask #4
    Process: org.wordpress.android.beta, PID: 8847
    java.lang.RuntimeException: An error occured while executing doInBackground()
            at android.os.AsyncTask$3.done(AsyncTask.java:300)
            at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:355)
            at java.util.concurrent.FutureTask.setException(FutureTask.java:222)
            at java.util.concurrent.FutureTask.run(FutureTask.java:242)
            at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
            at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
            at java.lang.Thread.run(Thread.java:818)
     Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'android.content.res.Resources android.content.Context.getResources()' on a null object reference
            at org.wordpress.android.util.HtmlToSpannedConverter.startImg(WPHtml.java:810)
            at org.wordpress.android.util.HtmlToSpannedConverter.handleStartTag(WPHtml.java:652)
            at org.wordpress.android.util.HtmlToSpannedConverter.startElement(WPHtml.java:992)
            at org.ccil.cowan.tagsoup.Parser.push(Parser.java:794)
            at org.ccil.cowan.tagsoup.Parser.rectify(Parser.java:1061)
            at org.ccil.cowan.tagsoup.Parser.stage(Parser.java:1026)
            at org.ccil.cowan.tagsoup.HTMLScanner.scan(HTMLScanner.java:632)
            at org.ccil.cowan.tagsoup.Parser.parse(Parser.java:449)
            at org.wordpress.android.util.HtmlToSpannedConverter.convert(WPHtml.java:552)
            at org.wordpress.android.util.WPHtml.fromHtml(WPHtml.java:159)
            at org.wordpress.android.util.WPHtml.fromHtml(WPHtml.java:122)
            at org.wordpress.android.ui.posts.EditPostPreviewFragment$LoadPostPreviewTask.doInBackground(EditPostPreviewFragment.java:78)
            at org.wordpress.android.ui.posts.EditPostPreviewFragment$LoadPostPreviewTask.doInBackground(EditPostPreviewFragment.java:62)
            at android.os.AsyncTask$2.call(AsyncTask.java:288)
            at java.util.concurrent.FutureTask.run(FutureTask.java:237)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
at java.lang.Thread.run(Thread.java:818)

[maxme]

Filled this new issue for empty views: https://github.com/wordpress-mobile/WordPress-Android/issues/2414

[maxme]

:shipit: :fireworks: