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

Add support for up to 4 fields to multipairfield #15125

Conversation

MichaelRoosz
Copy link
Contributor

@MichaelRoosz MichaelRoosz commented Nov 6, 2019

Adds support for up to 4 fields in the multipairfield config template.
Needed for another pull requests for Tag Manager (support for Custom Variables matomo-org/tag-manager#201).

Maybe the changes in "plugins/CorePluginsAdmin/angularjs/form-field/field-multituple.html" are not required? I am not 100% sure.

@tsteur
Copy link
Member

tsteur commented Nov 6, 2019

Looks good in general @MichaelHeerklotz but haven't tested it yet. Could you maybe add an example to Morpheus/templates/demo.twig so it will be rendered in a screenshot test?

@MichaelRoosz
Copy link
Contributor Author

@tsteur Thank you for your feedback! I have fixed/updated the styles a bit and updated the demo template.

var self = this;
$scope.$watch('formValue', function () {
$scope.$watch('formValue', function () {;
Copy link
Member

Choose a reason for hiding this comment

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

The semicolon might be there by accident?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$scope.$watch('formValue', function () {;
$scope.$watch('formValue', function () {

@tsteur
Copy link
Member

tsteur commented Nov 11, 2019

Change looks good 👍 any chance you could include this UI change https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/36950/Morpheus_load.png ? (screenshot update looks expected)

@sgiehl
Copy link
Member

sgiehl commented Feb 4, 2020

@tsteur guess we won't merge that in the last 3.x release, right? Could update the base branch otherwise, merge into 4.x-dev and update the screenshot afterwards...

@tsteur
Copy link
Member

tsteur commented Feb 4, 2020

Yes, Matomo 4 👍

@sgiehl sgiehl changed the base branch from 3.x-dev to 4.x-dev February 6, 2020 07:06
@sgiehl sgiehl added this to the 4.0.0 milestone Feb 6, 2020
@sgiehl sgiehl changed the base branch from 4.x-dev to multipairfield_4_field_support February 7, 2020 12:57
@sgiehl
Copy link
Member

sgiehl commented Feb 7, 2020

I'll merge this into a local branch and recreate the PR with updated tests

@sgiehl sgiehl merged commit f3a6eb3 into matomo-org:multipairfield_4_field_support Feb 7, 2020
@mattab mattab added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Sep 29, 2020
@MichaelRoosz MichaelRoosz deleted the multipairfield_4_field_support branch October 29, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants