[Vue] remove use of angularjs #502
Conversation
…omponents, removing adapters and not using angularjs services
|
@diosmosis Seems this one needs an update. There are a couple of tests failing. Guess something has been added on 4.x-dev, that is no longer available on this branch or similar... |
|
@sgiehl fixed the broken merge |
There was a problem hiding this comment.
Had a look through the code. Besides the comments I left it looks good for me.
Also did a rough testing in the UI. But due to my limited time I'm not able to test every detail.
Maybe @AltamashShaikh has some time to do a more excessive testing.
Or we otherwise merge this one, once my comments are solved, and create bug reports later if anything pops up.
javascripts/tagmanagerHelper.js
Outdated
| var $scope = piwikHelper.getAngularDependency('$rootScope'); | ||
| var piwikUrl = piwikHelper.getAngularDependency('piwikUrl'); | ||
| var containerId = piwikUrl.getSearchParam('idContainer'); | ||
| var template = $('<div class="ui-confirm"><h2>Select a variable</h2><div></div><input role="no" type="button" value="' + _pk_translate('General_Cancel') +'"/></div>') |
There was a problem hiding this comment.
Might be good to make the Select a variable translatable.
javascripts/tagmanagerHelper.js
Outdated
| var createVueApp = CoreHome.createVueApp; | ||
| var VariableSelect = TagManager.VariableSelect; | ||
| var MatomoUrl = CoreHome.MatomoUrl; | ||
| var containerId = MatomoUrl.parsed.value.containerId; |
There was a problem hiding this comment.
| var containerId = MatomoUrl.parsed.value.containerId; | |
| var containerId = MatomoUrl.parsed.value.idContainer; |
| this.editVersion(null, containerId, 0, function () { window.location.reload(); }); | ||
| }; | ||
| tagManagerHelper.editVersion = function ($scope, idContainer, idContainerVersion, callback) { | ||
| tagManagerHelper.editVersion = function (idContainer, idContainerVersion, callback) { |
There was a problem hiding this comment.
on line 84, there is another method, where the first parameter needs to be removed.
Otherwise the create new variable link on the edit tag page throws an error.
|
@sgiehl applied the fixes. I tried doing some more testing and fixed one potential issue, but couldn't find anything else. |
There was a problem hiding this comment.
Ok. As tests are still passing let's merge this one. @AltamashShaikh if vue migration related issue pops up in TagManager when working on the 5.x release, feel free to create new issues and ping @diosmosis so she can provide a fix for it.
|
Hi @sgiehl, please use the correct pronouns when referring to me. |
|
Oh sorry. Sure. I hadn't really proofread what I wrote 🙈 |
Description:
Changes:
Review