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

Add error boundary to experimental navigation screen #23679

Merged
merged 3 commits into from
Jul 7, 2020

Conversation

adamziel
Copy link
Contributor

@adamziel adamziel commented Jul 3, 2020

Description

Solves #23417 by adding an error boundary to inform the user about any problems (instead of displaying the blank page).

There is a similar component in the editor package, but it's editor-specific enough to justify creating a "private" component for the purposes of edit-navigation package. Bringing a configurable ErrorBoundary component to @wordpress/components is definitely worth discussing as there are more places that could benefit from it (e.g. the widgets screen).

How has this been tested?

  1. Update e.g. navigation editor to throw an error
  2. Go to the navigation screen
  3. Confirm you see the following error message:

Zrzut ekranu 2020-07-3 o 15 13 03

Types of changes

New feature (non-breaking change which adds functionality)

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.

@adamziel adamziel self-assigned this Jul 3, 2020
@adamziel adamziel requested review from talldan, draganescu and noisysocks and removed request for talldan and draganescu July 3, 2020 13:17
@adamziel adamziel added [Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Feature] Navigation Screen labels Jul 3, 2020
@adamziel adamziel removed the [Feature] List View Menu item in the top toolbar to select blocks from a list of links. label Jul 3, 2020
@github-actions
Copy link

github-actions bot commented Jul 3, 2020

Size Change: +8.41 kB (0%)

Total Size: 1.14 MB

Filename Size Change
build/annotations/index.js 3.67 kB +45 B (1%)
build/api-fetch/index.js 3.39 kB -9 B (0%)
build/autop/index.js 2.82 kB +1 B
build/block-directory/index.js 7.67 kB +513 B (6%) 🔍
build/block-editor/index.js 115 kB +6.38 kB (5%) 🔍
build/block-editor/style-rtl.css 10.8 kB +85 B (0%)
build/block-editor/style.css 10.8 kB +85 B (0%)
build/block-library/editor-rtl.css 7.57 kB -48 B (0%)
build/block-library/editor.css 7.57 kB -48 B (0%)
build/block-library/index.js 130 kB +227 B (0%)
build/block-serialization-default-parser/index.js 1.88 kB -3 B (0%)
build/block-serialization-spec-parser/index.js 3.1 kB -5 B (0%)
build/blocks/index.js 48.2 kB +70 B (0%)
build/components/index.js 199 kB +42 B (0%)
build/components/style-rtl.css 15.8 kB +4 B (0%)
build/components/style.css 15.8 kB +4 B (0%)
build/compose/index.js 9.56 kB -85 B (0%)
build/core-data/index.js 11.4 kB +9 B (0%)
build/data/index.js 8.46 kB +13 B (0%)
build/date/index.js 5.38 kB -89 B (1%)
build/dom/index.js 3.23 kB +38 B (1%)
build/edit-navigation/index.js 10.8 kB +843 B (7%) 🔍
build/edit-navigation/style-rtl.css 1.08 kB +59 B (5%) 🔍
build/edit-navigation/style.css 1.08 kB +59 B (5%) 🔍
build/edit-post/index.js 304 kB +221 B (0%)
build/edit-site/index.js 16.5 kB -98 B (0%)
build/edit-widgets/index.js 9.35 kB +25 B (0%)
build/editor/index.js 45 kB +227 B (0%)
build/editor/style-rtl.css 3.78 kB +4 B (0%)
build/editor/style.css 3.78 kB +5 B (0%)
build/element/index.js 4.65 kB -2 B (0%)
build/format-library/index.js 7.71 kB -14 B (0%)
build/hooks/index.js 2.13 kB +1 B
build/is-shallow-equal/index.js 709 B -1 B
build/keyboard-shortcuts/index.js 2.52 kB +4 B (0%)
build/list-reusable-blocks/index.js 3.12 kB +3 B (0%)
build/media-utils/index.js 5.32 kB +23 B (0%)
build/notices/index.js 1.79 kB +1 B
build/nux/index.js 3.4 kB +1 B
build/plugins/index.js 2.56 kB +2 B (0%)
build/primitives/index.js 1.4 kB -98 B (6%)
build/priority-queue/index.js 789 B +1 B
build/redux-routine/index.js 2.85 kB -4 B (0%)
build/rich-text/index.js 13.9 kB -107 B (0%)
build/server-side-render/index.js 2.71 kB +36 B (1%)
build/shortcode/index.js 1.7 kB +1 B
build/token-list/index.js 1.28 kB +1 B
build/url/index.js 4.06 kB -4 B (0%)
build/viewport/index.js 1.85 kB -1 B
build/warning/index.js 1.13 kB -4 B (0%)
build/wordcount/index.js 1.17 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-library/style-rtl.css 7.78 kB 0 B
build/block-library/style.css 7.79 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/data-controls/index.js 1.29 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/edit-post/style-rtl.css 5.57 kB 0 B
build/edit-post/style.css 5.57 kB 0 B
build/edit-site/style-rtl.css 3.03 kB 0 B
build/edit-site/style.css 3.03 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/keycodes/index.js 1.94 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/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B

compressed-size-action

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Need to check that onError isn't undefined, but otherwise this is good. Approving in advance so that I don't hold you up.

Agree that a component which edit-post, edit-navigation and edit-widgets shares would be good. It could contain both Attempt recovery and Copy error.

}

reboot() {
this.props.onError();
Copy link
Member

@noisysocks noisysocks Jul 7, 2020

Choose a reason for hiding this comment

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

⚠️ Need to check for undefined.

Suggested change
this.props.onError();
if ( this.props.onError ) {
this.props.onError();
}

@adamziel adamziel merged commit 517c34c into master Jul 7, 2020
@adamziel adamziel deleted the add/navigation-screen-error-boundary branch July 7, 2020 14:31
@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants