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 note about google analytics importer to site without data card. #14728

Merged
merged 3 commits into from Oct 2, 2019

Conversation

diosmosis
Copy link
Member

The note is added to core since the importer will be released on the marketplace, so we can't add it through an event.

This cannot be merged until the importer is released on the marketplace.

@diosmosis diosmosis added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Aug 2, 2019
@diosmosis diosmosis added this to the 3.12.0 milestone Aug 2, 2019
@@ -77,6 +77,9 @@

<h3>{{ 'CoreAdminHome_HttpTrackingApi'|translate }}</h3>
<p>{{ 'CoreAdminHome_HttpTrackingApiDescription'|translate('<a href="https://developer.matomo.org/api-reference/tracking-api" rel="noreferrer noopener" target="_blank">','</a>')|raw }}</p>

<h3>{{ 'CoreAdminHome_ImportFromGoogleAnalytics'|translate }}</h3>
<p>{{ 'CoreAdminHome_ImportFromGoogleAnalyticsDescription'|translate('<a href="https://plugins.matomo.org/GoogleAnalyticsImporter" rel="noopener noreferrer" target="_blank">','</a>')|raw }}</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On cloud we would rather want a message re getting in touch with support or so? or maybe even hide it...
And maybe if plugin is activated the message should be also different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsteur

For changing the message if it is activated, I think we can just check if isPluginActivated(...) in the Controller.

For hiding the element, I can either:

  • add an event like CoreAdminHome.disableGoogleAnalyticsImporterMessage or something
  • or just use a div w/ an ID so it can be hidden through css

How does all of that sound to you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either is fine. Can use the CSS I suppose 👍

@mattab might also need to think re whitelabel? Should it be removed then as well? I reckon eg if "RemoveLinks" setting is disabled then this would need to be removed? Maybe also in general? (not sure)... in this case it would need to be an event though @diosmosis so we can remove it on demand.

Or we move the addition of this into the ProfessionalServices plugin (and then showing it using the template event). It's not a commercial notice the GA importer as it is free but could be still moved in there? Not sure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event seems like it would be more flexible, I'll just use that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Can you maybe let me know once this is done so I can adjust white label and cloud?

@diosmosis
Copy link
Member Author

@tsteur / @mattab modified the PR to use an event

@diosmosis diosmosis added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Oct 2, 2019
@tsteur
Copy link
Member

tsteur commented Oct 2, 2019

LGTM if tests pass @diosmosis

@diosmosis diosmosis merged commit c6782cf into 3.x-dev Oct 2, 2019
@diosmosis diosmosis deleted the ga-getting-started-note branch October 2, 2019 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants