Navigation Menu

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 the plugin-settings directive #18432

Merged
merged 537 commits into from Dec 21, 2021
Merged

[Vue] migrate the plugin-settings directive #18432

merged 537 commits into from Dec 21, 2021

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Dec 2, 2021

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

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.

Generally seems to work. Found a couple of smaller issues that needs to be fixed.

@peterhashair
Copy link
Contributor

peterhashair commented Dec 16, 2021

@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
Copy link
Member

sgiehl commented Dec 17, 2021

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

@sgiehl sgiehl removed the Needs Review PRs that need a code review label Dec 17, 2021
@diosmosis
Copy link
Member Author

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

@sgiehl
Copy link
Member

sgiehl commented Dec 17, 2021

@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
Copy link
Member Author

diosmosis commented Dec 17, 2021

@sgiehl 👍 sure, go ahead!

Peter Zhang and others added 4 commits December 19, 2021 19:56
…nagers enabled (#18515)

* Update PluginSettings.vue

* built vue files

Co-authored-by: peterhashair <peterhashair@users.noreply.github.com>
@sgiehl sgiehl merged commit 91220ba into 4.x-dev Dec 21, 2021
@sgiehl sgiehl deleted the vue-plugin-settings branch December 21, 2021 10:51
@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
Labels
Do not close PRs with this label won't be marked as stale by the Close Stale Issues action not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants