Block Directory: Try/block directory inline error msgs. #19589

StevenDufresne · 2020-01-13T06:49:48Z

Description

This PR is an experiment to add inline error messages to the block directory. This PR also introduces a change in execution that needs discussion.

Background Info

We currently, download the assets (which means we inject scripts into the DOM)
Then we install the assets by calling an API rest endpoint (.... the plugin ends up in their plugins folder)
We currently execute download then install as a callback.
During the download sequence, the block is registered which closes the menu (because the way the inserter works) and then the install is executed with the menu closed. Should any issues arise, the error message will appear with a bit of a delay.
If the block fails at install we offer them a button and allow them to remove the block.
- If they don't remove and reload the page, the block is broken. There is no way to fix it.

Types of changes

Changes:

Along with the obvious changes to add errors inline, this PR changes the order of execution. Instead of immediate injecting the assets in the download sequence and then installing. This PR first installs and then downloads.

Why?

Before this PR, the inserter disappears as soon as the block gets registered in the download phase (the block registers and update the state). This means that if an error is throw on install, the context has already changed. We can't show anything inline, it's gone.
If a block fails at install there isn't much recourse to fix it. Now, or potentially in the future. This means we have a broken plugin in the document. We therefore would need to have a robust "removal" experience. By flipping it, nothing is added until everything is :100 with the server.
We could maintain a thinner front end package since the error logic and validation stays on the server and does creep to the front end.

Cons

There is a delay as we install (subject to the network speed)
- Added a loading state to the button to mitigate.
... probably more

Known Issues

We should consider a better way to close the inserter. There is a callback that is passed in from the inserter slot onSelect, but it has a lower priority and depending on the state of the app, you could see a bit of a flicker. I have seen it sporadically.

Screenshots

	Before	After
