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

Error state for individual Key Metric Widget Tiles #7310

Closed
jimmymadon opened this issue Jul 14, 2023 · 5 comments
Closed

Error state for individual Key Metric Widget Tiles #7310

jimmymadon opened this issue Jul 14, 2023 · 5 comments
Labels
P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@jimmymadon
Copy link
Collaborator

jimmymadon commented Jul 14, 2023

Feature Description

Key Metric widget tiles are much smaller than our usual widgets and hence require new error components to be designed as per the Figma mocks.

This issue was split out from #7149 since point 2 in that issue's AC was making things quite complicated.

This issue (#7310) focuses on implementing the error state for individual KMW tiles as per the following screenshot:
Screenshot 2023-07-11 at 17 59 49

The more complicated scenario which "combines" the error state together in the rare case that all widget tiles throw an error will be tackled in #7311:
Screenshot 2023-06-09 at 13 06 57


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • If any of the Key Metric widget tiles throws an error, an error component should be displayed as per this Figma mock.
    • Clicking on the "Retry" button should behave like our existing ReportErrorActions component invalidating the resolution of the selectors that threw an error thereby attempting to resolve those selectors again.
    • Clicking on the "Get Help" link should also behave like our existing ReportErrorActions component and use the getErrorTroubleshootingLinkURL selector.
  • Any Key Metric widget tiles implemented already should have this functionality added to them. Tiles implemented after this feature is executed will reuse the components created as part of this issue.

Implementation Brief

ℹ️ POC Pull Request

Please note that due to the nature of discovery for IB on this issue, code changes were naturally explored. As a result a POC pull request was opened demonstrating a fully working error state for the <LoyalVisitorsWidget> component. Please see #7324. Please note, however, that this is merely a proof of concept and should be taken merely as a guideline for implementation.


While the existing <WidgetReportError> could potentially be utilised and will correctly render an error message, error description and correct "Retry" handler, it will not display and format the contents in a means that satisfies the Figma design for a single key metric widget error state.

Looking into the <WidgetReportError> component, and ultimately the <ReportError> component (used by the <WidgetReportError> component), the following components could be utilised when creating a new error component for Key Metrics Widget tiles: <CTA> and <ReportErrorActions>. These two components can be combined within said new error component, and be styled as per the Figma design. The CTA component will need to be updated in order to receive the header text/label text above the error title, described in more detail below.

The following guidelines should be followed to satisfy this issue:

  1. Create a new <MetricTileError> component that is to be rendered in place of the contents of a Key Metrics Widget tile when an error state is encountered.
    • Said component should accept the following props:
      • <object|array> error the error object passed directly from the individual Key Metric Widget tile component, should an error occur.
      • headerText the header text/label text to be rendered above the loading error title within the error component, which will likely be the title prop of the key metric widget in question that is erroring. This can be named accordingly as the author sees fit, as long as it is retained throughout the steps to follow.
      • moduleSlug the moduleSlug passed down from the kmw tile, necessary as the base tiles such as WidgetTileNumeric may not be used for a single module slug only.
    • This component can likely reside at assets/js/components/KeyMetrics/MetricTileError.js
    • Import the following internal components that will be used as the based for the rendering of this component: <CTA>, <ReportErrorActions>.
    • The return of this component should make use of the <CTA> and <ReportErrorActions> components. The new headerText prop (see sections to follow around updating the CTA component) should be passed to the <CTA> component. The error prop as received in the <MetricTileError> component should be forwarded to the <CTA> component, and the title prop of the <CTA> component should be hard coded to __( 'Data Loading Failed', 'google-site-kit' ) as per the Figma design.
    • For the <ReportErrorActions> component nested within the <CTA> component, the moduleSlug and error props as received by the <MetricTileError> component should be forwarded to the <ReportErrorActions> component, i.e error={ error } moduleSlug={ moduleSlug }.
    • Style the <MetricTileError> component and it's elements according to the Figma design. Use the existing SASS files (and add as necessary) at assets/sass/components/key-metrics. Add necessary wrapper elements to target/override CSS styling rules, take note that the elements within the flex box will need to wrap, and the order of the button and help text elements should be reversed.
    • See the <MetricTileError> component in the POC PR.
  2. Update the existing <CTA> component (located at assets/js/components/notifications/CTA.js) to accept a new headerText prop, to be used to render pre-title heading/label text as per the Figma design. Said prop should be of type string, default to '' and be rendered prior to title (if present) in an applicable element wrapper.
    • See the updated <CTA> component in the POC PR.
  3. Add a means to check for error states within each of the Key Metrics Widget components. This can be achieved via the getErrorForSelector method within a useSelect implementation using the MODULES_ANALYTICS_4 module.
    • See such a change within the <LoyalVisitorsWidget> component in the POC PR here and here.
  4. Update each applicable key metrics widget component within assets/js/modules/analytics-4/components/widgets with the above logic to check for errors, and pass the error as an error prop on the final key metric widget return component, i.e error={ error }. A moduleSlug prop should also be passed to the the final key metric widget return component so that correct retry actions can be executed if an error state is encountered, and said moduleSlug should be the slug of the module in which the widget is being rendered within, i.e "analytics-4" for the <LoyalVisitorsWidget>. However, given this is defined at the singular KMW tile level, and passed down to the eventual <MetricTileError> component.
    • This should apply to all key metrics widgets component types, i.e <MetricTileNumeric>, <MetricTileTable> and <MetricTileText>. Within each of these components, the newly added error and moduleSlug props should be forwarded to the final <WidgetTileError> component. See such a change to the <MetricTileNumeric> in the POC PR here.
    • Within each of these base widget tile components, the props should be updated to accept an error prop of type oneOf PropTypes.arrayOf( PropTypes.object ) or PropTypes.object, and a moduleSlug prop of type PropTypes.string.isRequired. The error prop should not be required.
    • If there is the presence of an error object (via the error prop), said widget should render the <MetricTileError> component, forwarding the title, error, headerText and moduleSlug props as passed to said widget rendering the <MetricTileError> component.

📚 New Story State/Scenario Requirements (Including VRT Validation)

Given that the error states for individual Key Metrics Widgets are new, existing stories should be updated to include error state stories, each with a VRT scenario.

The following current stories need updating to include error states:

  • assets/js/modules/analytics-4/components/widgets/LoyalVisitorsWidget.stories.js
  • assets/js/modules/analytics-4/components/widgets/NewVisitorsWidget.stories.js
  • assets/js/modules/analytics-4/components/widgets/PopularContentWidget.stories.js
  • assets/js/modules/search-console/components/widgets/PopularKeywordsWidget.stories.js
  • assets/js/modules/analytics-4/components/widgets/TopConvertingTrafficSourceWidget.stories.js
  • assets/js/modules/analytics-4/components/widgets/TopTrafficSourceWidget.stories.js
  • Top Cities Widget is still WIP and pending completion, but should be actioned once Create the "Top cities driving traffic" key metric widget tile #6252 is complete.

For each of the existing stories above, the following should be undertaken:

  • An Error story with accompanying scenario should be created.
    • Errors are to be simulated via the dispatch object used by existing story args
    • See existing usage of dispatch().receiveError() for inspiration, as used in unrelated stories such as assets/js/modules/analytics/components/dashboard/DashboardOverallPageMetricsWidgetGA4.stories.js

Once Error states have been created, validate that they appear as part of the VRT tests suite. Visual references will likely need to be added as this will be the first run of VRT tests for said Error states within the above stories.

Test Coverage

VRT Test

  • VRT tests should be automatically covered via implementation of the Error scenarios in the stories section above.

Unit Tests

  • N/A No unit tests are required for this issue.

QA Brief

In order to successfully QA the outcome of this issue, a proxy will be required in order to modify HTTP responses and force error states within various components. The Tweak Google Chrome extension should be installed and activated prior to proceeding. Once installed and activated, perform the following steps:

  • Via the Site Kit Tester Settings plugin, enable the userInput and ga4Reporting feature flags.
  • Visit the Google Site Kit dashboard.
  • Open the Tweak extension window. Ensure that the Regex options is enabled via the button that reads looks as follows:
image
  • Add the following URL pattern into the URL input: ^(.*)\/wp-json\/google-site-kit\/v1\/modules\/(.*)$
  • Ensure that the response remains as GET but that the response code is set to 400.
  • Update the Response Payload to be as follows:
{
  "code": 400,
  "message": "Generic Test Erorr",
  "data": {
    "status" : 400,
    "reason": "badRequest"
  }
}

Toggle on the rule via the play icon near the top of the extension window. The final state of the Tweak extension window should look as follows:

image
  • With the Tweak rule in place and active, clear your local cookies and session data via the Application tab of the Chrome developer tools console:

Site Kit by Google Dashboard ‹ sitekit 10uplabs com — WordPress 2023-08-02 at 2 10 52 PM

  • Refresh the Site it dashboard, you should be forced to log in again.
  • Following the log in, you should be presented with error states on the KMW tiles as follows:
image
  • In order to enable viewing of all KMW tiles, copy and past the following JavaScript snippet into your developer toosl console:
await googlesitekit.data.dispatch('core/user').setKeyMetricsSetting('widgetSlugs', ['kmAnalyticsAdSenseTopEarningContent', 'kmAnalyticsEngagedTrafficSource', 'kmAnalyticsLoyalVisitors', 'kmAnalyticsNewVisitors', 'kmAnalyticsPopularContent', 'kmAnalyticsPopularProducts', 'kmAnalyticsTopCities', 'kmAnalyticsTopConvertingTrafficSource', 'kmAnalyticsTopCountries', 'kmAnalyticsTopTrafficSource', 'kmSearchConsolePopularKeywords'] );
await googlesitekit.data.dispatch('core/user').saveKeyMetricsSettings();
  • This should then render all KMW tiles, and all (complete to date) tiles should error as follows:
image
  • Validate that each KMW tile error state renders according to the designs outline in the "Acceptance Criteria" section further above.

Changelog entry

  • Add error and retry UI for Key Metric Widgets.
@jimmymadon jimmymadon added P0 High priority Type: Enhancement Improvement of an existing feature labels Jul 14, 2023
@jimmymadon jimmymadon self-assigned this Jul 14, 2023
@jimmymadon jimmymadon changed the title Error state for Key Metric Widget / Tiles Error state for individual Key Metric Widget Tiles Jul 14, 2023
@jimmymadon jimmymadon assigned 10upsimon and unassigned jimmymadon Jul 14, 2023
@10upsimon 10upsimon assigned techanvil and 10upsimon and unassigned 10upsimon and techanvil Jul 17, 2023
@techanvil
Copy link
Collaborator

techanvil commented Jul 20, 2023

Hi @10upsimon, nice work so far here on the IB front. A few notes:

  • We do need to pass moduleSlug in from the widgets through to ReportErrorActions, as in addition to the GA4 widgets we presently do have a Search Console widget, PopularKeywordsWidget, and we'll have an AdSense one down the line too. We may yet need to support multiple moduleSlugs as alluded to in my previous feedback on Error state for Key Metric Widget / Tiles #7149, depending on how Create the "Top earning content" key metric widget tile #6248 turns out (and indeed Combined error state when ALL Key Metric Widget Tiles throw an error #7311), but we can cross that bridge if we come to it.
  • Obviously the POC is just that, but I've left a few minor comments on it for future reference.
  • This is a very minor one, but I find the wording on the following point slightly confusing, as the inclusion of a scenario definition on a story is what defines the VRT for that story. The wording here makes it sound as though the inclusion in the VRT suite is automatic but somewhat incidental to the stories/scenarios, but it's really part and parcel of the scenario definition.
    • Given that the error states for individual Key Metrics Widgets are new, existing stories should be updated to include error state stories and scenarios, which will be automatically included as part of visual tests (VRT) as well.

    • I'd suggest a slight simplification along these lines:
    • Given that the error states for individual Key Metrics Widgets are new, existing stories should be updated to include error state stories, each with a VRT scenario.

  • assets/js/modules/search-console/components/widgets/PopularKeywordsWidget.stories.js is missing from the list of story files to update.

@techanvil techanvil assigned 10upsimon and unassigned techanvil Jul 20, 2023
@10upsimon
Copy link
Collaborator

Much appreciated @techanvil, I am iterating based on your last reply.

@10upsimon
Copy link
Collaborator

@techanvil I have updated the IB to include:

  • References to moduleSlug, where it is defined (i.e in what components) and how it is passed down from KMW tiles down to <MetricTileError> when necessary, and all props and PropTypes updates required.
  • Fixes to the stories reference list to include the missing story file at assets/js/modules/search-console/components/widgets/PopularKeywordsWidget.stories.js.
  • Fixes to the stories wording to be less ambiguous as per your feedback.

I've also updated the POC PR to include examples of such moduleSlug implementation, and also addressed your other comments within the PR, notably within these commits:

Thank you for your continued thoroughness!

@10upsimon 10upsimon assigned techanvil and unassigned 10upsimon Jul 21, 2023
@techanvil
Copy link
Collaborator

Thanks @10upsimon! Nice work here, happy to give this the old LGTM and move over to the EB. 🎉

IB ✅

@techanvil techanvil removed their assignment Jul 24, 2023
@10upsimon 10upsimon self-assigned this Jul 24, 2023
@10upsimon 10upsimon removed their assignment Aug 2, 2023
@tofumatt tofumatt self-assigned this Aug 3, 2023
@tofumatt tofumatt removed their assignment Aug 3, 2023
@wpdarren wpdarren self-assigned this Aug 4, 2023
@wpdarren
Copy link
Collaborator

wpdarren commented Aug 6, 2023

QA Update: ✅

Verified:

  • When the Key Metric widget tiles throws an error, an error component is displayed as per this Figma
  • Clicking on the Retry button behaves like existing ReportErrorActions component invalidating the resolution of the selectors that threw an error thereby attempting to resolve those selectors again. When clicked the tile displays data.
  • Clicking on the Get Help link behaves like our existing ReportErrorActions component and uses the getErrorTroubleshootingLinkURL selector. A new opens up and points to the Site Kit trouble shooting page.
  • All Key Metric widget tiles implemented have this functionality added to them.
  • Tested UI on desktop and smaller viewports.

Note: the Get Help link text is all in lower case on the figma designs but I feel its correct how it's appearing on my test site as 'Get help' as this matches our other error messages.

Screenshots

image
image
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants