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

Scaffold new Unified Dashboard widget contexts and widget areas #4031

Closed
tofumatt opened this issue Sep 13, 2021 · 10 comments
Closed

Scaffold new Unified Dashboard widget contexts and widget areas #4031

tofumatt opened this issue Sep 13, 2021 · 10 comments
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling
Milestone

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented Sep 13, 2021

Feature Description

Each new section on the main dashboard and entity dashboard should get its own widget context. We should create a new widget context for both the Main Dashboard and the Entity Dashboard. Using new contexts/variables will make it easier to sort out old/new usage, registrations, etc.

Each context should have a single Widget area that all initial widgets will use. A Primary suffix for each area, based on the context name, should be sufficient for now (eg. MainDashboardTrafficPrimary). This gives us the ability to add more widget areas later—but for now we’ll use a single area.


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

Acceptance criteria

  • The DashboardMainApp should have the following new widget contexts, rendered one after the other, in this order:
    • mainDashboardTraffic
    • mainDashboardContent
    • mainDashboardSpeed
    • mainDashboardMonetization
  • The DashboardEntityApp should have the following new widget contexts, rendered one after the other, in this order:
    • entityDashboardTraffic
    • entityDashboardContent
    • entityDashboardSpeed
    • entityDashboardMonetization
  • Each of the new widget contexts should also have a single Widget Area that will render all of their widgets. There should be a widget area for each of the new widget context with the context name and a Primary suffix. So there should be the following widget areas:
    • mainDashboardTrafficPrimary
    • mainDashboardContentPrimary
    • mainDashboardSpeedPrimary
    • mainDashboardMonetizationPrimary
    • entityDashboardTrafficPrimary
    • entityDashboardContentPrimary
    • entityDashboardSpeedPrimary
    • entityDashboardMonetizationPrimary
  • There should be a WidgetContextRenderer for each Widget Context listed on the respective DashboardMainApp and DashboardEntityApp components.

Implementation Brief

Define the view contexts

Inside assets/js/googlesitekit/widgets/default-contexts.js, add the following constants:

  • These will be for the DashboardMainApp:
    • CONTEXT_MAIN_DASHBOARD_TRAFFIC = 'mainDashboardTraffic';
    • CONTEXT_MAIN_DASHBOARD_CONTENT = 'mainDashboardContent';
    • CONTEXT_MAIN_DASHBOARD_SPEED = 'mainDashboardSpeed';
    • CONTEXT_MAIN_DASHBOARD_MONETIZATION = 'mainDashboardMonetization';
  • And the following are for the entity dashboard:
    • CONTEXT_ENTITY_DASHBOARD_TRAFFIC = 'entityDashboardTraffic';
    • CONTEXT_ENTITY_DASHBOARD_CONTENT = 'entityDashboardContent';
    • CONTEXT_ENTITY_DASHBOARD_SPEED = 'entityDashboardSpeed';
    • CONTEXT_ENTITY_DASHBOARD_MONETIZATION = 'entityDashboardMonetization';
  • You will need to optionally export them along with the existing contexts based on whether the unifiedDashboard feature flag is enabled. For example, something like:
    export default {
      // Current contexts exports go here
      CONTEXT_DASHBOARD,
      CONTEXT_PAGE_DASHBOARD,
      CONTEXT_MODULE_SEARCH_CONSOLE,
      CONTEXT_MODULE_ANALYTICS,
      CONTEXT_MODULE_ADSENSE,
      ...( isFeatureEnabled( 'unifiedDashboard' ) ? {
        // Add the new contexts here
      } : {} ),
    };

Define the widget areas

Inside assets/js/googlesitekit/widgets/default-areas.js, add the following constants:

  • For the DashboardMainApp:
    • AREA_MAIN_DASHBOARD_TRAFFIC_PRIMARY = 'mainDashboardTrafficPrimary';
    • AREA_MAIN_DASHBOARD_CONTENT_PRIMARY = 'mainDashboardContentPrimary';
    • AREA_MAIN_DASHBOARD_SPEED_PRIMARY = 'mainDashboardSpeedPrimary';
    • AREA_MAIN_DASHBOARD_MONETIZATION_PRIMARY = 'mainDashboardMonetizationPrimary';
  • For the DashboardEntityApp:
    • AREA_ENTITY_DASHBOARD_TRAFFIC_PRIMARY = 'entityDashboardTrafficPrimary';
    • AREA_ENTITY_DASHBOARD_CONTENT_PRIMARY = 'entityDashboardContentPrimary';
    • AREA_ENTITY_DASHBOARD_SPEED_PRIMARY = 'entityDashboardSpeedPrimary';
    • AREA_ENTITY_DASHBOARD_MONETIZATION_PRIMARY = 'entityDashboardMonetizationPrimary';
  • Similarly to the view contexts above, you will need to optionally export these depending on whether the unifiedDashboard feature is enabled.

Register the widget areas

Inside assets/js/googlesitekit/widgets/register-defaults.js

  • Check if the unifiedDashboard feature flag is enabled using isFeatureEnabled.
  • If the feature IS enabled, register the following widget areas with the following props:
    • AREA_MAIN_DASHBOARD_TRAFFIC_PRIMARY
      • title: Find out how your audience is growing
      • subtitle: Track your site's traffic over time
      • style: WIDGET_AREA_STYLES.BOXES
      • priority: 1,
      • CONTEXT_MAIN_DASHBOARD_TRAFFIC
    • AREA_MAIN_DASHBOARD_CONTENT_PRIMARY
      • title: See how your content is doing
      • subtitle: Keep track of your most popular pages and how people found them from Search
      • style: WIDGET_AREA_STYLES.COMPOSITE
      • priority: 1,
      • CONTEXT_MAIN_DASHBOARD_CONTENT
    • AREA_MAIN_DASHBOARD_SPEED_PRIMARY
      • title: Find out how visitors experience your site
      • subtitle: Keep track of how fast your pages are and get specific recommendations on what to improve
      • style: WIDGET_AREA_STYLES.BOXES
      • priority: 1,
      • CONTEXT_MAIN_DASHBOARD_SPEED
    • AREA_MAIN_DASHBOARD_MONETIZATION_PRIMARY
      • title: Find out how much you’re earning from your content
      • subtitle: Track your revenue over time
      • style: WIDGET_AREA_STYLES.BOXES
      • priority: 1
      • CONTEXT_MAIN_DASHBOARD_MONETIZATION
  • For now, the entity dashboard can be the same as the main dashboard. So repeat the last step for the respective entity widget areas, i.e.
    • AREA_ENTITY_DASHBOARD_TRAFFIC_PRIMARY
      • title: Find out how your audience is growing
      • subtitle: Track your site's traffic over time
      • style: WIDGET_AREA_STYLES.BOXES
      • priority: 1,
    • CONTEXT_ENTITY_DASHBOARD_TRAFFIC
    • etc.

Render the view contexts

Inside assets/js/components/DashboardMainApp:

  • Import the CONTEXT_MAIN_* contexts we just created from assets/js/googlesitekit/widgets/default-contexts.js.
  • For each context render a WidgetContextRenderer
    • the slug prop will be the context
    • the Header prop will be the Header component (assets/js/components/Header.js) for now
  • Note that the order is important, and should be as follows:
    1. CONTEXT_MAIN_DASHBOARD_TRAFFIC
    2. CONTEXT_MAIN_DASHBOARD_CONTENT
    3. CONTEXT_MAIN_DASHBOARD_SPEED
    4. CONTEXT_MAIN_DASHBOARD_MONETIZATION

Inside assets/js/components/DashboardEntityApp, repeat the process using the CONTEXT_ENTITY_* contexts.

  • Again, the order is important, and should be:
    1. CONTEXT_ENTITY_DASHBOARD_TRAFFIC
    2. CONTEXT_ENTITY_DASHBOARD_CONTENT
    3. CONTEXT_ENTITY_DASHBOARD_SPEED
    4. CONTEXT_ENTITY_DASHBOARD_MONETIZATION

Test Coverage

  • No new tests required.

Visual Regression Changes

  • This shouldn't require any VRT changes.

QA Brief

  • Verify that the correct widget context and area constants have been added and registered and are inside DashboardMainApp and DashboardEntityApp in the correct order.

Changelog entry

  • N/A
@tofumatt tofumatt added P0 High priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling labels Sep 13, 2021
@felixarntz
Copy link
Member

@tofumatt It may be good to clarify in the last bullet point of the ACs how these renders should be organized? Basically in the order above, 4 on the main dashboard, 4 on the entity dashboard, each one below each other.

@felixarntz felixarntz assigned tofumatt and unassigned felixarntz Sep 14, 2021
@tofumatt tofumatt removed their assignment Sep 14, 2021
@tofumatt
Copy link
Collaborator Author

You referenced useFeature in the code examples in the IB, but that's a React hook that's only available inside a function component. You'll want to use isFeatureEnabled instead.

I like the approach of conditional extending the object you're exporting here, but maybe it makes sense to leave the exports as-is rather than creating the defaultContexts variable, instead doing something like:

export default {
	CONTEXT_DASHBOARD,
	CONTEXT_PAGE_DASHBOARD,
	CONTEXT_MODULE_SEARCH_CONSOLE,
	CONTEXT_MODULE_ANALYTICS,
	CONTEXT_MODULE_ADSENSE,
	( isFeatureEnabled( 'unifiedDashboard' ) ? { /* OTHER, NEW CONSTANTS */ } : {}  ),
};

I think on a call I suggested the variable approach but I suppose it's not needed.

@tofumatt tofumatt assigned johnPhillips and unassigned tofumatt Sep 16, 2021
@johnPhillips
Copy link
Contributor

Thanks @tofumatt, I've edited the IB. Back to you for another pass 👍

@tofumatt
Copy link
Collaborator Author

IB ✅

@johnPhillips
Copy link
Contributor

@tofumatt I just noticed that when I did the IB I missed out registering the entity widget areas. Looking at the designs, the titles for the widgets are going to need to interpolate things like the post title and URL?

Screenshot 2021-09-24 at 12 06 52

Possibly very straightforward but I'd be keen to get your thoughts when you have a chance.

@johnPhillips
Copy link
Contributor

After a conversation with @tofumatt, we decided that the entity dashboard widget areas can be identical for now (same widths, titles etc). IB updated.

@johnPhillips
Copy link
Contributor

@tofumatt Thinking about this more carefully, I'm not sure this is the right approach:

You will need to optionally export them along with the existing contexts based on whether the unifiedDashboard feature flag is enabled

If we optionally export these constants then of course all the consumers will have to optionally import them, for example when registering widgets:

widgets.registerWidget(
  'adsenseTopEarningPages',
  {
    Component: DashboardTopEarningPagesWidget,
    width: [ widgets.WIDGET_WIDTHS.HALF, widgets.WIDGET_WIDTHS.FULL ],
    priority: 2,
    wrapWidget: false,
  },
  [ 
    AREA_DASHBOARD_EARNINGS, 
    ...( isFeatureEnabled ? [ AREA_MAIN_DASHBOARD_MONETIZATION_PRIMARY ] : [] )
   ]
);

It seems a lot of logic all over the place for the sake of some strings... what do you think?

@tofumatt
Copy link
Collaborator Author

I don't think that's correct, because what's exported is an object with a bunch of attributes. In this case the attributes won't be present on the object, but that seems fine—if anything you could check to see if said attribute was undefined or not and even conditionally register the widget to the area based on that—though I think using isFeatureEnabled is nicer.

Because we can import the default export (an object) and use something like WIDGET_AREAS.AREA_MAIN_DASHBOARD_MONETIZATION_PRIMARY. Does that make sense?

@johnPhillips
Copy link
Contributor

Just a note that we discussed the above on a call and it was decided that conditionally exporting the widget constants was unnecessary. I'll strike-through the relevant steps in the IB.

@johnPhillips johnPhillips removed their assignment Sep 29, 2021
@asvinb asvinb self-assigned this Sep 29, 2021
@asvinb
Copy link
Collaborator

asvinb commented Sep 29, 2021

QA: ✅

  • Widget context and area constants have been added and registered and are inside DashboardMainApp and DashboardEntityApp in the correct order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

5 participants