Skip to content

Conversation

@jeherve
Copy link
Member

@jeherve jeherve commented Aug 5, 2020

Changes proposed in this Pull Request:

The contents of the dashboard notices is not always a string. It can be a React element.
When that's the case, let's avoid any errors by not using the replace method which is only available for strings.

Jetpack product discussion

  • N/A

Does this pull request change what data or activity we track or use?

  • N/A

Testing instructions:

Proposed changelog entry for your changes:

  • Admin Page: avoid blank dashboard when some specific notices (such as Offline mode) are displayed.

Follow-up of #16690

The contents of the dashboard notices is not always a string. It can be a React element.
When that's the case, let's avoid any errors by not using the replace method which is only available for strings.
@jeherve jeherve added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu [Pri] High labels Aug 5, 2020
@jeherve jeherve added this to the 8.8.1 milestone Aug 5, 2020
@jeherve jeherve requested a review from sergeymitr August 5, 2020 07:53
@jeherve jeherve self-assigned this Aug 5, 2020
@jetpackbot
Copy link
Collaborator

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16721

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against 1209386

Copy link
Contributor

@brbrr brbrr left a comment

Choose a reason for hiding this comment

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

At first, I saw the original error and compiled JS has not included changes from this PR. After hard reload it worked like a charm

@brbrr brbrr added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 5, 2020
@jeherve jeherve merged commit 9e5f06c into master Aug 5, 2020
@jeherve jeherve deleted the fix/error-notice-string branch August 5, 2020 11:41
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 5, 2020
jeherve added a commit that referenced this pull request Aug 5, 2020
… string (#16721)

Follow-up of #16690

The contents of the dashboard notices is not always a string. It can be a React element.
When that's the case, let's avoid any errors by not using the replace method which is only available for strings.
@jeherve
Copy link
Member Author

jeherve commented Aug 5, 2020

Cherry-picked to branch-8.8 in e32ebbc

davidlonjon added a commit that referenced this pull request Aug 6, 2020
…ic/jetpack into add/jetpack-lazy-images-package

* 'add/jetpack-lazy-images-package' of github.com:Automattic/jetpack: (40 commits)
  Lodash: Revert to previous version (#16735)
  New class to handle async XML-RPC requests (#16674)
  Social Previews: Sidebar design updates (#16725)
  Update E2E locator for classic connection flow (#16708)
  Woo Services: update to use existing Jetpack plugin install tools (#16672)
  Admin Page: avoid blank dashboard when notice is not a string (#16721)
  Admin Page: update string still using old i18n format (#16722)
  Social Previews: Add sidebar UI (#16633)
  Fix recurring payments block disconnecting (sometimes) when existing article is reopened in block editor. (#16640)
  Connection Register: Add current user email to connection register request (#16712)
  Update versions to start 8.9 Release cycle (#16706)
  Donations block: Make currency and amounts editable (#16593)
  Update dependency @automattic/calypso-color-schemes to v2
  Error Notice: removing HTML code, adjusting maximum width. (#16690)
  Update dependency swiper to v6 (#16587)
  Site Scan: Short-circuit update_threats_link() if Admin Bar is not present (#16677)
  Update vulnerable NPM packages (#16659)
  E2E Tests: Add Jetpack updater test (#16437)
  check for subdir site before rendering Ads.txt section (#16671)
  VideoPress Block: Retain alignment support (#16651)
  ...
netsuso pushed a commit to Automattic/vip-go-mu-plugins that referenced this pull request Aug 6, 2020
jeherve added a commit that referenced this pull request Aug 10, 2020
pereirinha pushed a commit that referenced this pull request Sep 10, 2020
… string (#16721)

Follow-up of #16690

The contents of the dashboard notices is not always a string. It can be a React element.
When that's the case, let's avoid any errors by not using the replace method which is only available for strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu [Pri] High [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants