@diosmosis opened this Pull Request on December 2nd 2021 Member

Description:

This PR is based off of #18431.

Changes:

  • Migrate the plugin-settings directive.
  • Allow FormField.vue to execute conditions without supplying all-settings param. Doesn't make sense in a Vue context since the parent component can keep track of the list of all values. Still there, but only for BC.
  • Add AjaxHelper.post() method.

IMPORTANT NOTE: This PR is still using angularjs for expression evaluation. There doesn't seem to be an established library to do something similar, though a handful exist (none of them seem very popular). We'll either have to use one of those or roll our own. I'm not sure which is the best option, but suggestions are welcome.

image

Review

@diosmosis commented on December 6th 2021 Member

UI tests passing for this one (some screenshots in plugins need an update, but holding off on that until after the specific PRs are merged)

@github-actions[bot] commented on December 14th 2021 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@peterhashair commented on December 14th 2021 Contributor

@diosmosis hasn't finished reviewing the VUE code on that one, but did resolve conflicts for that one, can you review the merge conflicts, in case I merge the wrong parts.

@peterhashair commented on December 16th 2021 Contributor

@diosmosis did a fix for one of the issues in #18453 for this version here. When we type a wrong password in the modal, then close it, passwordConfirmation still has the value. Separate the modal-show and save it into 2 different functions.
Other than that, it's working as expected.

@sgiehl commented on December 17th 2021 Member

@diosmosis would be good to review #18515 and merge it before pushing any further changes, to avoid merge conflicts.

@diosmosis commented on December 17th 2021 Member

@sgiehl I will take a look as soon as I can but I will be pretty busy with other things for the moment.

@sgiehl commented on December 17th 2021 Member

@diosmosis no problem. Btw. I found another regression in TagManager (top controls aren't visible anymore). Shall I create a issue for that and assign it to you?

@diosmosis commented on December 17th 2021 Member

@sgiehl :+1: sure, go ahead!

This Pull Request was closed on December 21st 2021
Powered by GitHub Issue Mirror