Success
Install Error	_{^{Disregard that the component actually was loaded below.}}
Download Error		_{^{Mimicking this issue was tough and therefore I couldn't resolve the "Retry" work... but it is retrying :).}}

How has this been tested?

Unit tests were added to cover the changes

Manual Testing

Manual test were done by doing the following: ( using these block: Card, Boxer, Listicle)

Case Success

Search/Add Block
a. Notice that the block is added to the document without error
b. Notice that the plugin is not in the regular WordPress plugin folder

Case Install Error:

Search/Add a block
a. Make the /install endpoint return an error on first call
b. Notice the error message is showing with the proper copy..
Click Retry
a. Notice the error message is removed
b. Notice the plugin is in WordPress plugin folder

Case Download Error:

Search/Add a block
a. Add a throw statement in the download function in action.js to mimick and issue with adding assets to the DOM
b. Notice the error message is showing with the proper copy.

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. .

…actions.

…ding state when the installing is happening and make the 'Add block' button show a loading state while installing

…ly retry call from the notice component.

…l test since its not used and therefore could introduce debt.

…k. Add tests for that component.

* Declare sideEffects for build-style * fixup! Declare sideEffects for build-style * Add CHANGELOG entries

* RNMobile - Refactor of Caption component into two: BlockCaption and Caption component * RNMobile - Refactor of Caption component * RNMobile - Caption refactor passing accessibility props and adding missing accessibility label for the Video block's caption * RNMobile - Caption: Specify p tag for both platforms

* Return focus to original element * Fix list e2e tests * Fix annotation e2e tests * Fix some rich text e2e tests * Fix tests * Focus after link * Add onFocus * Corrections

* wip * Wait for block to mount * Fix some tests * Revert moving focus effects * Fix align error * Fix navigation mode * Fix capturing toolbar * Remove has-toolbar-captured * Add docs * Simplify multi selection selector * Fix alignment * Clean up * Only add block node for selected block * Simplify block DOM node state * Fix alignment tests * Clean up * Clean up * Avoid node in state

…ordPress#19528) * adds no title label to links to pages or posts with no title * fixes a wrong return and an explicit comparison * undo mistaken CSS edit

* Block: reposition tabbable inserter * Fix e2e

* Block Library: Add a Post Excerpt block. * Block Library: Make Post Excerpt block editable. * E2E Tests: Add Post Excerpt block fixtures.

* Components: Add isFocusable state to Button * Make isFocusable true by default * Update styles * Remove stopImmediatePropagation call * Fix unit tests * Fix e2e tests * Make isFocusable prop experimental and false by default

* Improved style of image block placeholder during upload * Improved style of VIDEO block placeholder during upload * Removing unreachable code. * Removed redundant style props

…location (WordPress#19516)

* adds media replace flow to the cover block * adds video suppor to upload in media replace control

* Block: try merging effects * Merge multi selected effect

StevenDufresne · 2020-02-03T03:36:17Z

Ah dang, this went poorly. I'm going to close and reopen.

StevenDufresne · 2020-02-03T04:24:12Z

Reopened here: #20001

StevenDufresne added 9 commits January 10, 2020 08:44

Block Directory: Add tests for install, uninstall and download block …

5f2112a

…actions.

Add in the ability to set error notices into the state.

8b12043

Add a notice component that should show errors inline. Also add a loa…

3217e52

…ding state when the installing is happening and make the 'Add block' button show a loading state while installing

Separate the installAndDownload function so we can make a download on…

2f3b81b

…ly retry call from the notice component.

Update install tests because of new install triggers. Remove uninstal…

f977e18

…l test since its not used and therefore could introduce debt.

Add some tests for the block items that are affected by the change.

78af7c3

Remove a commented out component from test.

835af58

Update the block notice to fix an issue with the full install callbac…

2377468

…k. Add tests for that component.

Remove empty new line added.

2da9406

StevenDufresne marked this pull request as ready for review January 13, 2020 06:50

StevenDufresne changed the title ~~Try/block directory inline error msgs~~ Block Directory: Try/block directory inline error msgs. Jan 13, 2020

sirreal and others added 19 commits January 13, 2020 09:29

Packages: Mark build-styles as side-effectful (WordPress#19535)

701c277

* Declare sideEffects for build-style * fixup! Declare sideEffects for build-style * Add CHANGELOG entries

RichText: don't set focus when applying format (WordPress#19536)

2380f2d

* Return focus to original element * Fix list e2e tests * Fix annotation e2e tests * Fix some rich text e2e tests * Fix tests * Focus after link * Add onFocus * Corrections

Fix the height of the tags tokens (WordPress#19592)

6e42279

Update changelogs

ce2fce1

Adds a "(no title)" label to links to pages or posts with no title (W…

e7342de

…ordPress#19528) * adds no title label to links to pages or posts with no title * fixes a wrong return and an explicit comparison * undo mistaken CSS edit

Block: reposition tabbable inserter (WordPress#19596)

06782e1

* Block: reposition tabbable inserter * Fix e2e

Block Library: Add a Post Excerpt block. (WordPress#19579)

5e2ac97

* Block Library: Add a Post Excerpt block. * Block Library: Make Post Excerpt block editable. * E2E Tests: Add Post Excerpt block fixtures.

[RNMobile] Fix media upload placeholder (WordPress#19497)

4aad528

* Improved style of image block placeholder during upload * Improved style of VIDEO block placeholder during upload * Removing unreachable code. * Removed redundant style props

RNMobile: Consolidate logic hiding blocks from release builds to one …

adf6735

…location (WordPress#19516)

Remove onSelectURL (WordPress#19606)

4a3ac85

Adds media replace flow to the cover block (WordPress#19583)

67db0c6

* adds media replace flow to the cover block * adds video suppor to upload in media replace control

Fix Navigable Container component usage code (WordPress#19615)

d89119d

Block: try merging effects (WordPress#19617)

5338c47

* Block: try merging effects * Merge multi selected effect

Editor keyboard shortcuts: fix Toggle Sidebar (WordPress#19605)

4ecf279

Fix focus capture (WordPress#19621)

8f7d582

StevenDufresne closed this Feb 3, 2020

StevenDufresne mentioned this pull request Feb 3, 2020

Block Directory: Add error messages inline. #20001

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Directory: Try/block directory inline error msgs. #19589

Block Directory: Try/block directory inline error msgs. #19589

StevenDufresne commented Jan 13, 2020

StevenDufresne commented Feb 3, 2020

StevenDufresne commented Feb 3, 2020

Block Directory: Try/block directory inline error msgs. #19589

Block Directory: Try/block directory inline error msgs. #19589

Conversation

StevenDufresne commented Jan 13, 2020

Description

Background Info

Types of changes

Changes:

Why?

Cons

Known Issues

Screenshots

How has this been tested?

Manual Testing

Case Success

Case Install Error:

Case Download Error:

Checklist:

StevenDufresne commented Feb 3, 2020

StevenDufresne commented Feb 3, 2020