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 UI for activating Analytics as part of plugin setup to the activation banner #4343

Closed
felixarntz opened this issue Nov 10, 2021 · 3 comments
Labels
P1 Medium priority Type: Enhancement Improvement of an existing feature

Comments

@felixarntz
Copy link
Member

felixarntz commented Nov 10, 2021

The UI to activate Analytics as part of the setup is being implemented in #4341, adding it to the splash screen. This issue is a follow-up to that one with the goal to incorporate the same UI into the plugin activation banner.


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

Acceptance criteria

  • The assets/js/components/setup/ActivateAnalyticsNotice.js component from Implement splash UI for initially activating Analytics #4341 should be incorporated into the plugin activation banner (ActivationApp / ActivationMain components) as follows:
    • It should only show if the serviceSetupV2 feature is enabled.
    • It should only show if the proxy is being used.
    • It should only show if the plugin is not connected yet (i.e. ! canViewDashboard in this particular UI).
    • It should only show if the analytics module is not active.
    • When shown, it should be displayed below the header/description but above the tracking checkbox.
    • When the user clicks the CTA button to sign in, if the above checkbox is enabled (see core/forms store), the following tweaks should be made (exactly in accordance to the same behavior described for the CTA button in Implement splash UI for initially activating Analytics #4341):
      • A new GA tracking event start_setup_with_analytics should be fired in addition to the existing GA events on that button click. The category should be the current view context (i.e. VIEW_CONTEXT_ACTIVATION).
      • Before forwarding to the proxy setup URL, a request to activate the analytics module should be made. (This will ensure that the Analytics OAuth scopes will be immediately requested as part of the setup.)
      • The existing request to the proxy setup URL should have an additional query parameter redirect which should be the URL to the Analytics setup UI.
      • See Implement splash UI for initially activating Analytics #4341 for the implementation of these parts - it's the same, so the code there can probably be reused here.

Implementation Brief

  • Once Implement splash UI for initially activating Analytics #4341 has been merged.
  • Using assets/js/components/activation/activation-main.js,
    • Render ActivateAnalyticsNotice below the header/description but above the tracking checkbox if:
      • serviceSetupV2 feature flag is enabled.
        • Use the useFeature hook with serviceSetupV2 as parameter to see if the feature is enabled.
      • the proxy is being used.
        • Use the isUsingProxy selector of core/site data store to see if the proxy is being used.
      • the plugin is not connected yet.
        • Use the hasCapability selector of core/user data store to see if the plugin is connected, with PERMISSION_VIEW_DASHBOARD as parameter.
      • analytics module is not active.
        • Use the isModuleActive selector of core/modules data store with analytics as parameter to see if the module is active.
  • Using assets/js/components/activation/activation-app.js,
    • Check if the checkbox from ActivateAnalyticsNotice is checked by querying the core/forms data store via the getValue selector.
    • Update the onButtonClick function:
      • If the checkbox is checked,
        • Call the trackEvent function with the parameters as per the AC.
        • dispatch the activateModule action from the core/modules store, with analytics as parameter and grab the moduleReauthURL property from the response object as a result of the activateModule action.
        • Update the proxySetupURL to append the redirect parameter, with the value set to moduleReauthURL obtained previously.
  • Add stories for ActivationMain component in assets/js/components/activation/activation-main.stories.js

Test Coverage

  • No new tests to be added.

QA Brief

Changelog entry

@felixarntz felixarntz added P1 Medium priority Type: Enhancement Improvement of an existing feature labels Nov 10, 2021
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Nov 10, 2021
@asvinb asvinb assigned asvinb and unassigned asvinb Nov 16, 2021
@eugene-manuilov eugene-manuilov self-assigned this Nov 16, 2021
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@eugene-manuilov eugene-manuilov removed their assignment Nov 16, 2021
@asvinb asvinb self-assigned this Nov 23, 2021
@asvinb
Copy link
Collaborator

asvinb commented Nov 24, 2021

@felixarntz The ActivateAnalyticsNotice component looks rather big compared to other elements on the page:
image

I'am thinking whether we can tighten things a little bit, by reducing the padding, font size and the width of the SVG which will result in the following:
image

Happy to hear your views. cc @marrrmarrr

@felixarntz
Copy link
Member Author

@asvinb As mentioned in our sync call, it's true that this looks strange, but we've decided to remove this "replication of the splash screen UI" approach from the plugin activation banner. I've opened #4403 to address this separately.

I guess we can close this issue (and the PR #4400) then - sorry about that, but in the long run the simplification here probably makes sense from both a UX and engineering perspective, so that we no longer need to double-maintain the splash screen UI.

cc @eclarke1 @FlicHollis Per the above this can be removed from the current sprint (although most work on it has already been done, so we shouldn't add anything else to the sprint instead).

@felixarntz felixarntz removed their assignment Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants