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 "Gathering data" output to DataBlock component and update Site Kit Dashboard usage #4692

Closed
tofumatt opened this issue Jan 20, 2022 · 12 comments
Labels
P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented Jan 20, 2022

Feature Description

In the Figma mockups for Zero Data States, all DataBlock components show "Gathering data…" text instead of their stat when they are gathering data. In order to allow components to render this, we should add a new prop gatheringData to the DataBlock component that will render the "gathering data…" text as shown:

CleanShot 2022-01-20 at 19 52 27

We should add this functionality to all relevant components at the same time, both to make this easier to test. That will make this issue a bit larger but I think will overall streamline it and make it easier to track.

Note: the Figma designs/screenshots show "gathering data…." (with four full-stops), but they're slightly off. The ACs/IB in this issue use the text "gathering data…" with an ellipsis () character. The ellipsis version should be used in this issue.


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

Acceptance criteria

  • The changes for this issue should only happen when the zeroDataStates feature flag is enabled; when that flag is not enabled, the component should behave the same as before. Create a zeroDataStates feature flag #4726
  • The DataBlock component should support showing a "gathering data" state by usage of a gatheringData boolean prop. This prop should be false by default.
  • When the gatheringData prop is set to true, the "gathering data…" output as shown in the Figma mocks/screenshot above should be shown instead of any data supplied to the component.
  • There should be new Storybook stories for:
    • SearchFunnelWidget gathering Search Console data
  • Update all <DataBlock /> instances outside of the WordPress Dashboard (/wp-admin/) and the Admin Bar to receive the gatheringData prop from their parent component if the component does/can check for select( module ).isGatheringData() and isGatheringData is true. Related issue Update DataBlock component usage to use the "Gathering data" prop #4711 will deal with the WP Dashboard/Admin Bar instances.

Implementation Brief

  • The change below should only happen when the zeroDataStates feature flag is enabled.
  • Create assets/js/components/GatheringDataNotice.js which exports the GatheringDataNotice functional component.
    • It should accept the following props:
      • size prop which can be either small or normal.
      • noBorder boolean prop which is false by default.
      • overlay boolean prop which is false by default.
    • It should render the gathering data… message as per the designs in Figma on both, small and large screens, wrapped within a p tag, itself wrapped within a div having the googlesitekit-gathering-data-notice class.
    • All the styles should go in assets/sass/components/global/_googlesitekit-gathering-data-notice.scss.
      • For the small size, refer to the styled of the notice displayed in the Acquisition section.
      • For the normal size, the notice should be styled as per the designs. Refer to the notice displayed under "Total impressions"
      • If noBorder prop is false, there should be a bottom border for the notice.
      • If overlay is true, the notice should be styled as an overlay. Refer to how it is displayed on top of the graph for the Search Funnel widget.
    • If possible look into animating the dots at the end of the message. (Some inspiration: https://nzbin.github.io/three-dots/)
  • Using assets/js/components/DataBlock.js,
    • Add the boolean gatheringData prop which is false by default.
    • If gatheringData is true, render the GatheringDataNotice and do not render elements having the following selectors:
      • .googlesitekit-data-block__datapoint
      • .googlesitekit-data-block__change-source-wrapper
      • .googlesitekit-data-block__sparkline
  • Update the usage of DataBlock in the widgets below and check if its parent component does/can check for select( module ).isGatheringData() and its value should be passed to the gatheringData prop. Widgets to look at:
    • SearchFunnelWidget
    • AdSense ModuleOverviewWidget
    • DashboardOverallPageMetricsWidget
  • Update stories for the above widgets to include state when gatheringData is true.

Test Coverage

  • No new tests to be added.

QA Brief

  • Setup a new site with no data.
  • Enable the zeroDataStates feature flag.
  • Setup the Analytics module.
  • Verify that the search funnel widget on the main dashboard show the "gathering data..." message.
  • Go to the entity dashboard and check if the DashboardOverallPageMetricsWidget component is rendering the "gathering data..." message.
  • The color for the "gathering data..." message in Figma does not pass accessibility standards. An alternative color has been used.

Changelog entry

  • Update DataBlock component to display the new gathering state.
@asvinb
Copy link
Collaborator

asvinb commented Jan 21, 2022

@tofumatt In the AC, it says:

Update all instances ...

Turns out we use the DataBlock in many instances, for e.g in the AdminBar* components. Do we want to have the "gathering data..." message displayed in those blocks as well or just the main ones on the dashboard such as the SearchFunnelWidget and ModuleOverviewWidget?

@asvinb asvinb assigned tofumatt and unassigned asvinb Jan 21, 2022
@felixarntz
Copy link
Member

@asvinb The "gathering data..." should be displayed on any DataBlocks where the respective service/API still isGatheringData (via the respective module store's selector). So I'd say yes, eventually this should be on all DataBlocks. Let's review how much effort that is - potentially we can say that this issue is only for the main dashboard and entity dashboard and then we can have follow-up one for other areas (e.g. WP dashboard widget and admin bar). What do you think @tofumatt?

@tofumatt
Copy link
Collaborator Author

tofumatt commented Jan 24, 2022

I see twenty files in total where we need to update the usage, so I'm on the fence about creating a new issue for the remaining ones. It'll be easier to QA everything all in one commit (easier for QA to spot a place where it's missing, for instance). Also: the approach will be similar throughout, so I figured it'd be best to do it all in one fell swoop.

I think most of the usage is outside the Admin bar—I only see four AdminBar instances, and only four on the WP Dashboard as well.

At that rate I think it's better to just combine everything into one issue 🙂


If you still think it's too much for a single issue though, let me know and maybe we can convert just one widget or something to start with, and file a new one for everywhere else.

@tofumatt tofumatt assigned asvinb and unassigned tofumatt Jan 24, 2022
@felixarntz
Copy link
Member

@tofumatt My main point is that the way other areas look, how their layout is, how much space there is available etc is quite different from the Site Kit dashboard. I can see how the design we've defined for DataBlocks in the dashboard may potentially look odd in the small WP dashboard widget or the admin bar UI.

So I'm just conscious of blocking everything from potential issues in just a part of it. That's why my suggestion would be to update all DataBlocks in all Site Kit dashboard (main and entity dashboard) widgets, but have a separate one for DataBlocks in other areas (WP dashboard and admin bar).

It's less about the amount of work to do, but more about the implications of the changes, potential problems etc. What do you think?

@tofumatt tofumatt changed the title Add "Gathering data" output to DataBlock component Add "Gathering data" output to DataBlock component and update Site Kit Dashboard usage Jan 24, 2022
@tofumatt
Copy link
Collaborator Author

tofumatt commented Jan 24, 2022

Ah I see, sorry I misunderstood!

Yes, in that case that makes total sense. Okay, let's do everything except WP Dashboard and Admin Bar in this issue. 👍🏻

(I've updated the ACs here to reflect this approach.)

@asvinb Can you write that into the IB? I've filed #4711 to take care of the WP Dashboard and Admin Bar instances of <DataBlock />, so this issue can address all other instances of it 🙂

@asvinb asvinb assigned tofumatt and unassigned asvinb Jan 25, 2022
@asvinb
Copy link
Collaborator

asvinb commented Jan 25, 2022

Thanks @tofumatt IB updated!

@tofumatt
Copy link
Collaborator Author

Looks great @asvinb! IB ✅

Just one thing to note here: the text should use an ellipsis so it's "gathering data…" and not "gathering data…." with four full-stops. (I've added it to the issue as well; it was mentioned in the ACs but kinda subtly.)

I think that was a mistake in the Figma docs, but we should go with the ellipsis character.

@tofumatt tofumatt removed their assignment Jan 25, 2022
@tofumatt
Copy link
Collaborator Author

Note that I added in the ACs/IB changes below should only happen when the unifiedDashboard feature flag is enabled; otherwise the component should behave as before. I forgot to add that for this issue.

It might be good to actually pass props to <DataBlock /> either way but only have it change its output based on the feature flag, to reduce the places we need to check with useFeature

@eugene-manuilov eugene-manuilov removed their assignment Feb 23, 2022
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Feb 28, 2022
@wpdarren wpdarren self-assigned this Feb 28, 2022
@wpdarren
Copy link
Collaborator

wpdarren commented Mar 2, 2022

QA Update: ❌

@asvinb I have a number observations and realise they could be because we're still engineering and/or figma design 😄

  1. On the main and entity dashboard: the 'gathering data...' text does not appear in the graph. It does in the figma design.

These issues are possibly out of scope for this ticket but wanted to highlight them so it can be confirmed if we need to create new tickets or if these belong to another ticket still being engineered.

  1. On the main dashboard, when Analytics is not connected, the CTA within the widget does not match the figma design.
  2. On the main and entity dashboard, the graph does not have any dates on the x-axis. It does on the figma design.

image

  1. When Analytics is connected, there's a widget that appears above the search funnel widget, that says gathering data in the old style design. This appears on main and entity dashboard. See screenshot below.

image

  1. On the main and entity dashboard, with Analytics connected. When you click the Unique visitors from search tab, there's an error: Table has no columns. then when you click on any of the other tabs, the error below appears.

image

image

@asvinb
Copy link
Collaborator

asvinb commented Mar 3, 2022

@wpdarren Good questions. Kindly find me feedback below:

  1. For the graphs, this will be taken care of in Add a "gatheringData" prop to the GoogleChart and Sparkline components #4696
  2. The CTA is being taken care of in Update Blue Box CTA components to more visually-illustrative versions #4694
  3. Most likely will be taken care of in Add a "gatheringData" prop to the GoogleChart and Sparkline components #4696
  4. The current issue is to add the "gathering data..." message to the DataBlocks component only and the issue about the widget above the DataBlocks is out of scope but most likely it'll be taken care of in one of the tickets in the epic Update all Widgets to pass isGatheringData prop to chart/table/other components #4697.
  5. This should be a non issue since we have an updated layout for it: Update Blue Box CTA components to more visually-illustrative versions #4694

Let me know if you need any more info.

Thanks!

@asvinb asvinb assigned wpdarren and unassigned asvinb Mar 3, 2022
@wpdarren
Copy link
Collaborator

wpdarren commented Mar 3, 2022

@asvinb thank you for taking the time to look at the points in my comment, appreciate it.

I will make a note of these and move this ticket on since it passes the QAB.

@wpdarren
Copy link
Collaborator

wpdarren commented Mar 3, 2022

QA Update: ✅

Verified:

  • The search funnel widget on the main dashboard shows the "gathering data..." message.
  • We don't appear to have figma designs for the entity dashboard but can confirm that the "gathering data..." message is appearing and is the same styles as the main dashboard.
Screenshots

image

image

@wpdarren wpdarren removed their assignment Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants