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

Pass scoped Widget component to registered widget component function #2613

Closed
felixarntz opened this issue Jan 13, 2021 · 3 comments
Closed
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Milestone

Comments

@felixarntz
Copy link
Member

felixarntz commented Jan 13, 2021

Widgets that pass wrapWidget: false need to render the wrapping Widget component themselves. For this purpose, we right now export the Widget component publicly on googlesitekit.widgets.components.

Somewhat annoying here, components that render Widget on its own currently are required to pass the widget's slug to the component - duplicating that from where the widget is registered, making it error-prone.

We should change this so that we pass a scoped version of Widget to every registered component so that they don't need to import it and don't need to pass the slug prop. This follows the pattern established in #2252 where components already get access to special widget state components via props, such as WidgetReportZero.


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

Acceptance criteria

  • The slug prop of Widget should be renamed to a more explicit widgetSlug, matching what other publicly usable Widget* components are doing.
  • The getWidgetComponentProps function should be expanded to include a scoped version of Widget in the props object. (This will have WidgetRenderer pass it through to every registered widget component.)
  • The googlesitekit.widgets.components property should be removed (i.e. Widgets.components internally).
  • Every Site Kit widget that currently renders <Widget /> manually in its component should rely on the Widget it receives from props instead of importing it. The slug / widgetSlug must no longer be passed since it's already taken care of.

Implementation Brief

  • Rename the slug property to be widgetSlug in the Widget component. Update all occurrences of the Widget component to use the new property.
  • Add a new property to the returning object from the getWidgetComponentProps function. It should have the Widget name and scoped the Widget component as a value (similarly to other props in the returned object).
  • Edit the following components to rely on the Widget property passed to the component instead of Widgets.components.Widget one. Plus remove slug properties from the rendered <Widget> components since they are not needed anymore.
    • The DashboardSummaryWidget component in the AdSense module;
    • The DashboardTopEarningPagesWidget component in the AdSense module;
    • The DashboardAllTrafficWidget component in the Analytics module;
    • The DashboardPopularPagesWidget component in the Analytics module;
    • The DashboardPageSpeedWidget component in the Page Speed module;
    • The DashboardPopularKeywordsWidget component in the Search Console module;
  • Remove the components property of the Widgets object in the assets/js/googlesitekit/widgets/index.js file.
  • Update storybook stories to account for the aforementioned changes.

Test Coverage

  • Adjust existing tests if any starts failing due to the requested changes.

Visual Regression Changes

  • No visual changes are expected.

QA Brief

  • Review each of the following widgets and match the data returned with the previous version:
    • The DashboardSummaryWidget component in the AdSense module;
    • The DashboardTopEarningPagesWidget component in the AdSense module;
    • The DashboardAllTrafficWidget component in the Analytics module;
    • The DashboardPopularPagesWidget component in the Analytics module;
    • The DashboardPageSpeedWidget component in the Page Speed module;
    • The DashboardPopularKeywordsWidget component in the Search Console module;

Changelog entry

  • Simplify usage of the Widget component so that widget components can use a scoped version of it via props rather than manually importing it and re-specifying the widget slug.
@felixarntz felixarntz added P0 High priority Type: Enhancement Improvement of an existing feature Next Up labels Jan 13, 2021
@felixarntz felixarntz added this to the Sprint 41 milestone Jan 13, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Jan 13, 2021
@eugene-manuilov eugene-manuilov self-assigned this Jan 13, 2021
@eugene-manuilov eugene-manuilov added the QA: Eng Requires specialized QA by an engineer label Jan 13, 2021
@eugene-manuilov eugene-manuilov removed their assignment Jan 13, 2021
@aaemnnosttv aaemnnosttv self-assigned this Jan 15, 2021
@aaemnnosttv
Copy link
Collaborator

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Jan 15, 2021
@eclarke1 eclarke1 removed the Next Up label Jan 18, 2021
@benbowler benbowler self-assigned this Jan 18, 2021
@benbowler
Copy link
Collaborator

Executed, I'll wait on the merge of #2252 to finalise tests and storybooks so I don't have a moving target.

@benbowler benbowler removed their assignment Jan 30, 2021
@eugene-manuilov eugene-manuilov removed their assignment Feb 1, 2021
@fhollis fhollis added the Rollover Issues which role over to the next sprint label Feb 1, 2021
@fhollis fhollis modified the milestones: Sprint 41, Sprint 42 Feb 1, 2021
@felixarntz felixarntz assigned felixarntz and benbowler and unassigned felixarntz Feb 2, 2021
@benbowler benbowler assigned felixarntz and unassigned benbowler Feb 3, 2021
@felixarntz felixarntz removed their assignment Feb 4, 2021
@tofumatt tofumatt self-assigned this Feb 5, 2021
@tofumatt
Copy link
Collaborator

tofumatt commented Feb 5, 2021

QA ✅

@tofumatt tofumatt removed their assignment Feb 5, 2021
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 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

7 participants