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 piwik-period-selector angularjs component #18382
Conversation
…te if no translations are loaded
…larjs comment no longer being there)
…on groups to work
ea49462
to
8fb53a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment. Otherwise the code is looking fine.
While clicking through the UI I found this regressions:
This happens on 4.5.0 too, it's a materialize issue.
Fixed + another related bug (will now only change the date if it is a full YYYY-MM-DD date).
Undid those changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment. Besides that the CustomAlerts submodule was changed in this PR as well. This should be reverted before merging. When that is changed and tests are passing, feel free to merge this one.
Co-authored-by: Stefan Giehl <stefan@matomo.org>
There are a couple unrelated failures, one is fixed in another branch, but I'll create a new branch for it just so it's there. I'll look into the other one. |
Hello there, does this migration Vue makes possible to override/modify the period selector calendar? |
@tassoman how specifically do you override/modify it? I can't see any angularjs code in the plugin. |
Hi @tassoman, I took another look and if you mean the HTML element removal your plugin does then it won't work with Vue anymore. Vue uses a virtual DOM and keeps HTML in sync with it, so if you remove an element, it will just be put back again. I would suggest a different approach:
|
Thanks for inspecting my code. I was stuck after the adoption of Angular. |
Description:
This PR is based off of #18352.
Changes:
expander
that reference Vue $refs. Without something like this, the directive will be initiated before the rest of the component is mounted, so refs for child elements will be undefined.Review