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

Reduce code complexity in GoogleChart #6029

Closed
nfmohit opened this issue Oct 20, 2022 · 2 comments
Closed

Reduce code complexity in GoogleChart #6029

nfmohit opened this issue Oct 20, 2022 · 2 comments
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Oct 20, 2022

Feature Description

This is a follow-up of #4895, which introduces the complexity rule in ESLint. This resulted in the following lint error:

.../wordpress/wp-content/plugins/google-site-kit/assets/js/components/GoogleChart.js
  53:16  error  Function 'GoogleChart' has a complexity of 22. Maximum allowed is 20  complexity

This is the component in question.

While the error is currently ignored, the aim of this issue is to reduce said complexity.


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

Acceptance criteria

  • The ESLint ignore for complexity should be removed from the component identified in the title
  • The component should be refactored to reduce the complexity to be within the acceptable range set by the current rule

Implementation Brief

  • Using google-site-kit/assets/js/components/GoogleChart.js,
    • Remove ESLint ignore for complexity
    • Extract the logic to set modifiedData, loadingWidthToUse, loadingHeightToUse, combinedChartEvents, and chartOptions into individual functions
    • Use newly created functions to deliver props to <Chart />

Test Coverage

  • No new tests are needed

QA Brief

  • Check Google Charts appearance across this site - all functionality should remain the same

Changelog entry

  • Reduce code complexity in GoogleChart component.
@aaemnnosttv aaemnnosttv added P2 Low priority Type: Enhancement Improvement of an existing feature labels Dec 16, 2022
@sashadoes sashadoes assigned sashadoes and unassigned sashadoes Dec 20, 2022
@aaemnnosttv aaemnnosttv self-assigned this Jan 5, 2023
@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Jan 5, 2023

@nfmohit it should come up later if this didn't happen, but it's probably a good idea to mention removing the related ESLint ignore rule here as well :) (I've updated it for completeness)

IB ✅

@aaemnnosttv aaemnnosttv removed their assignment Jan 5, 2023
@sashadoes sashadoes self-assigned this Jan 14, 2023
@sashadoes sashadoes removed their assignment Jan 19, 2023
@techanvil techanvil assigned techanvil and sashadoes and unassigned techanvil Jan 20, 2023
@sashadoes sashadoes assigned techanvil and unassigned sashadoes Jan 20, 2023
@techanvil techanvil assigned sashadoes and unassigned techanvil Jan 23, 2023
@sashadoes sashadoes assigned techanvil and unassigned sashadoes Jan 23, 2023
@techanvil techanvil removed their assignment Jan 24, 2023
@wpdarren wpdarren self-assigned this Jan 24, 2023
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Checked Google Charts appearance across this site to check that all functionality remains the same
    • Tested main and entity dashboard with live data (oi.ie)
    • Tested main and entity dashboard with gathering and zero data states
    • Tested main and entity view only dashboard
    • Tested the Site Kit widget on WP dashboard
    • Confirmed that there are no new warnings or errors in the console

@wpdarren wpdarren removed their assignment Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Low priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants