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

[Mobile] Cover block upload progress #21197

Conversation

mkevins
Copy link
Contributor

@mkevins mkevins commented Mar 27, 2020

Description

This PR adds the MediaUploadProgress component to cover block. This adds a progress bar to the block to indicate the progress of ongoing uploads. It also updates the id and url of the media to the new server id and url upon upload completion.

Related PRs

Testing steps

  1. Add cover block to a post
  2. Tap "Add Image or Video"
  3. Tap "Choose from Device"
  4. Pick an image to upload from your device

Expected:

  1. The image should appear as a background to the block
  2. A progress bar should display at the top of the image and update as the upload progresses
  3. After the upload completes, the media url should be the server url (i.e. begins with https:, not file:)

Screenshots

Progress bar URL scheme
cover-upload-progress-bar cover-upload-progress-html

Additional changes

In order to ensure the progress bar was displayed on top of the media, it was necessary to add a z-index to the MediaUploadProgress component's progressBar class: 7f00459.

I also simplified a few other areas via optional chaining: 5cd723e.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@mkevins mkevins added [Feature] Media Anything that impacts the experience of managing media [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Mar 27, 2020
@github-actions
Copy link

github-actions bot commented Mar 27, 2020

Size Change: 0 B

Total Size: 889 kB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.53 kB 0 B
build/block-library/style.css 7.54 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/index.js 2.71 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.18 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 400 B 0 B
build/editor/editor-styles.css 402 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@chipsnyder chipsnyder left a comment

Choose a reason for hiding this comment

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

LGTM

Tested on iOS and Android
I was able to add Images from the device on iOS and Android. Also tested taking images and video and those worked as well.

Note:
I am getting validation errors if I back out during upload and come back in. I'm just mentioning it here to capture it; however, this would probably be handled in another phase. I can add more info here if this is unexpected.

@mkevins
Copy link
Contributor Author

mkevins commented Mar 31, 2020

Note:
I am getting validation errors if I back out during upload and come back in. I'm just mentioning it here to capture it; however, this would probably be handled in another phase. I can add more info here if this is unexpected.

Indeed, this will be handled in another PR. Thanks for testing and reviewing!

@geriux
Copy link
Member

geriux commented Apr 6, 2020

Hey, @mkevins 👋 While reviewing I noticed some linting issues, I guess since this is not targeting master is not running the Travis checks.

[0]   198:22  error  'id' is already declared in the upper scope   no-shadow
[0]   199:17  error  'url' is already declared in the upper scope  no-shadow

And a few prettier warnings just FYI.

@geriux
Copy link
Member

geriux commented Apr 6, 2020

I also simplified a few other areas via optional chaining: 5cd723e.

This is so cool! I didn't see this PR enabling it! Cannot wait to try it :D

Copy link
Member

@geriux geriux left a comment

Choose a reason for hiding this comment

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

@mkevins mkevins merged commit 4702fd7 into rnmobile/feature/cover-block-uploads-dev-only Apr 9, 2020
@mkevins
Copy link
Contributor Author

mkevins commented Apr 9, 2020

Thanks for testing and reviewing!

mkevins added a commit that referenced this pull request Apr 9, 2020
* RNMobile - Enable Cover for Production

* RNMobile - Cover block - Move gradient background to support overlay functionality

* RNMobile - Cover - Image With Focal Point - Check for url changes

* Enable upload options for cover block behind dev flag

* Use [].prototype.some for media-library-only prop

* Use destructuring in renderBackground function

* [Mobile] Cover block upload progress (#21197)

* Add progress indicator for uploading media

* Use default import syntax for Video component

* Use optional chaining

* Add z-index to upload progress bar

* Fix lint

Co-authored-by: Gerardo Pacheco <[email protected]>
@mkevins mkevins deleted the rnmobile/feature/cover-block-uploads-progress branch July 6, 2020 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Feature] Media Anything that impacts the experience of managing media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants