Gb/upload media file

[marecar3]

Description :
This PR introduces image file upload from Gutenberg which implementation is based on this issue.

This is the first and base iteration of a task that should be implemented. Initial idea is to present an image picker from gutenberg editor which will trigger the upload process when the user picks an image.

Following PR's :
gutenberg-mobile
gutenberg

Note :

• Missing final colors and dimensions for progress bar as it's hard to take it from the png.
• Missing the error state (when an upload fails)
• Not sure if edit toolbar button should open media picker or image chooser after an image is uploaded with success.

@iamthomasbishop please can you help me with the above notes? Thanks!

[wpmobilebot]

	2 Warnings
⚠️	PR is missing at least one label.
⚠️	PR is not assigned to a milestone.

Generated by 🚫 Danger

[iamthomasbishop]

Hey @marecar3 looking good!

Here are the specs, which I'll also drop in the Zeplin project asap.

final colors and dimensions for progress bar

• Progress bar: #0087BE ($blue-wordpress)
• Track: #c8d7e1 ($gray-lighten-20)
• Height: 5pt
• Width: 100% of image

error state (when an upload fails)

A visual example of the loading and error states:

• Overlay: #2E4453 ($gray-dark) at 60% opacity – this should match Aztec, so we need to double check it matches. // cc @etoledom is this correct?
• Icon and text are both #FFFFFF
• We're using gridicon-refresh for the icon, but we can use any available icon that has the same effect
• Text is 14pt SF Pro Text Semibold on iOS, and I'm thinking it's 14sp Roboto in Medium

Not sure if edit toolbar button should open media picker or image chooser after an image is uploaded with success.

I would match what exists on Gutenberg web here (although I do think the pencil icon here is confusing). So tapping the pencil on the toolbar would open the same picker that you chose the current image from. I'm not a huge fan of this workflow, but for consistency and brevity, let's start there.

Another option would be to utilize the same flow that we have built for Aztec for now, which looks (roughly) like this:

Inside the wider Gutenberg flow, it could look like this:

I've got other suggestions, but for the alpha/beta let's keep it simple.

[hypest]

Hey @marecar3 , not sure if it was part of the goal for this PR but, it seems that leaving the editor and re-entering while the image is still uploading doesn't seem to work correctly:

1. The block validation fails upon re-entering
2. The image upload indicator doesn't seem to update on re-enter and it never gets cleared

Do you think we can address those issues during this PR?

[mzorz]

it seems that leaving the editor and re-entering while the image is still uploading doesn't seem to work correctly

hi @hypest 👋 This I think exceeds the scope of this particular PR, I've tracked it here wordpress-mobile/gutenberg-mobile#459 and I was planning to tackle it (in conjunction with Marko when he's back) as I intend to do this wiring in the new GutenbergEditPostActivity introduced in #8874

For some context, keeping states up to date for in-progress uploads was one of the trickiest part of the async media upload project before (see #5780 for one example of various cases that have to be taken into account), so I'd like to bring that "case-awareness" here as well

[mzorz]

Clarification:

The block validation fails upon re-entering

This is something we should still investigate and work on as needed. I was talking only about media progress re-attachment in my previous comment

[mzorz]

Nice job @marecar3 - I've found some issues that have enough merit to be tackled on separate PRs (like media upload progress reattachment), but I think these set of PRs (this one and the references gutenberg and gutenberg-mobile ones) covers the base need for media uploads.

I've left a comment around naming convention that I'd like to get addressed before merge, and here the most important issues:

One of the issues that may be good to try fixing is this one:

• After uploading finishes, tapping on the image shows a red screen, logcat shows this:

2019-01-04 16:06:32.665 12480-12699/org.wordpress.android E/unknown:ReactNative: Exception in native call
    com.facebook.react.uimanager.IllegalViewOperationException: Unable to execute operation dispatchViewManagerCommand on view with tag: 117, since the view does not exists
        at com.facebook.react.uimanager.UIImplementation.assertViewExists(UIImplementation.java:919)
        at com.facebook.react.uimanager.UIImplementation.dispatchViewManagerCommand(UIImplementation.java:798)
        at com.facebook.react.uimanager.UIManagerModule.dispatchCommand(UIManagerModule.java:637)
        at com.facebook.react.uimanager.UIManagerModule.dispatchViewManagerCommand(UIManagerModule.java:632)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:160)
        at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
        at android.os.Handler.handleCallback(Handler.java:790)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
        at android.os.Looper.loop(Looper.java:164)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$3.run(MessageQueueThreadImpl.java:192)
        at java.lang.Thread.run(Thread.java:764)

The message is the same on the red screen: Unable to execute operation dispatchViewManagerCommand on view with tag: 117, since the view does not exists.

• CircleCI is failing, so let's give it a spin as well before merging.

Leaving comments on each PR separately, and I think we'll be good to approve this after that so we can move on (after the errors above are taken care of / tracked separately if you decide to). One more thing: I think we may want to change the base branch to a feature branch in WPAndroid repo before actually merging though. Leaving that for @hypest to decide.

[mzorz]

Maybe we should follow the naming convention used in https://github.com/wordpress-mobile/WordPress-Android/pull/8879/files#diff-38b6aea9cfff5f3e0e4b7389b44de602R3185 and have the verb conjugation be in the past tense, i.e. onUploadMediaButtonClicked and onMediaLibraryButtonClicked, following the same pattern of on<EventHappened> as used in interface EditorFragmentListener

[marecar3]

@mzorz thanks for the comment, fixed.

[hypest]

I think we may want to change the base branch to a feature branch in WPAndroid repo before actually merging though. Leaving that for @hypest to decide.

Thanks for the ping @mzorz . I think that wpandroid's develop needs to be treated as a branch more stable than the gutenberg-mobile's develop so yeah, let's target a feature branch for the media upload work.

By the way, I re-triggered CircleCI.

[marecar3]

Base branch is changed to origin/gb/feature_upload_media_file .

[mzorz]

LGTM!