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

Jump to bottom

[Vue] migrate tracking code generation controllers to Vue #18552

Merged
merged 640 commits into from Jan 13, 2022
Merged

[Vue] migrate tracking code generation controllers to Vue #18552

merged 640 commits into from Jan 13, 2022

Conversation

diosmosis
Copy link
Member

Description:

This PR is based off of #18549.

Changes:

  • Migrate the js tracking code generator controller and twig content.
  • Migrate the image tracking code generator controller and twig content.
  • Add some extra exports to CoreHome.

Review

@diosmosis
Merge branch 'vue-reporting-pages-model' into vue-reporting-menu
c2fb339
@diosmosis
Merge branch 'vue-reporting-menu' into vue-widget-loader
5101693
@diosmosis
emit/broadcast on correct scope in wrapper
cb0441a
@diosmosis
rebuild vue
2991433
@diosmosis
Merge branch '4.x-dev' into vue-period-selector
6d33299
@diosmosis
Merge branch 'vue-period-selector' into vue-reporting-pages-model
4833261
@diosmosis
Merge branch 'vue-reporting-pages-model' into vue-reporting-menu
2ec6bf7
@diosmosis
Merge branch 'vue-reporting-menu' into vue-widget-loader
00aac86
@diosmosis
Merge branch 'next_release' into vue-widget-loader
1e0ecfa
@diosmosis
Merge branch 'vue-post-event-regression' into vue-widget-loader
2519f24
@diosmosis
fix some issues
5546f95
@diosmosis
Merge branch 'vue-widget-loader' into vue-widget
6497ead
@diosmosis
couple more fixes
6e56c3c
@diosmosis
fix another title issue
cc0b4fb
@diosmosis
Merge branch 'vue-widget-loader' into vue-widget
17de94c
@diosmosis
rebuild vue
9168633
@diosmosis
do not report on ajax errors in notifications if not logged in
9d732a8
@diosmosis
migrate reporting page and model
4539552
@diosmosis
rebuild vue
8587588
@diosmosis
create sites selector model adapter
8fd921f
@diosmosis
fix siteselector vue bug, initial site is only set if there is just o…
a77e739 
…ne site available
@diosmosis
rebuild vue
01d723c
@diosmosis
migrate plugin settings directive
de7b07f
@diosmosis
remove TODO
95d3df8
@diosmosis
migrate plugin filter directive
51a301c
@diosmosis
migrate two more plugins directives
7504461
@diosmosis
migrate save button
69e59be
@diosmosis
fix a bunch of bugs
913ec0a
@diosmosis
Merge branch 'vue-widget' into vue-core-plugins-admin
f699c79
@diosmosis
fix another widget bug
070ba66
@diosmosis diosmosis force-pushed the vue-coreadminhome-tracking-generator branch from ae8ba39 to c807090 Compare January 12, 2022 00:32
@diosmosis diosmosis marked this pull request as ready for review January 12, 2022 03:05
@diosmosis diosmosis added the Needs Review PRs that need a code review label Jan 12, 2022
@sgiehl
Copy link
Member

sgiehl commented Jan 12, 2022

While clicking through the UI I found one minor issue. Not sure if that was the case before, but I guess it should be easy to fix:

When you choose a goal for image tracking and afterwards switch the site in the selector to a site where a goal with that id doesn't exist, the goal selector switches to None, but the tracking code still includes idGoal=x for the previously selected goal.

sgiehl
sgiehl reviewed Jan 12, 2022
View reviewed changes
plugins/CoreAdminHome/vue/src/JsTrackingCodeGenerator/JsTrackingCodeGenerator.vue Outdated
{{ translate('CoreAdminHome_JSTrackingIntro1') }}
<br/><br/>
{{ translate('CoreAdminHome_JSTrackingIntro2') }}
<span v-html="jsTrackingIntro3a"></span> {{ translate('CoreAdminHome_JSTrackingIntro3b') }}
Copy link
Member

Choose a reason for hiding this comment

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

I guess the translated text of CoreAdminHome_JSTrackingIntro3b is not auto escaped here, causing the escaped </head> to be double escaped. This is also visible in the UI screenshot failure.

@diosmosis
Copy link
Member Author

When you choose a goal for image tracking and afterwards switch the site in the selector to a site where a goal with that id doesn't exist, the goal selector switches to None, but the tracking code still includes idGoal=x for the previously selected goal.

Should be fixed.

@diosmosis
Copy link
Member Author

@sgiehl should be fixed

sgiehl
sgiehl approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

works now 👍

@sgiehl sgiehl merged commit fc61b60 into 4.x-dev Jan 13, 2022
@sgiehl sgiehl deleted the vue-coreadminhome-tracking-generator branch January 13, 2022 19:48
@justinvelluppillai justinvelluppillai added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.7.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@diosmosis @sgiehl @justinvelluppillai @bx80 @weblate