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

[Vue] migrate debugging angularjs controller to vue component #432

Merged
merged 8 commits into from Mar 11, 2022

Conversation

diosmosis
Copy link
Member

Description:

Changes:

  • migrate debugging angularjs controller to vue component

Review

@diosmosis diosmosis added this to the Current sprint milestone Feb 16, 2022
@diosmosis diosmosis marked this pull request as draft February 16, 2022 20:16
@diosmosis diosmosis marked this pull request as ready for review March 9, 2022 19:18
@diosmosis
Copy link
Member Author

Note: UI test failure looks random.

Also, note that this will break existing previews since the HTML appears to be saved on the filesystem. Disabling and enabling the preview fixes it. If needed we can perhaps disable all previews in an update in core.

Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@diosmosis
We should update it in core, since users might complain that after a recent upgrade Matomo Tag Manager preview is not working.
Also, we can regenerate all the containers or make use of this method generateContainerIfHasPreviewRelease to regenerate only when preview enabled.

@diosmosis
Copy link
Member Author

@AltamashShaikh created a core PR here: matomo-org/matomo#18921

Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@diosmosis I just saw this
https://github.com/matomo-org/tag-manager/blob/4.x-dev/TagManager.php#L55

On update we are triggering regenerateReleasedContainers so we don't need the change in core, we can keep the plugin changes but no need to make changes in core

@diosmosis
Copy link
Member Author

diosmosis commented Mar 11, 2022

@AltamashShaikh oh I see, that's interesting, I'll close the core PR, nice catch, thanks!

@diosmosis diosmosis merged commit 7f4921e into 4.x-dev Mar 11, 2022
@diosmosis diosmosis deleted the vue-debugging branch March 11, 2022 17:01
@innocraft-automation innocraft-automation removed this from the Current sprint milestone Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants