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

[RNMobile] File block IV - Error handling, retry, and cancel upload #27146

Merged
merged 6 commits into from
Nov 26, 2020

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Nov 20, 2020

gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#2826
WPiOS (testing) PR: wordpress-mobile/WordPress-iOS#15363
WPAndroid (testing) PR: wordpress-mobile/WordPress-Android#13449

Description

This PR implements Error handling for the File Block.

Error

We keep reusing elements from the other media blocks. In this case, the retry action sheet only has the Retry option.
I have chosen to add the Error label down the file name because of two reasons:

  • The name can get large
  • An issue with RichText where its width is lost on row layout (to be fixed).
    ( cc @iamthomasbishop )

How has this been tested?

To test:

Test cancel action:

  • Add a File block to an empty new post.
  • Add a file to the block.
  • While the file is uploading, tap on the block's background to see the cancel action sheet.
    • If you tap on the file name, it will start editing the file name fileld. This is intended (cc @iamthomasbishop )
  • Tap the Stop upload action.
    • The File block should go back to the empty state.

Test error and retry:

  • Add a File block to an empty new post.
  • While it's uploading, long-press anywhere in the editor to simulate an upload error.
    • See the block switch to its error state.
  • While in the error state, tap on the File block background to see the Retry action sheet.
  • After reconnecting the device, press the Retry action.
    • Check that the file starts uploading again until it successfully finishes uploading.

Screenshots

Types of changes

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.

@etoledom etoledom added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Nov 20, 2020
@etoledom etoledom self-assigned this Nov 20, 2020
@github-actions
Copy link

github-actions bot commented Nov 20, 2020

Size Change: 0 B

Total Size: 1.2 MB

ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 134 kB 0 B
build/block-editor/style-rtl.css 11.3 kB 0 B
build/block-editor/style.css 11.3 kB 0 B
build/block-library/editor-rtl.css 8.96 kB 0 B
build/block-library/editor.css 8.96 kB 0 B
build/block-library/index.js 148 kB 0 B
build/block-library/style-rtl.css 8.23 kB 0 B
build/block-library/style.css 8.23 kB 0 B
build/block-library/theme-rtl.css 792 B 0 B
build/block-library/theme.css 793 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 827 B 0 B
build/data/index.js 8.8 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.42 kB 0 B
build/edit-post/style.css 6.4 kB 0 B
build/edit-site/index.js 24 kB 0 B
build/edit-site/style-rtl.css 3.92 kB 0 B
build/edit-site/style.css 3.92 kB 0 B
build/edit-widgets/index.js 26.3 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.81 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.92 kB 0 B
build/rich-text/index.js 13.3 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.05 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

Copy link
Contributor

@jd-alexander jd-alexander left a comment

Choose a reason for hiding this comment

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

Thanks for these changes @etoledom I followed the testing instructions and I was able to accomplish the retry/cancel operations on the Android PR that has the File Block enabled for the CI build. On Android, I changed the container style to

.errorContainer {
	flex-direction: row;
	align-items: flex-start;
}

It aligned the items but there's a lot of space. I wasn't able to test this on iOS as yet.

Below was the Android outcome of adding this style.

@etoledom etoledom force-pushed the rnmobile/file-block-IV-cancel-retry-error branch from d4dbaa7 to c945f92 Compare November 25, 2020 09:05
@etoledom
Copy link
Contributor Author

Nice catch!

I took the idea and the chance to also fix the Botton style on Android. Now it looks more close to the original design:

Screenshot_20201125-100813

@etoledom etoledom force-pushed the rnmobile/file-block-IV-cancel-retry-error branch from c945f92 to 68bdbd0 Compare November 25, 2020 09:33
@etoledom
Copy link
Contributor Author

Also added unit test for Error state 🎉

@etoledom
Copy link
Contributor Author

All PRs updated and ready for another look 👀

Copy link
Contributor

@jd-alexander jd-alexander left a comment

Choose a reason for hiding this comment

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

Things are looking good here @etoledom LGTM 🚢 Verified the behavior via the Android testing PR.

@etoledom etoledom force-pushed the rnmobile/file-block-IV-cancel-retry-error branch from e3509a4 to b84da00 Compare November 26, 2020 07:47
@etoledom etoledom merged commit a8b99c6 into master Nov 26, 2020
@etoledom etoledom deleted the rnmobile/file-block-IV-cancel-retry-error branch November 26, 2020 10:54
@github-actions github-actions bot added this to the Gutenberg 9.5 milestone Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants